Skip to content
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

Jump to bottom

Tracer library HTTP instrumentation is auto-configured unnecessarily #33287

Closed
wants to merge 2 commits into from
Closed

Tracer library HTTP instrumentation is auto-configured unnecessarily #33287

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a20edda
Remove Brave HTTP support
jonatan-ivanov Nov 21, 2022
27a62d5
Remove OTel HTTP support
jonatan-ivanov Nov 21, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
41 changes: 0 additions & 41 deletions ...n/java/org/springframework/boot/actuate/autoconfigure/tracing/BraveAutoConfiguration.java
View file
Open in desktop
Expand Up @@ -35,13 +35,6 @@
import brave.baggage.CorrelationScopeDecorator;
import brave.context.slf4j.MDCScopeDecorator;
import brave.handler.SpanHandler;
import brave.http.HttpClientHandler;
import brave.http.HttpClientRequest;
import brave.http.HttpClientResponse;
import brave.http.HttpServerHandler;
import brave.http.HttpServerRequest;
import brave.http.HttpServerResponse;
import brave.http.HttpTracing;
import brave.propagation.B3Propagation;
import brave.propagation.CurrentTraceContext;
import brave.propagation.CurrentTraceContext.ScopeDecorator;
Expand All @@ -51,8 +44,6 @@
import brave.sampler.Sampler;
import io.micrometer.tracing.brave.bridge.BraveBaggageManager;
import io.micrometer.tracing.brave.bridge.BraveCurrentTraceContext;
import io.micrometer.tracing.brave.bridge.BraveHttpClientHandler;
import io.micrometer.tracing.brave.bridge.BraveHttpServerHandler;
import io.micrometer.tracing.brave.bridge.BravePropagator;
import io.micrometer.tracing.brave.bridge.BraveSpanCustomizer;
import io.micrometer.tracing.brave.bridge.BraveTracer;
Expand Down Expand Up @@ -144,24 +135,6 @@ public Sampler braveSampler(TracingProperties properties) {
return Sampler.create(properties.getSampling().getProbability());
}

@Bean
@ConditionalOnMissingBean
public HttpTracing httpTracing(Tracing tracing) {
return HttpTracing.newBuilder(tracing).build();
}

@Bean
@ConditionalOnMissingBean
public HttpServerHandler<HttpServerRequest, HttpServerResponse> httpServerHandler(HttpTracing httpTracing) {
return HttpServerHandler.create(httpTracing);
}

@Bean
@ConditionalOnMissingBean
public HttpClientHandler<HttpClientRequest, HttpClientResponse> httpClientHandler(HttpTracing httpTracing) {
return HttpClientHandler.create(httpTracing);
}

