From 8418b1890745887611e544a8546f8b6a97f0bf09 Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Mon, 12 Aug 2024 14:06:45 +0100 Subject: [PATCH] Use early static registration of EventPublishingContextWrapper Add `OpenTelemetryEventPublisherApplicationListener` which uses a single static `ContextStorage` wrapper which gets applied as early as possible. The static wrapper is then updated as beans come and go. By adding the wrapper early, we hope to avoid silent failures which can occur if the `ContextStorage` gets initialized before the wrapper is added. Closes gh-41439 --- .../OpenTelemetryAutoConfiguration.java | 5 +- ...etryEventPublisherApplicationListener.java | 178 ++++++++++++++++++ .../main/resources/META-INF/spring.factories | 4 + .../BaggagePropagationIntegrationTests.java | 56 ++++-- .../OpenTelemetryAutoConfigurationTests.java | 44 +++++ 5 files changed, 271 insertions(+), 16 deletions(-) create mode 100644 spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/tracing/OpenTelemetryEventPublisherApplicationListener.java diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/tracing/OpenTelemetryAutoConfiguration.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/tracing/OpenTelemetryAutoConfiguration.java index 7db17f7214..a6d6a8f48c 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/tracing/OpenTelemetryAutoConfiguration.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/tracing/OpenTelemetryAutoConfiguration.java @@ -24,7 +24,6 @@ import io.micrometer.tracing.exporter.SpanFilter; import io.micrometer.tracing.exporter.SpanReporter; import io.micrometer.tracing.otel.bridge.CompositeSpanExporter; import io.micrometer.tracing.otel.bridge.EventListener; -import io.micrometer.tracing.otel.bridge.EventPublishingContextWrapper; import io.micrometer.tracing.otel.bridge.OtelBaggageManager; import io.micrometer.tracing.otel.bridge.OtelCurrentTraceContext; import io.micrometer.tracing.otel.bridge.OtelPropagator; @@ -35,7 +34,6 @@ import io.micrometer.tracing.otel.bridge.Slf4JEventListener; import io.opentelemetry.api.OpenTelemetry; import io.opentelemetry.api.metrics.MeterProvider; import io.opentelemetry.api.trace.Tracer; -import io.opentelemetry.context.ContextStorage; import io.opentelemetry.context.propagation.ContextPropagators; import io.opentelemetry.context.propagation.TextMapPropagator; import io.opentelemetry.sdk.resources.Resource; @@ -164,8 +162,7 @@ public class OpenTelemetryAutoConfiguration { @Bean @ConditionalOnMissingBean - OtelCurrentTraceContext otelCurrentTraceContext(EventPublisher publisher) { - ContextStorage.addWrapper(new EventPublishingContextWrapper(publisher)); + OtelCurrentTraceContext otelCurrentTraceContext() { return new OtelCurrentTraceContext(); } diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/tracing/OpenTelemetryEventPublisherApplicationListener.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/tracing/OpenTelemetryEventPublisherApplicationListener.java new file mode 100644 index 0000000000..eac080a07b --- /dev/null +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/tracing/OpenTelemetryEventPublisherApplicationListener.java @@ -0,0 +1,178 @@ +/* + * Copyright 2012-2024 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.boot.actuate.autoconfigure.tracing; + +import java.util.List; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.function.UnaryOperator; + +import io.micrometer.tracing.otel.bridge.EventPublishingContextWrapper; +import io.micrometer.tracing.otel.bridge.OtelTracer.EventPublisher; +import io.opentelemetry.context.Context; +import io.opentelemetry.context.ContextStorage; +import io.opentelemetry.context.Scope; + +import org.springframework.boot.context.event.ApplicationStartingEvent; +import org.springframework.context.ApplicationContext; +import org.springframework.context.ApplicationEvent; +import org.springframework.context.ApplicationListener; +import org.springframework.context.event.ContextClosedEvent; +import org.springframework.context.event.ContextRefreshedEvent; +import org.springframework.context.event.GenericApplicationListener; +import org.springframework.core.Ordered; +import org.springframework.core.ResolvableType; +import org.springframework.util.ClassUtils; +import org.springframework.util.LinkedMultiValueMap; +import org.springframework.util.MultiValueMap; + +/** + * {@link ApplicationListener} to add an OpenTelemetry {@link ContextStorage} wrapper for + * {@link EventPublisher} bean support. A single {@link ContextStorage} wrapper is added + * as early as possible then updated with {@link EventPublisher} beans as needed. + * + * @author Phillip Webb + */ +class OpenTelemetryEventPublisherApplicationListener implements GenericApplicationListener { + + private static final boolean OTEL_CONTEXT_PRESENT = ClassUtils.isPresent("io.opentelemetry.context.ContextStorage", + null); + + private static final boolean MICROMETER_OTEL_PRESENT = ClassUtils + .isPresent("io.micrometer.tracing.otel.bridge.OtelTracer", null); + + @Override + public int getOrder() { + return Ordered.HIGHEST_PRECEDENCE; + } + + @Override + public boolean supportsEventType(ResolvableType eventType) { + Class type = eventType.getRawClass(); + return (type != null) && (ApplicationStartingEvent.class.isAssignableFrom(type) + || ContextRefreshedEvent.class.isAssignableFrom(type) + || ContextClosedEvent.class.isAssignableFrom(type)); + } + + @Override + public void onApplicationEvent(ApplicationEvent event) { + if (!OTEL_CONTEXT_PRESENT || !MICROMETER_OTEL_PRESENT) { + return; + } + if (event instanceof ApplicationStartingEvent) { + EventPublisherBeansContextWrapper.addWrapperIfNecessary(); + } + if (event instanceof ContextRefreshedEvent contextRefreshedEvent) { + ApplicationContext applicationContext = contextRefreshedEvent.getApplicationContext(); + List publishers = applicationContext + .getBeansOfType(EventPublisher.class, true, false) + .values() + .stream() + .map(EventPublishingContextWrapper::new) + .toList(); + EventPublisherBeansContextWrapper.instance.put(applicationContext, publishers); + } + if (event instanceof ContextClosedEvent contextClosedEvent) { + EventPublisherBeansContextWrapper.instance.remove(contextClosedEvent.getApplicationContext()); + } + } + + /** + * The single {@link ContextStorage} wrapper that delegates to {@link EventPublisher} + * beans. + */ + static class EventPublisherBeansContextWrapper implements UnaryOperator { + + private static final AtomicBoolean added = new AtomicBoolean(); + + private static final EventPublisherBeansContextWrapper instance = new EventPublisherBeansContextWrapper(); + + private final MultiValueMap publishers = new LinkedMultiValueMap<>(); + + private volatile ContextStorage delegate; + + static void addWrapperIfNecessary() { + if (added.compareAndSet(false, true)) { + ContextStorage.addWrapper(instance); + } + } + + @Override + public ContextStorage apply(ContextStorage contextStorage) { + return new EventPublisherBeansContextStorage(contextStorage); + } + + void put(ApplicationContext applicationContext, List publishers) { + synchronized (this) { + this.publishers.addAll(applicationContext, publishers); + this.delegate = null; + } + } + + void remove(ApplicationContext applicationContext) { + synchronized (this) { + this.publishers.remove(applicationContext); + this.delegate = null; + } + } + + private ContextStorage getDelegate(ContextStorage parent) { + ContextStorage delegate = this.delegate; + if (delegate == null) { + synchronized (this) { + delegate = parent; + for (List publishers : this.publishers.values()) { + for (EventPublishingContextWrapper publisher : publishers) { + delegate = publisher.apply(delegate); + } + } + } + } + return delegate; + } + + /** + * The wrapped {@link ContextStorage} that delegates to the + * {@link EventPublisherBeansContextWrapper}. + */ + class EventPublisherBeansContextStorage implements ContextStorage { + + private final ContextStorage parent; + + EventPublisherBeansContextStorage(ContextStorage wrapped) { + this.parent = wrapped; + } + + @Override + public Scope attach(Context toAttach) { + return getDelegate(this.parent).attach(toAttach); + } + + @Override + public Context current() { + return getDelegate(this.parent).current(); + } + + @Override + public Context root() { + return getDelegate(this.parent).root(); + } + + } + + } + +} diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/resources/META-INF/spring.factories b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/resources/META-INF/spring.factories index e6f6ca994b..f2ae3549ac 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/resources/META-INF/spring.factories +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/resources/META-INF/spring.factories @@ -10,3 +10,7 @@ org.springframework.boot.actuate.autoconfigure.metrics.ValidationFailureAnalyzer # Environment Post Processors org.springframework.boot.env.EnvironmentPostProcessor=\ org.springframework.boot.actuate.autoconfigure.tracing.LogCorrelationEnvironmentPostProcessor + +# Application Listeners +org.springframework.context.ApplicationListener=\ +org.springframework.boot.actuate.autoconfigure.tracing.OpenTelemetryEventPublisherApplicationListener diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/tracing/BaggagePropagationIntegrationTests.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/tracing/BaggagePropagationIntegrationTests.java index c4b2e29de4..6cda28003d 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/tracing/BaggagePropagationIntegrationTests.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/tracing/BaggagePropagationIntegrationTests.java @@ -31,10 +31,13 @@ import org.junit.jupiter.params.provider.EnumSource; import org.slf4j.MDC; import org.springframework.boot.actuate.autoconfigure.opentelemetry.OpenTelemetryAutoConfiguration; +import org.springframework.boot.actuate.autoconfigure.tracing.OpenTelemetryEventPublisherApplicationListener.EventPublisherBeansContextWrapper; import org.springframework.boot.autoconfigure.AutoConfigurations; import org.springframework.boot.test.context.runner.ApplicationContextRunner; import org.springframework.boot.testsupport.classpath.ForkedClassPath; import org.springframework.context.ApplicationContext; +import org.springframework.context.ApplicationContextInitializer; +import org.springframework.context.ConfigurableApplicationContext; import static org.assertj.core.api.Assertions.assertThat; @@ -55,6 +58,7 @@ class BaggagePropagationIntegrationTests { @BeforeEach @AfterEach void setup() { + EventPublisherBeansContextWrapper.addWrapperIfNecessary(); MDC.clear(); } @@ -155,6 +159,7 @@ class BaggagePropagationIntegrationTests { enum AutoConfig implements Supplier { BRAVE_DEFAULT { + @Override public ApplicationContextRunner get() { return new ApplicationContextRunner() @@ -162,20 +167,24 @@ class BaggagePropagationIntegrationTests { .withPropertyValues("management.tracing.baggage.remote-fields=x-vcap-request-id,country-code,bp", "management.tracing.baggage.correlation.fields=country-code,bp"); } + }, OTEL_DEFAULT { + @Override public ApplicationContextRunner get() { - return new ApplicationContextRunner().withConfiguration(AutoConfigurations.of( - OpenTelemetryAutoConfiguration.class, - org.springframework.boot.actuate.autoconfigure.tracing.OpenTelemetryAutoConfiguration.class)) + return new ApplicationContextRunner().withInitializer(new OtelApplicationContextInitializer()) + .withConfiguration(AutoConfigurations.of(OpenTelemetryAutoConfiguration.class, + org.springframework.boot.actuate.autoconfigure.tracing.OpenTelemetryAutoConfiguration.class)) .withPropertyValues("management.tracing.baggage.remote-fields=x-vcap-request-id,country-code,bp", "management.tracing.baggage.correlation.fields=country-code,bp"); } + }, BRAVE_W3C { + @Override public ApplicationContextRunner get() { return new ApplicationContextRunner() @@ -184,21 +193,25 @@ class BaggagePropagationIntegrationTests { "management.tracing.baggage.remote-fields=x-vcap-request-id,country-code,bp", "management.tracing.baggage.correlation.fields=country-code,bp"); } + }, OTEL_W3C { + @Override public ApplicationContextRunner get() { - return new ApplicationContextRunner().withConfiguration(AutoConfigurations.of( - OpenTelemetryAutoConfiguration.class, - org.springframework.boot.actuate.autoconfigure.tracing.OpenTelemetryAutoConfiguration.class)) + return new ApplicationContextRunner().withInitializer(new OtelApplicationContextInitializer()) + .withConfiguration(AutoConfigurations.of(OpenTelemetryAutoConfiguration.class, + org.springframework.boot.actuate.autoconfigure.tracing.OpenTelemetryAutoConfiguration.class)) .withPropertyValues("management.tracing.propagation.type=W3C", "management.tracing.baggage.remote-fields=x-vcap-request-id,country-code,bp", "management.tracing.baggage.correlation.fields=country-code,bp"); } + }, BRAVE_B3 { + @Override public ApplicationContextRunner get() { return new ApplicationContextRunner() @@ -207,9 +220,11 @@ class BaggagePropagationIntegrationTests { "management.tracing.baggage.remote-fields=x-vcap-request-id,country-code,bp", "management.tracing.baggage.correlation.fields=country-code,bp"); } + }, BRAVE_B3_MULTI { + @Override public ApplicationContextRunner get() { return new ApplicationContextRunner() @@ -218,33 +233,39 @@ class BaggagePropagationIntegrationTests { "management.tracing.baggage.remote-fields=x-vcap-request-id,country-code,bp", "management.tracing.baggage.correlation.fields=country-code,bp"); } + }, OTEL_B3 { + @Override public ApplicationContextRunner get() { - return new ApplicationContextRunner().withConfiguration(AutoConfigurations.of( - OpenTelemetryAutoConfiguration.class, - org.springframework.boot.actuate.autoconfigure.tracing.OpenTelemetryAutoConfiguration.class)) + return new ApplicationContextRunner().withInitializer(new OtelApplicationContextInitializer()) + .withConfiguration(AutoConfigurations.of(OpenTelemetryAutoConfiguration.class, + org.springframework.boot.actuate.autoconfigure.tracing.OpenTelemetryAutoConfiguration.class)) .withPropertyValues("management.tracing.propagation.type=B3", "management.tracing.baggage.remote-fields=x-vcap-request-id,country-code,bp", "management.tracing.baggage.correlation.fields=country-code,bp"); } + }, OTEL_B3_MULTI { + @Override public ApplicationContextRunner get() { - return new ApplicationContextRunner().withConfiguration(AutoConfigurations.of( - OpenTelemetryAutoConfiguration.class, - org.springframework.boot.actuate.autoconfigure.tracing.OpenTelemetryAutoConfiguration.class)) + return new ApplicationContextRunner().withInitializer(new OtelApplicationContextInitializer()) + .withConfiguration(AutoConfigurations.of(OpenTelemetryAutoConfiguration.class, + org.springframework.boot.actuate.autoconfigure.tracing.OpenTelemetryAutoConfiguration.class)) .withPropertyValues("management.tracing.propagation.type=B3_MULTI", "management.tracing.baggage.remote-fields=x-vcap-request-id,country-code,bp", "management.tracing.baggage.correlation.fields=country-code,bp"); } + }, BRAVE_LOCAL_FIELDS { + @Override public ApplicationContextRunner get() { return new ApplicationContextRunner() @@ -252,6 +273,7 @@ class BaggagePropagationIntegrationTests { .withPropertyValues("management.tracing.baggage.local-fields=country-code,bp", "management.tracing.baggage.correlation.fields=country-code,bp"); } + }; boolean isOtel() { @@ -264,4 +286,14 @@ class BaggagePropagationIntegrationTests { } + static class OtelApplicationContextInitializer + implements ApplicationContextInitializer { + + @Override + public void initialize(ConfigurableApplicationContext applicationContext) { + applicationContext.addApplicationListener(new OpenTelemetryEventPublisherApplicationListener()); + } + + } + } diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/tracing/OpenTelemetryAutoConfigurationTests.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/tracing/OpenTelemetryAutoConfigurationTests.java index b96ac2da59..c7cc14af33 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/tracing/OpenTelemetryAutoConfigurationTests.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/tracing/OpenTelemetryAutoConfigurationTests.java @@ -26,6 +26,8 @@ import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; import io.micrometer.tracing.SpanCustomizer; +import io.micrometer.tracing.Tracer.SpanInScope; +import io.micrometer.tracing.otel.bridge.EventListener; import io.micrometer.tracing.otel.bridge.OtelCurrentTraceContext; import io.micrometer.tracing.otel.bridge.OtelPropagator; import io.micrometer.tracing.otel.bridge.OtelSpanCustomizer; @@ -37,6 +39,7 @@ import io.micrometer.tracing.otel.propagation.BaggageTextMapPropagator; import io.opentelemetry.api.common.AttributeKey; import io.opentelemetry.api.common.Attributes; import io.opentelemetry.api.metrics.MeterProvider; +import io.opentelemetry.api.trace.Span; import io.opentelemetry.api.trace.Tracer; import io.opentelemetry.api.trace.propagation.W3CTraceContextPropagator; import io.opentelemetry.context.propagation.ContextPropagators; @@ -52,15 +55,18 @@ import io.opentelemetry.sdk.trace.export.SpanExporter; import io.opentelemetry.sdk.trace.samplers.Sampler; import io.opentelemetry.semconv.ResourceAttributes; import org.assertj.core.api.InstanceOfAssertFactories; +import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.ValueSource; import org.mockito.Mockito; import org.springframework.boot.actuate.autoconfigure.observation.ObservationAutoConfiguration; +import org.springframework.boot.actuate.autoconfigure.tracing.OpenTelemetryEventPublisherApplicationListener.EventPublisherBeansContextWrapper; import org.springframework.boot.autoconfigure.AutoConfigurations; import org.springframework.boot.test.context.FilteredClassLoader; import org.springframework.boot.test.context.runner.ApplicationContextRunner; +import org.springframework.context.ConfigurableApplicationContext; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.core.annotation.Order; @@ -86,6 +92,11 @@ class OpenTelemetryAutoConfigurationTests { org.springframework.boot.actuate.autoconfigure.opentelemetry.OpenTelemetryAutoConfiguration.class, OpenTelemetryAutoConfiguration.class)); + @BeforeAll + static void addWrapper() { + EventPublisherBeansContextWrapper.addWrapperIfNecessary(); + } + @Test void shouldSupplyBeans() { this.contextRunner.run((context) -> { @@ -316,6 +327,28 @@ class OpenTelemetryAutoConfigurationTests { }); } + @Test // gh-41439 + void shouldPublishEventsWhenContextStorageIsInitializedEarly() { + this.contextRunner.withInitializer(this::initializeOpenTelemetry) + .withUserConfiguration(OtelEventListener.class) + .run((context) -> { + OtelEventListener listener = context.getBean(OtelEventListener.class); + io.micrometer.tracing.Tracer micrometerTracer = context.getBean(io.micrometer.tracing.Tracer.class); + io.micrometer.tracing.Span span = micrometerTracer.nextSpan().name("test"); + try (SpanInScope scoped = micrometerTracer.withSpan(span.start())) { + assertThat(listener.events).isNotEmpty(); + } + finally { + span.end(); + } + }); + } + + private void initializeOpenTelemetry(ConfigurableApplicationContext context) { + context.addApplicationListener(new OpenTelemetryEventPublisherApplicationListener()); + Span.current(); + } + private List getInjectors(TextMapPropagator propagator) { assertThat(propagator).as("propagator").isNotNull(); if (propagator instanceof CompositeTextMapPropagator compositePropagator) { @@ -531,4 +564,15 @@ class OpenTelemetryAutoConfigurationTests { } + static class OtelEventListener implements EventListener { + + private final List events = new ArrayList<>(); + + @Override + public void onEvent(Object event) { + this.events.add(event); + } + + } + }