From 653443adc1e42a6ab32a88b886cf63f56c4db3fc Mon Sep 17 00:00:00 2001 From: Andy Wilkinson Date: Thu, 11 Jul 2024 18:01:12 +0100 Subject: [PATCH] Revert "Merge pull request #41213 from timpeeters" This reverts commit b4017f0ef44fc624d6ae4610ffb5538b1d3a12c5, reversing changes made to 156237227ca515576dafcf1016c8f5c3b8f915af. See gh-41213 --- .../build.gradle | 1 - .../tracing/otlp/OtlpAutoConfiguration.java | 6 +- .../tracing/otlp/OtlpProperties.java | 29 +---- .../otlp/OtlpTracingConfigurations.java | 25 +---- ...itional-spring-configuration-metadata.json | 4 - ...OtlpAutoConfigurationIntegrationTests.java | 105 +----------------- .../otlp/OtlpAutoConfigurationTests.java | 15 --- 7 files changed, 10 insertions(+), 175 deletions(-) diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/build.gradle b/spring-boot-project/spring-boot-actuator-autoconfigure/build.gradle index 533adfbb10..3b4e685872 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/build.gradle +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/build.gradle @@ -161,7 +161,6 @@ dependencies { testImplementation("org.awaitility:awaitility") testImplementation("org.cache2k:cache2k-api") testImplementation("org.eclipse.jetty.ee10:jetty-ee10-webapp") - testImplementation("org.eclipse.jetty.http2:jetty-http2-server") testImplementation("org.glassfish.jersey.ext:jersey-spring6") testImplementation("org.glassfish.jersey.media:jersey-media-json-jackson") testImplementation("org.hamcrest:hamcrest") diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/tracing/otlp/OtlpAutoConfiguration.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/tracing/otlp/OtlpAutoConfiguration.java index 9d797721fb..abb3253f2f 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/tracing/otlp/OtlpAutoConfiguration.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/tracing/otlp/OtlpAutoConfiguration.java @@ -36,10 +36,8 @@ import org.springframework.context.annotation.Import; * the future, see: opentelemetry-java#3651. * Because this class configures components from the OTel SDK, it can't support HTTP/JSON. - * By default, we auto-configure HTTP/protobuf. If you want to use gRPC, you need to set - * {@code management.otlp.tracing.transport=grpc}. If you define a - * {@link OtlpHttpSpanExporter} or {@link OtlpGrpcSpanExporter}, this auto-configuration - * will back off. + * To keep things simple, we only auto-configure HTTP/protobuf. If you want to use gRPC, + * define an {@link OtlpGrpcSpanExporter} and this auto-configuration will back off. * * @author Jonatan Ivanov * @author Moritz Halbritter diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/tracing/otlp/OtlpProperties.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/tracing/otlp/OtlpProperties.java index bf93d58dd1..371de84911 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/tracing/otlp/OtlpProperties.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/tracing/otlp/OtlpProperties.java @@ -44,11 +44,6 @@ public class OtlpProperties { */ private Duration timeout = Duration.ofSeconds(10); - /** - * Transport used to send the spans. - */ - private Transport transport = Transport.HTTP; - /** * Method used to compress the payload. */ @@ -75,14 +70,6 @@ public class OtlpProperties { this.timeout = timeout; } - public Transport getTransport() { - return this.transport; - } - - public void setTransport(Transport transport) { - this.transport = transport; - } - public Compression getCompression() { return this.compression; } @@ -99,21 +86,7 @@ public class OtlpProperties { this.headers = headers; } - public enum Transport { - - /** - * HTTP transport. - */ - HTTP, - - /** - * gRPC transport. - */ - GRPC - - } - - public enum Compression { + enum Compression { /** * Gzip compression. diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/tracing/otlp/OtlpTracingConfigurations.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/tracing/otlp/OtlpTracingConfigurations.java index f222fb249d..1426967171 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/tracing/otlp/OtlpTracingConfigurations.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/tracing/otlp/OtlpTracingConfigurations.java @@ -20,8 +20,6 @@ import java.util.Map.Entry; import io.opentelemetry.exporter.otlp.http.trace.OtlpHttpSpanExporter; import io.opentelemetry.exporter.otlp.http.trace.OtlpHttpSpanExporterBuilder; -import io.opentelemetry.exporter.otlp.trace.OtlpGrpcSpanExporter; -import io.opentelemetry.exporter.otlp.trace.OtlpGrpcSpanExporterBuilder; import org.springframework.boot.actuate.autoconfigure.tracing.ConditionalOnEnabledTracing; import org.springframework.boot.autoconfigure.condition.ConditionalOnBean; @@ -68,14 +66,13 @@ class OtlpTracingConfigurations { } @Configuration(proxyBeanMethods = false) - @ConditionalOnMissingBean({ OtlpGrpcSpanExporter.class, OtlpHttpSpanExporter.class }) - @ConditionalOnBean(OtlpTracingConnectionDetails.class) - @ConditionalOnEnabledTracing("otlp") static class Exporters { @Bean - @ConditionalOnProperty(prefix = "management.otlp.tracing", name = "transport", havingValue = "http", - matchIfMissing = true) + @ConditionalOnMissingBean(value = OtlpHttpSpanExporter.class, + type = "io.opentelemetry.exporter.otlp.trace.OtlpGrpcSpanExporter") + @ConditionalOnBean(OtlpTracingConnectionDetails.class) + @ConditionalOnEnabledTracing("otlp") OtlpHttpSpanExporter otlpHttpSpanExporter(OtlpProperties properties, OtlpTracingConnectionDetails connectionDetails) { OtlpHttpSpanExporterBuilder builder = OtlpHttpSpanExporter.builder() @@ -88,20 +85,6 @@ class OtlpTracingConfigurations { return builder.build(); } - @Bean - @ConditionalOnProperty(prefix = "management.otlp.tracing", name = "transport", havingValue = "grpc") - OtlpGrpcSpanExporter otlpGrpcSpanExporter(OtlpProperties properties, - OtlpTracingConnectionDetails connectionDetails) { - OtlpGrpcSpanExporterBuilder builder = OtlpGrpcSpanExporter.builder() - .setEndpoint(connectionDetails.getUrl()) - .setTimeout(properties.getTimeout()) - .setCompression(properties.getCompression().name().toLowerCase()); - for (Entry header : properties.getHeaders().entrySet()) { - builder.addHeader(header.getKey(), header.getValue()); - } - return builder.build(); - } - } } diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/resources/META-INF/additional-spring-configuration-metadata.json b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/resources/META-INF/additional-spring-configuration-metadata.json index fb5bf2716d..c69880ebfd 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/resources/META-INF/additional-spring-configuration-metadata.json +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/resources/META-INF/additional-spring-configuration-metadata.json @@ -2102,10 +2102,6 @@ "type": "java.lang.Boolean", "description": "Whether auto-configuration of tracing is enabled to export OTLP traces." }, - { - "name": "management.otlp.tracing.transport", - "defaultValue": "http" - }, { "name": "management.prometheus.metrics.export.histogram-flavor", "defaultValue": "prometheus" diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/tracing/otlp/OtlpAutoConfigurationIntegrationTests.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/tracing/otlp/OtlpAutoConfigurationIntegrationTests.java index 9882e837ef..0fcc77811f 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/tracing/otlp/OtlpAutoConfigurationIntegrationTests.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/tracing/otlp/OtlpAutoConfigurationIntegrationTests.java @@ -16,15 +16,12 @@ package org.springframework.boot.actuate.autoconfigure.tracing.otlp; -import java.io.InputStream; +import java.io.IOException; import java.nio.charset.StandardCharsets; -import java.util.concurrent.BlockingQueue; -import java.util.concurrent.LinkedBlockingQueue; import java.util.concurrent.TimeUnit; import io.micrometer.tracing.Tracer; import io.opentelemetry.exporter.otlp.http.trace.OtlpHttpSpanExporter; -import io.opentelemetry.exporter.otlp.trace.OtlpGrpcSpanExporter; import io.opentelemetry.sdk.common.CompletableResultCode; import io.opentelemetry.sdk.trace.export.SpanExporter; import okhttp3.mockwebserver.MockResponse; @@ -32,16 +29,6 @@ import okhttp3.mockwebserver.MockWebServer; import okhttp3.mockwebserver.RecordedRequest; import okio.Buffer; import okio.GzipSource; -import org.eclipse.jetty.http.HttpFields; -import org.eclipse.jetty.http2.server.HTTP2CServerConnectionFactory; -import org.eclipse.jetty.io.Content; -import org.eclipse.jetty.server.Handler; -import org.eclipse.jetty.server.HttpConfiguration; -import org.eclipse.jetty.server.Request; -import org.eclipse.jetty.server.Response; -import org.eclipse.jetty.server.Server; -import org.eclipse.jetty.server.ServerConnector; -import org.eclipse.jetty.util.Callback; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -49,7 +36,6 @@ import org.junit.jupiter.api.Test; import org.springframework.boot.actuate.autoconfigure.observation.ObservationAutoConfiguration; import org.springframework.boot.actuate.autoconfigure.opentelemetry.OpenTelemetryAutoConfiguration; import org.springframework.boot.actuate.autoconfigure.tracing.MicrometerTracingAutoConfiguration; -import org.springframework.boot.actuate.autoconfigure.tracing.otlp.OtlpAutoConfigurationIntegrationTests.MockGrpcServer.RecordedGrpcRequest; import org.springframework.boot.autoconfigure.AutoConfigurations; import org.springframework.boot.test.context.runner.ApplicationContextRunner; @@ -71,18 +57,14 @@ class OtlpAutoConfigurationIntegrationTests { private final MockWebServer mockWebServer = new MockWebServer(); - private final MockGrpcServer mockGrpcServer = new MockGrpcServer(); - @BeforeEach - void startServers() throws Exception { + void setUp() throws IOException { this.mockWebServer.start(); - this.mockGrpcServer.start(); } @AfterEach - void stopServers() throws Exception { + void tearDown() throws IOException { this.mockWebServer.close(); - this.mockGrpcServer.close(); } @Test @@ -131,85 +113,4 @@ class OtlpAutoConfigurationIntegrationTests { }); } - @Test - void grpcSpanExporterShouldExportSpans() { - this.contextRunner - .withPropertyValues( - "management.otlp.tracing.endpoint=http://localhost:%d".formatted(this.mockGrpcServer.getPort()), - "management.otlp.tracing.headers.custom=42", "management.otlp.tracing.transport=grpc") - .run((context) -> { - context.getBean(Tracer.class).nextSpan().name("test").end(); - assertThat(context.getBean(OtlpGrpcSpanExporter.class).flush()) - .isSameAs(CompletableResultCode.ofSuccess()); - RecordedGrpcRequest request = this.mockGrpcServer.takeRequest(10, TimeUnit.SECONDS); - assertThat(request).isNotNull(); - assertThat(request.headers().get("Content-Type")).isEqualTo("application/grpc"); - assertThat(request.headers().get("custom")).isEqualTo("42"); - assertThat(request.bodyAsString()).contains("org.springframework.boot"); - }); - } - - static class MockGrpcServer { - - private final Server server = createServer(); - - private final BlockingQueue recordedRequests = new LinkedBlockingQueue<>(); - - void start() throws Exception { - this.server.start(); - } - - void close() throws Exception { - this.server.stop(); - } - - int getPort() { - return this.server.getURI().getPort(); - } - - RecordedGrpcRequest takeRequest(int timeout, TimeUnit unit) throws InterruptedException { - return this.recordedRequests.poll(timeout, unit); - } - - void recordRequest(RecordedGrpcRequest request) { - this.recordedRequests.add(request); - } - - private Server createServer() { - Server server = new Server(); - server.addConnector(createConnector(server)); - server.setHandler(new GrpcHandler()); - return server; - } - - private ServerConnector createConnector(Server server) { - ServerConnector connector = new ServerConnector(server, - new HTTP2CServerConnectionFactory(new HttpConfiguration())); - connector.setPort(0); - return connector; - } - - class GrpcHandler extends Handler.Abstract { - - @Override - public boolean handle(Request request, Response response, Callback callback) throws Exception { - try (InputStream in = Content.Source.asInputStream(request)) { - recordRequest(new RecordedGrpcRequest(request.getHeaders(), in.readAllBytes())); - } - response.getHeaders().add("Content-Type", "application/grpc"); - response.getHeaders().add("Grpc-Status", "0"); - callback.succeeded(); - return true; - } - - } - - record RecordedGrpcRequest(HttpFields headers, byte[] body) { - String bodyAsString() { - return new String(this.body, StandardCharsets.UTF_8); - } - } - - } - } diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/tracing/otlp/OtlpAutoConfigurationTests.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/tracing/otlp/OtlpAutoConfigurationTests.java index 2f9259c6db..eba8fe1125 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/tracing/otlp/OtlpAutoConfigurationTests.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/tracing/otlp/OtlpAutoConfigurationTests.java @@ -51,12 +51,6 @@ class OtlpAutoConfigurationTests { this.contextRunner.run((context) -> assertThat(context).doesNotHaveBean(OtlpHttpSpanExporter.class)); } - @Test - void shouldNotSupplyBeansIfGrpcTransportIsEnabledButPropertyIsNotSet() { - this.contextRunner.withPropertyValues("management.otlp.tracing.transport=grpc") - .run((context) -> assertThat(context).doesNotHaveBean(OtlpGrpcSpanExporter.class)); - } - @Test void shouldSupplyBeans() { this.contextRunner.withPropertyValues("management.otlp.tracing.endpoint=http://localhost:4318/v1/traces") @@ -64,15 +58,6 @@ class OtlpAutoConfigurationTests { .hasSingleBean(SpanExporter.class)); } - @Test - void shouldSupplyBeansIfGrpcTransportIsEnabled() { - this.contextRunner - .withPropertyValues("management.otlp.tracing.endpoint=http://localhost:4317/v1/traces", - "management.otlp.tracing.transport=grpc") - .run((context) -> assertThat(context).hasSingleBean(OtlpGrpcSpanExporter.class) - .hasSingleBean(SpanExporter.class)); - } - @Test void shouldNotSupplyBeansIfGlobalTracingIsDisabled() { this.contextRunner.withPropertyValues("management.tracing.enabled=false")