From 734a359e6ae98ab0a982a1382dad23fc9e5edeeb Mon Sep 17 00:00:00 2001 From: Yanming Zhou Date: Wed, 31 May 2023 13:17:34 +0800 Subject: [PATCH] Construct LettuceObservationContext with parent observation After this commit, LettuceObservationContext.setParentObservation() is called right after Context rather than Observation created, then Context.getParentObservation() could be used in ObservationPredicate to determine whether Observation should be created. Fixes #2591 Original pull request: #2592 --- .../MicrometerTracingAdapter.java | 25 ++++++++++--------- .../SynchronousIntegrationTests.java | 4 +++ .../lettuce/observability/TestConfig.java | 10 ++++++++ 3 files changed, 27 insertions(+), 12 deletions(-) diff --git a/src/main/java/org/springframework/data/redis/connection/lettuce/observability/MicrometerTracingAdapter.java b/src/main/java/org/springframework/data/redis/connection/lettuce/observability/MicrometerTracingAdapter.java index 36aacd4ba..b05c0cabe 100644 --- a/src/main/java/org/springframework/data/redis/connection/lettuce/observability/MicrometerTracingAdapter.java +++ b/src/main/java/org/springframework/data/redis/connection/lettuce/observability/MicrometerTracingAdapter.java @@ -44,6 +44,7 @@ import org.springframework.lang.Nullable; * arguments will be captured in traces including these that may contain sensitive details. * * @author Mark Paluch + * @author Yanming Zhou * @since 3.0 */ public class MicrometerTracingAdapter implements Tracing { @@ -121,29 +122,28 @@ public class MicrometerTracingAdapter implements Tracing { @Override public Tracer.Span nextSpan() { - return this.postProcessSpan(createObservation()); + return this.postProcessSpan(createObservation(null)); } @Override public Tracer.Span nextSpan(TraceContext traceContext) { - - if (traceContext instanceof MicrometerTraceContext micrometerTraceContext) { - - return micrometerTraceContext.observation == null ? nextSpan() - : postProcessSpan(createObservation().parentObservation(micrometerTraceContext.observation())); - } - - return nextSpan(); + return postProcessSpan(createObservation(traceContext)); } - private Observation createObservation() { + private Observation createObservation(@Nullable TraceContext traceContext) { return RedisObservation.REDIS_COMMAND_OBSERVATION.observation(observationRegistry, - () -> new LettuceObservationContext(serviceName)); + () -> { + LettuceObservationContext context = new LettuceObservationContext(serviceName); + if (traceContext instanceof MicrometerTraceContext micrometerTraceContext) { + context.setParentObservation(micrometerTraceContext.observation); + } + return context; + }); } private Tracer.Span postProcessSpan(Observation observation) { - return observation != null && !observation.isNoop() + return !observation.isNoop() ? new MicrometerSpan(observation.observationConvention(observationConvention)) : NoOpSpan.INSTANCE; } @@ -292,6 +292,7 @@ public class MicrometerTracingAdapter implements Tracing { record MicrometerTraceContextProvider(ObservationRegistry registry) implements TraceContextProvider { @Override + @Nullable public TraceContext getTraceContext() { Observation observation = registry.getCurrentObservation(); diff --git a/src/test/java/org/springframework/data/redis/connection/lettuce/observability/SynchronousIntegrationTests.java b/src/test/java/org/springframework/data/redis/connection/lettuce/observability/SynchronousIntegrationTests.java index 644dd84a7..1e1f5fa19 100644 --- a/src/test/java/org/springframework/data/redis/connection/lettuce/observability/SynchronousIntegrationTests.java +++ b/src/test/java/org/springframework/data/redis/connection/lettuce/observability/SynchronousIntegrationTests.java @@ -35,6 +35,7 @@ import io.micrometer.tracing.test.SampleTestRunner; * Collection of tests that log metrics and tracing using the synchronous API. * * @author Mark Paluch + * @author Yanming Zhou */ @ExtendWith(SpringExtension.class) @ContextConfiguration(classes = TestConfig.class) @@ -77,6 +78,9 @@ public class SynchronousIntegrationTests extends SampleTestRunner { .containsEntry("net.sock.peer.port", "" + SettingsUtils.getPort()); assertThat(finishedSpan.getTags()).containsKeys("db.operation"); } + + assertThat(TestConfig.PARENT_OBSERVATION_NAMES_COLLECTED_IN_PREDICATE).isNotEmpty(); + TestConfig.PARENT_OBSERVATION_NAMES_COLLECTED_IN_PREDICATE.clear(); }; } diff --git a/src/test/java/org/springframework/data/redis/connection/lettuce/observability/TestConfig.java b/src/test/java/org/springframework/data/redis/connection/lettuce/observability/TestConfig.java index d6def0d93..4849890fa 100644 --- a/src/test/java/org/springframework/data/redis/connection/lettuce/observability/TestConfig.java +++ b/src/test/java/org/springframework/data/redis/connection/lettuce/observability/TestConfig.java @@ -22,6 +22,8 @@ import io.micrometer.core.instrument.simple.SimpleMeterRegistry; import io.micrometer.observation.ObservationRegistry; import java.time.Duration; +import java.util.ArrayList; +import java.util.List; import java.util.concurrent.TimeUnit; import org.springframework.context.annotation.Bean; @@ -33,15 +35,23 @@ import org.springframework.data.redis.test.extension.ShutdownQueue; /** * @author Mark Paluch + * @author Yanming Zhou */ @Configuration class TestConfig { static final MeterRegistry METER_REGISTRY = new SimpleMeterRegistry(); static final ObservationRegistry OBSERVATION_REGISTRY = ObservationRegistry.create(); + static final List PARENT_OBSERVATION_NAMES_COLLECTED_IN_PREDICATE = new ArrayList<>(); static { OBSERVATION_REGISTRY.observationConfig().observationHandler(new DefaultMeterObservationHandler(METER_REGISTRY)); + OBSERVATION_REGISTRY.observationConfig().observationPredicate((name, context) -> { + if (context.getParentObservation() != null) { + PARENT_OBSERVATION_NAMES_COLLECTED_IN_PREDICATE.add(context.getParentObservation().getContextView().getName()); + } + return true; + }); } @Bean(destroyMethod = "timer")