Skip to content

Commit

Permalink
Make Context.getParentObservation() available for ObservationPredicate (
Browse files Browse the repository at this point in the history
#3867)

Before these changes, the parent is not available in the context when the ObservationPredicate instances are called, which means decisions cannot be made based on the parent.

Resolves gh-3678
  • Loading branch information
quaff committed Aug 16, 2023
1 parent 02165d0 commit c959eb3
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
* @author Jonatan Ivanov
* @author Tommy Ludwig
* @author Marcin Grzejszczak
* @author Yanming Zhou
* @since 1.10.0
*/
public interface Observation extends ObservationView {
Expand Down Expand Up @@ -125,10 +126,11 @@ static <T extends Context> Observation createNotStarted(String name, Supplier<T>
return NOOP;
}
Context context = contextSupplier.get();
context.setParentFromCurrentObservation(registry);
if (!registry.observationConfig().isObservationEnabled(name, context)) {
return NOOP;
}
return new SimpleObservation(name, registry, context == null ? new Context() : context);
return new SimpleObservation(name, registry, context);
}

// @formatter:off
Expand Down Expand Up @@ -168,6 +170,7 @@ static <T extends Context> Observation createNotStarted(@Nullable ObservationCon
}
ObservationConvention<T> convention;
T context = contextSupplier.get();
context.setParentFromCurrentObservation(registry);
if (customConvention != null) {
convention = customConvention;
}
Expand All @@ -177,7 +180,7 @@ static <T extends Context> Observation createNotStarted(@Nullable ObservationCon
if (!registry.observationConfig().isObservationEnabled(convention.getName(), context)) {
return NOOP;
}
return new SimpleObservation(convention, registry, context == null ? new Context() : context);
return new SimpleObservation(convention, registry, context);
}

/**
Expand Down Expand Up @@ -311,10 +314,11 @@ static <T extends Context> Observation createNotStarted(ObservationConvention<T>
return NOOP;
}
T context = contextSupplier.get();
context.setParentFromCurrentObservation(registry);
if (!registry.observationConfig().isObservationEnabled(observationConvention.getName(), context)) {
return NOOP;
}
return new SimpleObservation(observationConvention, registry, context == null ? new Context() : context);
return new SimpleObservation(observationConvention, registry, context);
}

/**
Expand Down Expand Up @@ -977,6 +981,20 @@ public void setParentObservation(@Nullable ObservationView parentObservation) {
this.parentObservation = parentObservation;
}

/**
* Sets the parent {@link ObservationView} to current one if parent is null and
* current one exists.
* @param registry the {@link ObservationRegistry} in using
*/
void setParentFromCurrentObservation(ObservationRegistry registry) {
if (this.parentObservation == null) {
Observation currentObservation = registry.getCurrentObservation();
if (currentObservation != null) {
setParentObservation(currentObservation);
}
}
}

/**
* Error that occurred while processing the {@link Observation}.
* @return error (null if there wasn't any)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
* @author Jonatan Ivanov
* @author Tommy Ludwig
* @author Marcin Grzejszczak
* @author Yanming Zhou
* @since 1.10.0
*/
class SimpleObservation implements Observation {
Expand All @@ -61,7 +62,6 @@ class SimpleObservation implements Observation {
this.convention = getConventionFromConfig(registry, context);
this.handlers = getHandlersFromConfig(registry, context);
this.filters = registry.observationConfig().getObservationFilters();
setParentFromCurrentObservation();
}

SimpleObservation(ObservationConvention<? extends Context> convention, ObservationRegistry registry,
Expand All @@ -78,14 +78,6 @@ class SimpleObservation implements Observation {
throw new IllegalStateException(
"Convention [" + convention + "] doesn't support context [" + context + "]");
}
setParentFromCurrentObservation();
}

private void setParentFromCurrentObservation() {
Observation currentObservation = this.registry.getCurrentObservation();
if (currentObservation != null) {
this.context.setParentObservation(currentObservation);
}
}

@Nullable
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
* @author Jonatan Ivanov
* @author Tommy Ludwig
* @author Marcin Grzejszczak
* @author Yanming Zhou
*/
class ObservationTests {

Expand Down Expand Up @@ -80,6 +81,20 @@ void matchingPredicateAndHandlerShouldNotResultInNoopObservation() {
assertThat(observation).isNotSameAs(Observation.NOOP);
}

@Test
void usingParentObservationToMatchPredicate() {
registry.observationConfig().observationHandler(context -> true);
registry.observationConfig()
.observationPredicate((s, context) -> !s.equals("child") || context.getParentObservation() != null);

Observation childWithoutParent = Observation.createNotStarted("child", registry);
assertThat(childWithoutParent).isSameAs(Observation.NOOP);

Observation childWithParent = Observation.createNotStarted("parent", registry)
.observe(() -> Observation.createNotStarted("child", registry));
assertThat(childWithParent).isNotSameAs(Observation.NOOP);
}

@Test
void havingAnObservationFilterWillMutateTheContext() {
registry.observationConfig().observationHandler(context -> true);
Expand Down

0 comments on commit c959eb3

Please sign in to comment.