@Bean
@ConditionalOnMissingBean(io.micrometer.tracing.Tracer.class)
BraveTracer braveTracerBridge(brave.Tracer tracer, CurrentTraceContext currentTraceContext) {
Expand All @@ -174,20 +147,6 @@ BravePropagator bravePropagator(Tracing tracing) {
return new BravePropagator(tracing);
}

@Bean
@ConditionalOnMissingBean
BraveHttpServerHandler braveHttpServerHandler(
HttpServerHandler<HttpServerRequest, HttpServerResponse> httpServerHandler) {
return new BraveHttpServerHandler(httpServerHandler);
}

@Bean
@ConditionalOnMissingBean
BraveHttpClientHandler braveHttpClientHandler(
HttpClientHandler<HttpClientRequest, HttpClientResponse> httpClientHandler) {
return new BraveHttpClientHandler(httpClientHandler);
}

@Bean
@ConditionalOnMissingBean(SpanCustomizer.class)
CurrentSpanCustomizer currentSpanCustomizer(Tracing tracing) {
Expand Down
20 changes: 0 additions & 20 deletions ...rg/springframework/boot/actuate/autoconfigure/tracing/OpenTelemetryAutoConfiguration.java
View file
Open in desktop
Expand Up @@ -18,18 +18,12 @@

import java.util.Collections;
import java.util.List;
import java.util.regex.Pattern;

import io.micrometer.tracing.SamplerFunction;
import io.micrometer.tracing.SpanCustomizer;
import io.micrometer.tracing.otel.bridge.DefaultHttpClientAttributesGetter;
import io.micrometer.tracing.otel.bridge.DefaultHttpServerAttributesExtractor;
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.OtelHttpClientHandler;
import io.micrometer.tracing.otel.bridge.OtelHttpServerHandler;
import io.micrometer.tracing.otel.bridge.OtelPropagator;
import io.micrometer.tracing.otel.bridge.OtelSpanCustomizer;
import io.micrometer.tracing.otel.bridge.OtelTracer;
Expand Down Expand Up @@ -164,20 +158,6 @@ OtelCurrentTraceContext otelCurrentTraceContext(EventPublisher publisher) {
return new OtelCurrentTraceContext();
}

@Bean
@ConditionalOnMissingBean
OtelHttpClientHandler otelHttpClientHandler(OpenTelemetry openTelemetry) {
return new OtelHttpClientHandler(openTelemetry, null, null, SamplerFunction.deferDecision(),
new DefaultHttpClientAttributesGetter());
}

@Bean
@ConditionalOnMissingBean
OtelHttpServerHandler otelHttpServerHandler(OpenTelemetry openTelemetry) {
return new OtelHttpServerHandler(openTelemetry, null, null, Pattern.compile(""),
new DefaultHttpServerAttributesExtractor());
}

@Bean
@ConditionalOnMissingBean
Slf4JEventListener otelSlf4JEventListener() {
Expand Down
64 changes: 1 addition & 63 deletions ...a/org/springframework/boot/actuate/autoconfigure/tracing/BraveAutoConfigurationTests.java
View file
Open in desktop
Expand Up @@ -25,21 +25,12 @@
import brave.baggage.BaggagePropagation;
import brave.baggage.CorrelationScopeConfig.SingleCorrelationField;
import brave.handler.SpanHandler;
import brave.http.HttpClientHandler;
import brave.http.HttpClientRequest;
import brave.http.HttpClientResponse;
import brave.http.HttpServerHandler;
import brave.http.HttpServerRequest;
import brave.http.HttpServerResponse;
import brave.http.HttpTracing;
import brave.propagation.CurrentTraceContext;
import brave.propagation.CurrentTraceContext.ScopeDecorator;
import brave.propagation.Propagation;
import brave.propagation.Propagation.Factory;
import brave.sampler.Sampler;
import io.micrometer.tracing.brave.bridge.BraveBaggageManager;
import io.micrometer.tracing.brave.bridge.BraveHttpClientHandler;
import io.micrometer.tracing.brave.bridge.BraveHttpServerHandler;
import io.micrometer.tracing.brave.bridge.BraveSpanCustomizer;
import io.micrometer.tracing.brave.bridge.BraveTracer;
import io.micrometer.tracing.brave.bridge.CompositeSpanHandler;
Expand All @@ -49,7 +40,6 @@
import io.micrometer.tracing.exporter.SpanReporter;
import org.assertj.core.api.InstanceOfAssertFactories;
import org.junit.jupiter.api.Test;
import org.mockito.Answers;

import org.springframework.boot.actuate.autoconfigure.tracing.BraveAutoConfigurationTests.SpanHandlerConfiguration.AdditionalSpanHandler;
import org.springframework.boot.autoconfigure.AutoConfigurations;
Expand Down Expand Up @@ -81,17 +71,10 @@ void shouldSupplyDefaultBeans() {
assertThat(context).hasSingleBean(CurrentTraceContext.class);
assertThat(context).hasSingleBean(Factory.class);
assertThat(context).hasSingleBean(Sampler.class);
assertThat(context).hasSingleBean(HttpTracing.class);
assertThat(context).hasSingleBean(HttpServerHandler.class);
assertThat(context).hasSingleBean(HttpClientHandler.class);
assertThat(context).hasSingleBean(BraveTracer.class);
assertThat(context).hasSingleBean(BraveHttpServerHandler.class);
assertThat(context).hasSingleBean(BraveHttpClientHandler.class);
assertThat(context).hasSingleBean(Propagation.Factory.class);
assertThat(context).hasSingleBean(BaggagePropagation.FactoryBuilder.class);
assertThat(context).hasSingleBean(BraveTracer.class);
assertThat(context).hasSingleBean(BraveHttpServerHandler.class);
assertThat(context).hasSingleBean(BraveHttpClientHandler.class);
assertThat(context).hasSingleBean(CompositeSpanHandler.class);
assertThat(context).hasSingleBean(SpanCustomizer.class);
assertThat(context).hasSingleBean(BraveSpanCustomizer.class);
Expand All @@ -111,24 +94,10 @@ void shouldBackOffOnCustomBeans() {
assertThat(context).hasSingleBean(Factory.class);
assertThat(context).hasBean("customSampler");
assertThat(context).hasSingleBean(Sampler.class);
assertThat(context).hasBean("customHttpTracing");
assertThat(context).hasSingleBean(HttpTracing.class);
assertThat(context).hasBean("customHttpServerHandler");
assertThat(context).hasSingleBean(HttpServerHandler.class);
assertThat(context).hasBean("customHttpClientHandler");
assertThat(context).hasSingleBean(HttpClientHandler.class);
assertThat(context).hasBean("customMicrometerTracer");
assertThat(context).hasSingleBean(io.micrometer.tracing.Tracer.class);
assertThat(context).hasBean("customBraveBaggageManager");
assertThat(context).hasSingleBean(BraveBaggageManager.class);
assertThat(context).hasBean("customBraveHttpServerHandler");
assertThat(context).hasSingleBean(BraveHttpServerHandler.class);
assertThat(context).hasBean("customBraveHttpClientHandler");
assertThat(context).hasSingleBean(BraveHttpClientHandler.class);
assertThat(context).hasBean("customHttpServerHandler");
assertThat(context).hasSingleBean(HttpServerHandler.class);
assertThat(context).hasBean("customHttpClientHandler");
assertThat(context).hasSingleBean(HttpClientHandler.class);
assertThat(context).hasBean("customCompositeSpanHandler");
assertThat(context).hasSingleBean(CompositeSpanHandler.class);
assertThat(context).hasBean("customSpanCustomizer");
Expand All @@ -140,11 +109,7 @@ void shouldBackOffOnCustomBeans() {

@Test
void shouldSupplyMicrometerBeans() {
this.contextRunner.run((context) -> {
assertThat(context).hasSingleBean(BraveTracer.class);
assertThat(context).hasSingleBean(BraveHttpServerHandler.class);
assertThat(context).hasSingleBean(BraveHttpClientHandler.class);
});
this.contextRunner.run((context) -> assertThat(context).hasSingleBean(BraveTracer.class));
}

@Test
Expand Down Expand Up @@ -387,23 +352,6 @@ Sampler customSampler() {
return mock(Sampler.class);
}

@Bean
HttpTracing customHttpTracing() {
return mock(HttpTracing.class);
}

@Bean
HttpServerHandler<HttpServerRequest, HttpServerResponse> customHttpServerHandler() {
HttpTracing httpTracing = mock(HttpTracing.class, Answers.RETURNS_MOCKS);
return HttpServerHandler.create(httpTracing);
}

@Bean
HttpClientHandler<HttpClientRequest, HttpClientResponse> customHttpClientHandler() {
HttpTracing httpTracing = mock(HttpTracing.class, Answers.RETURNS_MOCKS);
return HttpClientHandler.create(httpTracing);
}

@Bean
io.micrometer.tracing.Tracer customMicrometerTracer() {
return mock(io.micrometer.tracing.Tracer.class);
Expand All @@ -414,16 +362,6 @@ BraveBaggageManager customBraveBaggageManager() {
return mock(BraveBaggageManager.class);
}

@Bean
BraveHttpServerHandler customBraveHttpServerHandler() {
return mock(BraveHttpServerHandler.class);
}

@Bean
BraveHttpClientHandler customBraveHttpClientHandler() {
return mock(BraveHttpClientHandler.class);
}

@Bean
CompositeSpanHandler customCompositeSpanHandler() {
return new CompositeSpanHandler(Collections.emptyList(), Collections.emptyList(), Collections.emptyList());
Expand Down
20 changes: 0 additions & 20 deletions ...ringframework/boot/actuate/autoconfigure/tracing/OpenTelemetryAutoConfigurationTests.java
View file
Open in desktop
Expand Up @@ -21,8 +21,6 @@

import io.micrometer.tracing.SpanCustomizer;
import io.micrometer.tracing.otel.bridge.OtelCurrentTraceContext;
import io.micrometer.tracing.otel.bridge.OtelHttpClientHandler;
import io.micrometer.tracing.otel.bridge.OtelHttpServerHandler;
import io.micrometer.tracing.otel.bridge.OtelPropagator;
import io.micrometer.tracing.otel.bridge.OtelSpanCustomizer;
import io.micrometer.tracing.otel.bridge.OtelTracer;
Expand Down Expand Up @@ -68,8 +66,6 @@ void shouldSupplyBeans() {
assertThat(context).hasSingleBean(OtelTracer.class);
assertThat(context).hasSingleBean(EventPublisher.class);
assertThat(context).hasSingleBean(OtelCurrentTraceContext.class);
assertThat(context).hasSingleBean(OtelHttpClientHandler.class);
assertThat(context).hasSingleBean(OtelHttpServerHandler.class);
assertThat(context).hasSingleBean(OpenTelemetry.class);
assertThat(context).hasSingleBean(SdkTracerProvider.class);
assertThat(context).hasSingleBean(ContextPropagators.class);
Expand All @@ -91,8 +87,6 @@ void shouldNotSupplyBeansIfDependencyIsMissing(String packageName) {
assertThat(context).doesNotHaveBean(OtelTracer.class);
assertThat(context).doesNotHaveBean(EventPublisher.class);
assertThat(context).doesNotHaveBean(OtelCurrentTraceContext.class);
assertThat(context).doesNotHaveBean(OtelHttpClientHandler.class);
assertThat(context).doesNotHaveBean(OtelHttpServerHandler.class);
assertThat(context).doesNotHaveBean(OpenTelemetry.class);
assertThat(context).doesNotHaveBean(SdkTracerProvider.class);
assertThat(context).doesNotHaveBean(ContextPropagators.class);
Expand All @@ -116,10 +110,6 @@ void shouldBackOffOnCustomBeans() {
assertThat(context).hasSingleBean(EventPublisher.class);
assertThat(context).hasBean("customOtelCurrentTraceContext");
assertThat(context).hasSingleBean(OtelCurrentTraceContext.class);
assertThat(context).hasBean("customOtelHttpClientHandler");
assertThat(context).hasSingleBean(OtelHttpClientHandler.class);
assertThat(context).hasBean("customOtelHttpServerHandler");
assertThat(context).hasSingleBean(OtelHttpServerHandler.class);
assertThat(context).hasBean("customOpenTelemetry");
assertThat(context).hasSingleBean(OpenTelemetry.class);
assertThat(context).hasBean("customSdkTracerProvider");
Expand Down Expand Up @@ -221,16 +211,6 @@ OtelCurrentTraceContext customOtelCurrentTraceContext() {
return mock(OtelCurrentTraceContext.class);
}

@Bean
OtelHttpClientHandler customOtelHttpClientHandler() {
return mock(OtelHttpClientHandler.class);
}

@Bean
OtelHttpServerHandler customOtelHttpServerHandler() {
return mock(OtelHttpServerHandler.class);
}

@Bean
OpenTelemetry customOpenTelemetry() {
return mock(OpenTelemetry.class);
Expand Down