From 32ea07c1bc6da610d761f354108ca590d2169481 Mon Sep 17 00:00:00 2001 From: Jean Bisutti Date: Thu, 11 Jan 2024 22:09:17 +0100 Subject: [PATCH] Change Spring starter default otlp protocol from gRPC to http/protobuf (#10212) --- .../exporters/otlp/OtlpExporterUtil.java | 18 ++++++++- .../MetricExporterAutoConfigurationTest.java | 6 +-- .../SpanExporterAutoConfigurationTest.java | 6 +-- .../OtlpLogExporterAutoConfigurationTest.java | 32 ++++++++++++++- ...lpMetricExporterAutoConfigurationTest.java | 30 ++++++++++++-- ...OtlpSpanExporterAutoConfigurationTest.java | 40 +++++++++++++++---- 6 files changed, 112 insertions(+), 20 deletions(-) diff --git a/instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/exporters/otlp/OtlpExporterUtil.java b/instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/exporters/otlp/OtlpExporterUtil.java index 9c9b76a50b..6621c40adf 100644 --- a/instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/exporters/otlp/OtlpExporterUtil.java +++ b/instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/exporters/otlp/OtlpExporterUtil.java @@ -8,14 +8,17 @@ package io.opentelemetry.instrumentation.spring.autoconfigure.exporters.otlp; import io.opentelemetry.exporter.otlp.internal.OtlpConfigUtil; import java.time.Duration; import java.util.Map; -import java.util.Objects; import java.util.function.BiConsumer; import java.util.function.Function; import java.util.function.Supplier; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; class OtlpExporterUtil { private OtlpExporterUtil() {} + private static final Logger logger = LoggerFactory.getLogger(OtlpExporterUtil.class); + static E applySignalProperties( String dataType, OtlpExporterProperties properties, @@ -39,7 +42,18 @@ class OtlpExporterUtil { G grpcBuilder = newGrpcBuilder.get(); H httpBuilder = newHttpBuilder.get(); - boolean isHttpProtobuf = Objects.equals(protocol, OtlpConfigUtil.PROTOCOL_HTTP_PROTOBUF); + boolean isHttpProtobuf = !"grpc".equals(protocol); + + if (protocol != null + && !OtlpConfigUtil.PROTOCOL_GRPC.equals(protocol) + && !OtlpConfigUtil.PROTOCOL_HTTP_PROTOBUF.equals(protocol)) { + logger.warn( + "Unknown OTLP protocol '" + + protocol + + "', using '" + + OtlpConfigUtil.PROTOCOL_HTTP_PROTOBUF + + "'."); + } String endpoint = signalProperties.getEndpoint(); if (endpoint == null) { diff --git a/instrumentation/spring/spring-boot-autoconfigure/src/test/java/io/opentelemetry/instrumentation/spring/autoconfigure/exporters/MetricExporterAutoConfigurationTest.java b/instrumentation/spring/spring-boot-autoconfigure/src/test/java/io/opentelemetry/instrumentation/spring/autoconfigure/exporters/MetricExporterAutoConfigurationTest.java index 7b65c594cb..fb53294240 100644 --- a/instrumentation/spring/spring-boot-autoconfigure/src/test/java/io/opentelemetry/instrumentation/spring/autoconfigure/exporters/MetricExporterAutoConfigurationTest.java +++ b/instrumentation/spring/spring-boot-autoconfigure/src/test/java/io/opentelemetry/instrumentation/spring/autoconfigure/exporters/MetricExporterAutoConfigurationTest.java @@ -8,7 +8,7 @@ package io.opentelemetry.instrumentation.spring.autoconfigure.exporters; import static org.assertj.core.api.Assertions.assertThat; import io.opentelemetry.exporter.logging.LoggingMetricExporter; -import io.opentelemetry.exporter.otlp.metrics.OtlpGrpcMetricExporter; +import io.opentelemetry.exporter.otlp.http.metrics.OtlpHttpMetricExporter; import io.opentelemetry.instrumentation.spring.autoconfigure.exporters.logging.LoggingMetricExporterAutoConfiguration; import io.opentelemetry.instrumentation.spring.autoconfigure.exporters.otlp.OtlpMetricExporterAutoConfiguration; import org.junit.jupiter.api.Test; @@ -28,7 +28,7 @@ class MetricExporterAutoConfigurationTest { void defaultConfiguration() { contextRunner.run( context -> { - assertThat(context.getBean("otelOtlpMetricExporter", OtlpGrpcMetricExporter.class)) + assertThat(context.getBean("otelOtlpMetricExporter", OtlpHttpMetricExporter.class)) .as("OTLP exporter is enabled by default") .isNotNull(); assertThat(context.containsBean("otelLoggingMetricExporter")) @@ -43,7 +43,7 @@ class MetricExporterAutoConfigurationTest { .withPropertyValues("otel.exporter.logging.enabled=true") .run( context -> { - assertThat(context.getBean("otelOtlpMetricExporter", OtlpGrpcMetricExporter.class)) + assertThat(context.getBean("otelOtlpMetricExporter", OtlpHttpMetricExporter.class)) .as("OTLP exporter is present even with logging enabled") .isNotNull(); assertThat(context.getBean("otelLoggingMetricExporter", LoggingMetricExporter.class)) diff --git a/instrumentation/spring/spring-boot-autoconfigure/src/test/java/io/opentelemetry/instrumentation/spring/autoconfigure/exporters/SpanExporterAutoConfigurationTest.java b/instrumentation/spring/spring-boot-autoconfigure/src/test/java/io/opentelemetry/instrumentation/spring/autoconfigure/exporters/SpanExporterAutoConfigurationTest.java index 5730583646..558fe14708 100644 --- a/instrumentation/spring/spring-boot-autoconfigure/src/test/java/io/opentelemetry/instrumentation/spring/autoconfigure/exporters/SpanExporterAutoConfigurationTest.java +++ b/instrumentation/spring/spring-boot-autoconfigure/src/test/java/io/opentelemetry/instrumentation/spring/autoconfigure/exporters/SpanExporterAutoConfigurationTest.java @@ -8,7 +8,7 @@ package io.opentelemetry.instrumentation.spring.autoconfigure.exporters; import static org.assertj.core.api.Assertions.assertThat; import io.opentelemetry.exporter.logging.LoggingSpanExporter; -import io.opentelemetry.exporter.otlp.trace.OtlpGrpcSpanExporter; +import io.opentelemetry.exporter.otlp.http.trace.OtlpHttpSpanExporter; import io.opentelemetry.instrumentation.spring.autoconfigure.exporters.logging.LoggingSpanExporterAutoConfiguration; import io.opentelemetry.instrumentation.spring.autoconfigure.exporters.otlp.OtlpSpanExporterAutoConfiguration; import org.junit.jupiter.api.Test; @@ -28,7 +28,7 @@ class SpanExporterAutoConfigurationTest { void defaultConfiguration() { contextRunner.run( context -> { - assertThat(context.getBean("otelOtlpSpanExporter", OtlpGrpcSpanExporter.class)) + assertThat(context.getBean("otelOtlpSpanExporter", OtlpHttpSpanExporter.class)) .as("OTLP exporter is enabled by default") .isNotNull(); assertThat(context.containsBean("otelLoggingSpanExporter")) @@ -43,7 +43,7 @@ class SpanExporterAutoConfigurationTest { .withPropertyValues("otel.exporter.logging.enabled=true") .run( context -> { - assertThat(context.getBean("otelOtlpSpanExporter", OtlpGrpcSpanExporter.class)) + assertThat(context.getBean("otelOtlpSpanExporter", OtlpHttpSpanExporter.class)) .as("OTLP exporter is present even with logging enabled") .isNotNull(); assertThat(context.getBean("otelLoggingSpanExporter", LoggingSpanExporter.class)) diff --git a/instrumentation/spring/spring-boot-autoconfigure/src/test/java/io/opentelemetry/instrumentation/spring/autoconfigure/exporters/otlp/OtlpLogExporterAutoConfigurationTest.java b/instrumentation/spring/spring-boot-autoconfigure/src/test/java/io/opentelemetry/instrumentation/spring/autoconfigure/exporters/otlp/OtlpLogExporterAutoConfigurationTest.java index e2aa15e85c..519c3cff16 100644 --- a/instrumentation/spring/spring-boot-autoconfigure/src/test/java/io/opentelemetry/instrumentation/spring/autoconfigure/exporters/otlp/OtlpLogExporterAutoConfigurationTest.java +++ b/instrumentation/spring/spring-boot-autoconfigure/src/test/java/io/opentelemetry/instrumentation/spring/autoconfigure/exporters/otlp/OtlpLogExporterAutoConfigurationTest.java @@ -7,9 +7,11 @@ package io.opentelemetry.instrumentation.spring.autoconfigure.exporters.otlp; import static org.assertj.core.api.Assertions.assertThat; +import io.opentelemetry.exporter.otlp.http.logs.OtlpHttpLogRecordExporter; import io.opentelemetry.exporter.otlp.logs.OtlpGrpcLogRecordExporter; import io.opentelemetry.instrumentation.spring.autoconfigure.OpenTelemetryAutoConfiguration; import io.opentelemetry.sdk.logs.export.LogRecordExporter; +import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; import org.springframework.boot.autoconfigure.AutoConfigurations; import org.springframework.boot.test.context.runner.ApplicationContextRunner; @@ -64,11 +66,37 @@ class OtlpLogExporterAutoConfigurationTest { } @Test - void loggerPresentByDefault() { + void otlpHttpUsedByDefault() { runner.run( context -> assertThat( - context.getBean("otelOtlpLogRecordExporter", OtlpGrpcLogRecordExporter.class)) + context.getBean("otelOtlpLogRecordExporter", OtlpHttpLogRecordExporter.class)) .isNotNull()); } + + @Test + @DisplayName("use grpc when protocol set") + void useGrpc() { + runner + .withPropertyValues("otel.exporter.otlp.protocol=grpc") + .run( + context -> + assertThat( + context.getBean( + "otelOtlpLogRecordExporter", OtlpGrpcLogRecordExporter.class)) + .isNotNull()); + } + + @Test + @DisplayName("use http when unknown protocol set") + void useHttpWhenAnUnknownProtocolIsSet() { + runner + .withPropertyValues("otel.exporter.otlp.protocol=unknown") + .run( + context -> + assertThat( + context.getBean( + "otelOtlpLogRecordExporter", OtlpHttpLogRecordExporter.class)) + .isNotNull()); + } } diff --git a/instrumentation/spring/spring-boot-autoconfigure/src/test/java/io/opentelemetry/instrumentation/spring/autoconfigure/exporters/otlp/OtlpMetricExporterAutoConfigurationTest.java b/instrumentation/spring/spring-boot-autoconfigure/src/test/java/io/opentelemetry/instrumentation/spring/autoconfigure/exporters/otlp/OtlpMetricExporterAutoConfigurationTest.java index 87f3e0bee4..106ccd4a49 100644 --- a/instrumentation/spring/spring-boot-autoconfigure/src/test/java/io/opentelemetry/instrumentation/spring/autoconfigure/exporters/otlp/OtlpMetricExporterAutoConfigurationTest.java +++ b/instrumentation/spring/spring-boot-autoconfigure/src/test/java/io/opentelemetry/instrumentation/spring/autoconfigure/exporters/otlp/OtlpMetricExporterAutoConfigurationTest.java @@ -7,8 +7,10 @@ package io.opentelemetry.instrumentation.spring.autoconfigure.exporters.otlp; import static org.assertj.core.api.Assertions.assertThat; +import io.opentelemetry.exporter.otlp.http.metrics.OtlpHttpMetricExporter; import io.opentelemetry.exporter.otlp.metrics.OtlpGrpcMetricExporter; import io.opentelemetry.instrumentation.spring.autoconfigure.OpenTelemetryAutoConfiguration; +import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; import org.springframework.boot.autoconfigure.AutoConfigurations; import org.springframework.boot.test.context.runner.ApplicationContextRunner; @@ -25,19 +27,41 @@ class OtlpMetricExporterAutoConfigurationTest { void otlpEnabled() { runner .withPropertyValues("otel.exporter.otlp.enabled=true") + .run( + context -> + assertThat(context.getBean("otelOtlpMetricExporter", OtlpHttpMetricExporter.class)) + .isNotNull()); + } + + @Test + @DisplayName("use grpc when protocol set") + void useGrpc() { + runner + .withPropertyValues("otel.exporter.otlp.protocol=grpc") .run( context -> assertThat(context.getBean("otelOtlpMetricExporter", OtlpGrpcMetricExporter.class)) .isNotNull()); } + @Test + @DisplayName("use http when unknown protocol set") + void useHttpWhenAnUnknownProtocolIsSet() { + runner + .withPropertyValues("otel.exporter.otlp.protocol=unknown") + .run( + context -> + assertThat(context.getBean("otelOtlpMetricExporter", OtlpHttpMetricExporter.class)) + .isNotNull()); + } + @Test void otlpMetricsEnabled() { runner .withPropertyValues("otel.exporter.otlp.metrics.enabled=true") .run( context -> - assertThat(context.getBean("otelOtlpMetricExporter", OtlpGrpcMetricExporter.class)) + assertThat(context.getBean("otelOtlpMetricExporter", OtlpHttpMetricExporter.class)) .isNotNull()); } @@ -63,10 +87,10 @@ class OtlpMetricExporterAutoConfigurationTest { } @Test - void exporterPresentByDefault() { + void otlpHttpUsedByDefault() { runner.run( context -> - assertThat(context.getBean("otelOtlpMetricExporter", OtlpGrpcMetricExporter.class)) + assertThat(context.getBean("otelOtlpMetricExporter", OtlpHttpMetricExporter.class)) .isNotNull()); } } diff --git a/instrumentation/spring/spring-boot-autoconfigure/src/test/java/io/opentelemetry/instrumentation/spring/autoconfigure/exporters/otlp/OtlpSpanExporterAutoConfigurationTest.java b/instrumentation/spring/spring-boot-autoconfigure/src/test/java/io/opentelemetry/instrumentation/spring/autoconfigure/exporters/otlp/OtlpSpanExporterAutoConfigurationTest.java index c580f994a8..54449e437c 100644 --- a/instrumentation/spring/spring-boot-autoconfigure/src/test/java/io/opentelemetry/instrumentation/spring/autoconfigure/exporters/otlp/OtlpSpanExporterAutoConfigurationTest.java +++ b/instrumentation/spring/spring-boot-autoconfigure/src/test/java/io/opentelemetry/instrumentation/spring/autoconfigure/exporters/otlp/OtlpSpanExporterAutoConfigurationTest.java @@ -8,6 +8,7 @@ package io.opentelemetry.instrumentation.spring.autoconfigure.exporters.otlp; import static org.assertj.core.api.Assertions.assertThat; import io.opentelemetry.exporter.logging.LoggingSpanExporter; +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.instrumentation.spring.autoconfigure.MapConverterTestAutoConfiguration; @@ -31,17 +32,16 @@ class OtlpSpanExporterAutoConfigurationTest { AutoConfigurations.of( OpenTelemetryAutoConfiguration.class, OtlpSpanExporterAutoConfiguration.class, - MapConverterTestAutoConfiguration.class)) - .withBean(OtlpHttpSpanExporterBuilder.class, () -> otlpHttpSpanExporterBuilder); + MapConverterTestAutoConfiguration.class)); @Test - @DisplayName("when exporters are ENABLED should initialize OtlpGrpcSpanExporter bean") + @DisplayName("when exporters are ENABLED should initialize OtlpHttpSpanExporter bean") void otlpEnabled() { this.contextRunner .withPropertyValues("otel.exporter.otlp.enabled=true") .run( context -> - assertThat(context.getBean("otelOtlpSpanExporter", OtlpGrpcSpanExporter.class)) + assertThat(context.getBean("otelOtlpSpanExporter", OtlpHttpSpanExporter.class)) .isNotNull()); Mockito.verifyNoMoreInteractions(otlpHttpSpanExporterBuilder); @@ -53,7 +53,7 @@ class OtlpSpanExporterAutoConfigurationTest { .withPropertyValues("otel.exporter.otlp.traces.enabled=true") .run( context -> - assertThat(context.getBean("otelOtlpSpanExporter", OtlpGrpcSpanExporter.class)) + assertThat(context.getBean("otelOtlpSpanExporter", OtlpHttpSpanExporter.class)) .isNotNull()); } @@ -80,11 +80,11 @@ class OtlpSpanExporterAutoConfigurationTest { } @Test - @DisplayName("when otlp enabled property is MISSING should initialize OtlpGrpcSpanExporter bean") + @DisplayName("when otlp enabled property is MISSING should initialize OtlpHttpSpanExporter bean") void exporterPresentByDefault() { this.contextRunner.run( context -> - assertThat(context.getBean("otelOtlpSpanExporter", OtlpGrpcSpanExporter.class)) + assertThat(context.getBean("otelOtlpSpanExporter", OtlpHttpSpanExporter.class)) .isNotNull()); } @@ -92,6 +92,7 @@ class OtlpSpanExporterAutoConfigurationTest { @DisplayName("use http/protobuf when protocol set") void useHttp() { this.contextRunner + .withBean(OtlpHttpSpanExporterBuilder.class, () -> otlpHttpSpanExporterBuilder) .withPropertyValues( "otel.exporter.otlp.enabled=true", "otel.exporter.otlp.protocol=http/protobuf", @@ -113,6 +114,7 @@ class OtlpSpanExporterAutoConfigurationTest { @DisplayName("use http/protobuf with environment variables for headers using the MapConverter") void useHttpWithEnv() { this.contextRunner + .withBean(OtlpHttpSpanExporterBuilder.class, () -> otlpHttpSpanExporterBuilder) .withPropertyValues( "otel.exporter.otlp.enabled=true", "otel.exporter.otlp.protocol=http/protobuf") // are similar to environment variables in that they use the same converters @@ -125,6 +127,30 @@ class OtlpSpanExporterAutoConfigurationTest { Mockito.verifyNoMoreInteractions(otlpHttpSpanExporterBuilder); } + @Test + @DisplayName("use grpc when protocol set") + void useGrpc() { + this.contextRunner + .withPropertyValues("otel.exporter.otlp.protocol=grpc") + .run( + context -> + assertThat(context.getBean(OtlpGrpcSpanExporter.class)) + .as("Should contain the gRPC span exporter when grpc is set") + .isNotNull()); + } + + @Test + @DisplayName("use http when unknown protocol set") + void useHttpWhenAnUnknownProtocolIsSet() { + this.contextRunner + .withPropertyValues("otel.exporter.otlp.protocol=unknown") + .run( + context -> + assertThat(context.getBean(OtlpHttpSpanExporter.class)) + .as("Should contain the http span exporter when an unknown is set") + .isNotNull()); + } + @Test @DisplayName("logging exporter can still be configured") void loggingExporter() {