Skip to content

Change Spring starter default otlp protocol from gRPC to http/protobuf #10212

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

Merged
merged 3 commits into from
Jan 11, 2024
Merged

Change Spring starter default otlp protocol from gRPC to http/protobuf #10212

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b8e44d1
Change Spring starter default otlp protocol from gRPC to http/protobuf
jeanbisutti Jan 11, 2024
379cb29
Use gRPC constant
jeanbisutti Jan 11, 2024
ff0d20a
Improve warning message
jeanbisutti Jan 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 15 additions & 2 deletions ...o/opentelemetry/instrumentation/spring/autoconfigure/exporters/otlp/OtlpExporterUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,17 @@
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 <G, H, E> E applySignalProperties(
String dataType,
OtlpExporterProperties properties,
Expand All @@ -39,7 +42,17 @@ static <G, H, E> E applySignalProperties(
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
Copy link
Contributor

@laurit laurit Jan 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sdk chooses to fail when invalid protocol is specified, see https://github.com/open-telemetry/opentelemetry-java/blob/d9f9812d4375a4229caff43bd681c50b7a45776a/exporters/otlp/all/src/main/java/io/opentelemetry/exporter/otlp/internal/OtlpLogRecordExporterProvider.java#L75

Copy link
Member Author

@jeanbisutti jeanbisutti Jan 11, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps it could be rediscussed later with the perspective of what to do with the OTel starter configurations.

Copy link
Member Author

@jeanbisutti jeanbisutti Jan 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have created #10213

&& !"grpc".equals(protocol)
&& !OtlpConfigUtil.PROTOCOL_HTTP_PROTOBUF.equals(protocol)) {
logger.warn(
protocol
+ " protocol is not managed. "
Copy link
Contributor

@laurit laurit Jan 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd rephrase this to something like Unknown OTLP protocol '{}', using 'http/protobuf' instead.

Copy link
Member Author

@jeanbisutti jeanbisutti Jan 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

+ OtlpConfigUtil.PROTOCOL_HTTP_PROTOBUF
+ " will be used.");
}

String endpoint = signalProperties.getEndpoint();
if (endpoint == null) {
Expand Down
6 changes: 3 additions & 3 deletions ...y/instrumentation/spring/autoconfigure/exporters/MetricExporterAutoConfigurationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
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;
Expand All @@ -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"))
Expand All @@ -43,7 +43,7 @@ void loggingEnabledByConfiguration() {
.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))
Expand Down
6 changes: 3 additions & 3 deletions ...try/instrumentation/spring/autoconfigure/exporters/SpanExporterAutoConfigurationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
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;
Expand All @@ -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"))
Expand All @@ -43,7 +43,7 @@ void loggingEnabledByConfiguration() {
.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))
Expand Down
32 changes: 30 additions & 2 deletions ...rumentation/spring/autoconfigure/exporters/otlp/OtlpLogExporterAutoConfigurationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,11 @@

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;
Expand Down Expand Up @@ -64,11 +66,37 @@ void otlpLogsDisabled() {
}

@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());
}
}
30 changes: 27 additions & 3 deletions ...entation/spring/autoconfigure/exporters/otlp/OtlpMetricExporterAutoConfigurationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,10 @@

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;
Expand All @@ -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());
}

Expand All @@ -63,10 +87,10 @@ void otlpMetricsDisabled() {
}

@Test
void exporterPresentByDefault() {
void otlpHttpUsedByDefault() {
runner.run(
context ->
assertThat(context.getBean("otelOtlpMetricExporter", OtlpGrpcMetricExporter.class))
assertThat(context.getBean("otelOtlpMetricExporter", OtlpHttpMetricExporter.class))
.isNotNull());
}
}
40 changes: 33 additions & 7 deletions ...umentation/spring/autoconfigure/exporters/otlp/OtlpSpanExporterAutoConfigurationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
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;
Expand All @@ -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);
Expand All @@ -53,7 +53,7 @@ void otlpTracesEnabled() {
.withPropertyValues("otel.exporter.otlp.traces.enabled=true")
.run(
context ->
assertThat(context.getBean("otelOtlpSpanExporter", OtlpGrpcSpanExporter.class))
assertThat(context.getBean("otelOtlpSpanExporter", OtlpHttpSpanExporter.class))
.isNotNull());
}

Expand All @@ -80,18 +80,19 @@ void otlpTracesDisabled() {
}

@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());
}

@Test
@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",
Expand All @@ -113,6 +114,7 @@ void useHttp() {
@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
Expand All @@ -125,6 +127,30 @@ void useHttpWithEnv() {
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() {
Expand Down