From 7c7e4c2c48099c3a1785f80145887ca972464370 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. Fix GH-2591 --- .../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 36aacd4ba8..b05c0cabe7 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 @@ * 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 MicrometerTracer(ObservationRegistry observationRegistry) { @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 void finish() { 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 644dd84a7e..1e1f5fa19b 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 @@ * 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 SampleTestRunnerConsumer yourCode() { .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 d6def0d936..4849890fac 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.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 @@ /** * @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")