Skip to content

Scoped checked actions #3323

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Aug 1, 2022
Merged

Scoped checked actions #3323

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1af1fb9
Add methods similar to scoped and tryScoped that can handle checked c…
jonatan-ivanov Jul 29, 2022
c16dc3a
Add tests
jonatan-ivanov Jul 29, 2022
b5e4920
Add more verifications to scoped tests in TCK
jonatan-ivanov Aug 1, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
179 changes: 168 additions & 11 deletions ...test/src/main/java/io/micrometer/observation/tck/ObservationRegistryCompatibilityKit.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -293,54 +293,172 @@ void callableThrowingErrorShouldBeObserved() {

@Test
void runnableShouldBeScoped() {
registry.observationConfig().observationHandler(c -> true);
ObservationHandler<Observation.Context> handler = mock(ObservationHandler.class);
when(handler.supportsContext(isA(Observation.Context.class))).thenReturn(true);
registry.observationConfig().observationHandler(handler);
Observation observation = Observation.start("myObservation", registry);
Runnable runnable = () -> assertThat(registry.getCurrentObservation()).isSameAs(observation);
observation.scoped(runnable);
assertThat(registry.getCurrentObservation()).isNull();
assertThat(observation.getContext().getError()).isEmpty();

InOrder inOrder = inOrder(handler);
inOrder.verify(handler).onScopeOpened(isA(Observation.Context.class));
inOrder.verify(handler).onScopeClosed(isA(Observation.Context.class));
inOrder.verify(handler, times(0)).onError(isA(Observation.Context.class));
inOrder.verify(handler, times(0)).onStop(isA(Observation.Context.class));
}

@Test
void errorShouldBeReportedOnFailingScopedRunnable() {
registry.observationConfig().observationHandler(c -> true);
Observation.Context context = new Observation.Context();
Observation observation = Observation.start("myObservation", context, registry);
ObservationHandler<Observation.Context> handler = mock(ObservationHandler.class);
when(handler.supportsContext(isA(Observation.Context.class))).thenReturn(true);
registry.observationConfig().observationHandler(handler);
Observation observation = Observation.start("myObservation", registry);
RuntimeException error = new RuntimeException("simulated");

Runnable runnable = () -> {
assertThat(registry.getCurrentObservation()).isSameAs(observation);
throw error;
};
assertThatThrownBy(() -> observation.scoped(runnable)).isSameAs(error);
assertThat(context.getError()).containsSame(error);
assertThat(registry.getCurrentObservation()).isNull();
assertThat(observation.getContext().getError()).containsSame(error);

InOrder inOrder = inOrder(handler);
inOrder.verify(handler).onScopeOpened(isA(Observation.Context.class));
inOrder.verify(handler).onScopeClosed(isA(Observation.Context.class));
inOrder.verify(handler).onError(isA(Observation.Context.class));
inOrder.verify(handler, times(0)).onStop(isA(Observation.Context.class));
}

@Test
void checkedRunnableShouldBeScoped() throws Throwable {
ObservationHandler<Observation.Context> handler = mock(ObservationHandler.class);
when(handler.supportsContext(isA(Observation.Context.class))).thenReturn(true);
registry.observationConfig().observationHandler(handler);
Observation observation = Observation.start("myObservation", registry);
Observation.CheckedRunnable runnable = () -> assertThat(registry.getCurrentObservation()).isSameAs(observation);
observation.scopedChecked(runnable);
assertThat(registry.getCurrentObservation()).isNull();
assertThat(observation.getContext().getError()).isEmpty();

InOrder inOrder = inOrder(handler);
inOrder.verify(handler).onScopeOpened(isA(Observation.Context.class));
inOrder.verify(handler).onScopeClosed(isA(Observation.Context.class));
inOrder.verify(handler, times(0)).onError(isA(Observation.Context.class));
inOrder.verify(handler, times(0)).onStop(isA(Observation.Context.class));
}

@Test
void errorShouldBeReportedOnFailingScopedCheckedRunnable() {
ObservationHandler<Observation.Context> handler = mock(ObservationHandler.class);
when(handler.supportsContext(isA(Observation.Context.class))).thenReturn(true);
registry.observationConfig().observationHandler(handler);
Observation observation = Observation.start("myObservation", registry);
RuntimeException error = new RuntimeException("simulated");

Observation.CheckedRunnable runnable = () -> {
assertThat(registry.getCurrentObservation()).isSameAs(observation);
throw error;
};
assertThatThrownBy(() -> observation.scopedChecked(runnable)).isSameAs(error);
assertThat(registry.getCurrentObservation()).isNull();
assertThat(observation.getContext().getError()).containsSame(error);

InOrder inOrder = inOrder(handler);
inOrder.verify(handler).onScopeOpened(isA(Observation.Context.class));
inOrder.verify(handler).onScopeClosed(isA(Observation.Context.class));
inOrder.verify(handler).onError(isA(Observation.Context.class));
inOrder.verify(handler, times(0)).onStop(isA(Observation.Context.class));
}

@Test
void supplierShouldBeScoped() {
registry.observationConfig().observationHandler(c -> true);
ObservationHandler<Observation.Context> handler = mock(ObservationHandler.class);
when(handler.supportsContext(isA(Observation.Context.class))).thenReturn(true);
registry.observationConfig().observationHandler(handler);
Observation observation = Observation.start("myObservation", registry);
Supplier<String> supplier = () -> {
assertThat(registry.getCurrentObservation()).isSameAs(observation);
return "test";
};
String result = observation.scoped(supplier);
assertThat(result).isEqualTo("test");
assertThat(registry.getCurrentObservation()).isNull();
assertThat(result).isEqualTo("test");

InOrder inOrder = inOrder(handler);
inOrder.verify(handler).onScopeOpened(isA(Observation.Context.class));
inOrder.verify(handler).onScopeClosed(isA(Observation.Context.class));
inOrder.verify(handler, times(0)).onError(isA(Observation.Context.class));
inOrder.verify(handler, times(0)).onStop(isA(Observation.Context.class));
}

@Test
void errorShouldBeReportedOnFailingScopedSupplier() {
registry.observationConfig().observationHandler(c -> true);
Observation.Context context = new Observation.Context();
Observation observation = Observation.start("myObservation", context, registry);
ObservationHandler<Observation.Context> handler = mock(ObservationHandler.class);
when(handler.supportsContext(isA(Observation.Context.class))).thenReturn(true);
registry.observationConfig().observationHandler(handler);
Observation observation = Observation.start("myObservation", registry);
RuntimeException error = new RuntimeException("simulated");

Supplier<String> supplier = () -> {
assertThat(registry.getCurrentObservation()).isSameAs(observation);
throw error;
};
assertThatThrownBy(() -> observation.scoped(supplier)).isSameAs(error);
assertThat(context.getError()).containsSame(error);
assertThat(registry.getCurrentObservation()).isNull();
assertThat(observation.getContext().getError()).containsSame(error);

InOrder inOrder = inOrder(handler);
inOrder.verify(handler).onScopeOpened(isA(Observation.Context.class));
inOrder.verify(handler).onScopeClosed(isA(Observation.Context.class));
inOrder.verify(handler).onError(isA(Observation.Context.class));
inOrder.verify(handler, times(0)).onStop(isA(Observation.Context.class));
}

@Test
void checkedCallableShouldBeScoped() throws Throwable {
ObservationHandler<Observation.Context> handler = mock(ObservationHandler.class);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💬 7 similar findings have been found in this PR

unchecked: unchecked conversion

Expand here to view all instances of this finding
File Path Line Number
micrometer-observation-test/src/main/java/io/micrometer/observation/tck/ObservationRegistryCompatibilityKit.java 443
micrometer-observation-test/src/main/java/io/micrometer/observation/tck/ObservationRegistryCompatibilityKit.java 337
micrometer-observation-test/src/main/java/io/micrometer/observation/tck/ObservationRegistryCompatibilityKit.java 399
micrometer-observation-test/src/main/java/io/micrometer/observation/tck/ObservationRegistryCompatibilityKit.java 355
micrometer-observation-test/src/main/java/io/micrometer/observation/tck/ObservationRegistryCompatibilityKit.java 314
micrometer-observation-test/src/main/java/io/micrometer/observation/tck/ObservationRegistryCompatibilityKit.java 378
micrometer-observation-test/src/main/java/io/micrometer/observation/tck/ObservationRegistryCompatibilityKit.java 296

Visit the Lift Web Console to find more details in your report.

Reply with "@sonatype-lift help" for info about LiftBot commands.
Reply with "@sonatype-lift ignore" to tell LiftBot to leave out the above finding from this PR.
Reply with "@sonatype-lift ignoreall" to tell LiftBot to leave out all the findings from this PR and from the status bar in Github.

When talking to LiftBot, you need to refresh the page to see its response. Click here to get to know more about LiftBot commands.

Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

when(handler.supportsContext(isA(Observation.Context.class))).thenReturn(true);
registry.observationConfig().observationHandler(handler);
Observation observation = Observation.start("myObservation", registry);
Observation.CheckedCallable<String> callable = () -> {
assertThat(registry.getCurrentObservation()).isSameAs(observation);
return "test";
};
String result = observation.scopedChecked(callable);
assertThat(registry.getCurrentObservation()).isNull();
assertThat(result).isEqualTo("test");

InOrder inOrder = inOrder(handler);
inOrder.verify(handler).onScopeOpened(isA(Observation.Context.class));
inOrder.verify(handler).onScopeClosed(isA(Observation.Context.class));
inOrder.verify(handler, times(0)).onError(isA(Observation.Context.class));
inOrder.verify(handler, times(0)).onStop(isA(Observation.Context.class));
}

@Test
void errorShouldBeReportedOnFailingScopedCheckedCallable() {
ObservationHandler<Observation.Context> handler = mock(ObservationHandler.class);
when(handler.supportsContext(isA(Observation.Context.class))).thenReturn(true);
registry.observationConfig().observationHandler(handler);
Observation observation = Observation.start("myObservation", registry);
RuntimeException error = new RuntimeException("simulated");

Observation.CheckedCallable<String> callable = () -> {
assertThat(registry.getCurrentObservation()).isSameAs(observation);
throw error;
};
assertThatThrownBy(() -> observation.scopedChecked(callable)).isSameAs(error);
assertThat(registry.getCurrentObservation()).isNull();
assertThat(observation.getContext().getError()).containsSame(error);

InOrder inOrder = inOrder(handler);
inOrder.verify(handler).onScopeOpened(isA(Observation.Context.class));
inOrder.verify(handler).onScopeClosed(isA(Observation.Context.class));
inOrder.verify(handler).onError(isA(Observation.Context.class));
inOrder.verify(handler, times(0)).onStop(isA(Observation.Context.class));
}

@Test
Expand All @@ -359,6 +477,22 @@ void runnableShouldNotBeParentScopedIfParentIsNull() {
assertThat(registry.getCurrentObservation()).isNull();
}

@Test
void checkedRunnableShouldBeParentScoped() throws Throwable {
registry.observationConfig().observationHandler(c -> true);
Observation parent = Observation.start("myObservation", registry);
Observation.CheckedRunnable runnable = () -> assertThat(registry.getCurrentObservation()).isSameAs(parent);
Observation.tryScopedChecked(parent, runnable);
assertThat(registry.getCurrentObservation()).isNull();
}

@Test
void checkedRunnableShouldNotBeParentScopedIfParentIsNull() throws Throwable {
Observation.CheckedRunnable runnable = () -> assertThat(registry.getCurrentObservation()).isNull();
Observation.tryScopedChecked(null, runnable);
assertThat(registry.getCurrentObservation()).isNull();
}

@Test
void supplierShouldBeParentScoped() {
registry.observationConfig().observationHandler(c -> true);
Expand All @@ -382,6 +516,29 @@ void supplierShouldNotBeParentScopedIfParentIsNull() {
assertThat(registry.getCurrentObservation()).isNull();
}

@Test
void checkedCallableShouldBeParentScoped() throws Throwable {
registry.observationConfig().observationHandler(c -> true);
Observation parent = Observation.start("myObservation", registry);
Observation.CheckedCallable<String> callable = () -> {
assertThat(registry.getCurrentObservation()).isSameAs(parent);
return "test";
};
String result = Observation.tryScopedChecked(parent, callable);
assertThat(result).isEqualTo("test");
assertThat(registry.getCurrentObservation()).isNull();
}

@Test
void checkedCallableShouldNotBeParentScopedIfParentIsNull() throws Throwable {
Observation.CheckedCallable<String> callable = () -> {
assertThat(registry.getCurrentObservation()).isNull();
return "test";
};
Observation.tryScopedChecked(null, callable);
assertThat(registry.getCurrentObservation()).isNull();
}

@Test
void observationFieldsShouldBeSetOnContext() {
AssertingHandler assertingHandler = new AssertingHandler();
Expand Down
81 changes: 72 additions & 9 deletions micrometer-observation/src/main/java/io/micrometer/observation/Observation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -492,13 +492,13 @@ default <T> T observeChecked(CheckedCallable<T> checkedCallable) throws Throwabl
}

/**
* Wraps the given action in scope.
* @param action action to run
* Wraps the given action in a scope and signals an error.
* @param runnable the {@link Runnable} to run
*/
@SuppressWarnings("unused")
default void scoped(Runnable action) {
default void scoped(Runnable runnable) {
try (Scope scope = openScope()) {
action.run();
runnable.run();
}
catch (Exception exception) {
error(exception);
Expand All @@ -507,21 +507,54 @@ default void scoped(Runnable action) {
}

/**
* Wraps the given action in scope.
* @param action action to run
* @return result of the action
* Wraps the given action in a scope and signals an error.
* @param checkedRunnable the {@link CheckedRunnable} to run
*/
@SuppressWarnings("unused")
default void scopedChecked(CheckedRunnable checkedRunnable) throws Throwable {
try (Scope scope = openScope()) {
checkedRunnable.run();
}
catch (Throwable throwable) {
error(throwable);
throw throwable;
}
}

/**
* Wraps the given action in a scope and signals an error.
* @param supplier the {@link Supplier} to call
* @param <T> the type parameter of the {@link Supplier}
* @return the result from {@link Supplier#get()}
*/
@SuppressWarnings("unused")
default <T> T scoped(Supplier<T> action) {
default <T> T scoped(Supplier<T> supplier) {
try (Scope scope = openScope()) {
return action.get();
return supplier.get();
}
catch (Exception exception) {
error(exception);
throw exception;
}
}

/**
* Wraps the given action in a scope and signals an error.
* @param checkedCallable the {@link CheckedCallable} to call
* @param <T> the type parameter of the {@link CheckedCallable}
* @return the result from {@link CheckedCallable#call()}
*/
@SuppressWarnings("unused")
default <T> T scopedChecked(CheckedCallable<T> checkedCallable) throws Throwable {
try (Scope scope = openScope()) {
return checkedCallable.call();
}
catch (Throwable error) {
error(error);
throw error;
}
}

/**
* Tries to run the action against an Observation. If the Observation is null, we just
* run the action, otherwise we run the action in scope.
Expand All @@ -537,6 +570,21 @@ static void tryScoped(@Nullable Observation parent, Runnable action) {
}
}

/**
* Tries to run the action against an Observation. If the Observation is null, we just
* run the action, otherwise we run the action in scope.
* @param parent observation, potentially {@code null}
* @param checkedRunnable the {@link CheckedRunnable} to run
*/
static void tryScopedChecked(@Nullable Observation parent, CheckedRunnable checkedRunnable) throws Throwable {
if (parent != null) {
parent.scopedChecked(checkedRunnable);
}
else {
checkedRunnable.run();
}
}

/**
* Tries to run the action against an Observation. If the Observation is null, we just
* run the action, otherwise we run the action in scope.
Expand All @@ -551,6 +599,21 @@ static <T> T tryScoped(@Nullable Observation parent, Supplier<T> action) {
return action.get();
}

/**
* Tries to run the action against an Observation. If the Observation is null, we just
* run the action, otherwise we run the action in scope.
* @param parent observation, potentially {@code null}
* @param checkedCallable the {@link CheckedCallable} to call
* @param <T> the type parameter of the {@link CheckedCallable}
* @return the result from {@link CheckedCallable#call()}
*/
static <T> T tryScopedChecked(@Nullable Observation parent, CheckedCallable<T> checkedCallable) throws Throwable {
if (parent != null) {
return parent.scopedChecked(checkedCallable);
}
return checkedCallable.call();
}

/**
* Scope represent an action within which certain resources (e.g. tracing context) are
* put in scope (e.g. in a ThreadLocal). When the scope is closed the resources will
Expand Down