Merge pull request #41333 from eddumelendez

* pr/41333: Polish "Support Otlp Tracing's GRPC port from service connections" Support Otlp Tracing's GRPC port from service connections Closes gh-41333
spring-projects · Sep 6, 2024 · 8835593 · 8835593
2 parents f6505f7 + bac3303
commit 8835593
Show file tree

Hide file tree

Showing 11 changed files with 79 additions and 21 deletions.
diff --git a/...rg/springframework/boot/actuate/autoconfigure/tracing/otlp/OtlpTracingConfigurations.java b/...rg/springframework/boot/actuate/autoconfigure/tracing/otlp/OtlpTracingConfigurations.java
@@ -23,17 +23,20 @@
 import io.opentelemetry.exporter.otlp.trace.OtlpGrpcSpanExporter;
 import io.opentelemetry.exporter.otlp.trace.OtlpGrpcSpanExporterBuilder;
 
+import org.springframework.boot.actuate.autoconfigure.opentelemetry.otlp.Transport;
 import org.springframework.boot.actuate.autoconfigure.tracing.ConditionalOnEnabledTracing;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnBean;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
+import org.springframework.util.Assert;
 
 /**
  * Configurations imported by {@link OtlpAutoConfiguration}.
  *
  * @author Moritz Halbritter
+ * @author Eddú Meléndez
  */
 class OtlpTracingConfigurations {
 
@@ -59,7 +62,10 @@ static class PropertiesOtlpTracingConnectionDetails implements OtlpTracingConnec
 			}
 
 			@Override
-			public String getUrl() {
+			public String getUrl(Transport transport) {
+				Assert.state(transport == this.properties.getTransport(),
+						"Requested transport %s doesn't match configured transport %s".formatted(transport,
+								this.properties.getTransport()));
 				return this.properties.getEndpoint();
 			}
 
@@ -79,7 +85,7 @@ static class Exporters {
 		OtlpHttpSpanExporter otlpHttpSpanExporter(OtlpProperties properties,
 				OtlpTracingConnectionDetails connectionDetails) {
 			OtlpHttpSpanExporterBuilder builder = OtlpHttpSpanExporter.builder()
-				.setEndpoint(connectionDetails.getUrl())
+				.setEndpoint(connectionDetails.getUrl(Transport.HTTP))
 				.setTimeout(properties.getTimeout())
 				.setCompression(properties.getCompression().name().toLowerCase());
 			for (Entry<String, String> header : properties.getHeaders().entrySet()) {
@@ -93,7 +99,7 @@ OtlpHttpSpanExporter otlpHttpSpanExporter(OtlpProperties properties,
 		OtlpGrpcSpanExporter otlpGrpcSpanExporter(OtlpProperties properties,
 				OtlpTracingConnectionDetails connectionDetails) {
 			OtlpGrpcSpanExporterBuilder builder = OtlpGrpcSpanExporter.builder()
-				.setEndpoint(connectionDetails.getUrl())
+				.setEndpoint(connectionDetails.getUrl(Transport.GRPC))
 				.setTimeout(properties.getTimeout())
 				.setCompression(properties.getCompression().name().toLowerCase());
 			for (Entry<String, String> header : properties.getHeaders().entrySet()) {

diff --git a/...springframework/boot/actuate/autoconfigure/tracing/otlp/OtlpTracingConnectionDetails.java b/...springframework/boot/actuate/autoconfigure/tracing/otlp/OtlpTracingConnectionDetails.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2012-2023 the original author or authors.
+ * 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.
@@ -16,20 +16,34 @@
 
 package org.springframework.boot.actuate.autoconfigure.tracing.otlp;
 
+import org.springframework.boot.actuate.autoconfigure.opentelemetry.otlp.Transport;
 import org.springframework.boot.autoconfigure.service.connection.ConnectionDetails;
 
 /**
  * Details required to establish a connection to an OpenTelemetry service.
  *
  * @author Eddú Meléndez
+ * @author Moritz Halbritter
  * @since 3.2.0
  */
 public interface OtlpTracingConnectionDetails extends ConnectionDetails {
 
 	/**
 	 * Address to where tracing will be published.
 	 * @return the address to where tracing will be published
+	 * @deprecated since 3.4.0 for removal in 3.6.0 in favor of {@link #getUrl(Transport)}
 	 */
-	String getUrl();
+	@Deprecated(since = "3.4.0", forRemoval = true)
+	default String getUrl() {
+		return getUrl(Transport.HTTP);
+	}
+
+	/**
+	 * Address to where tracing will be published.
+	 * @param transport the transport to use
+	 * @return the address to where tracing will be published
+	 * @since 3.4.0
+	 */
+	String getUrl(Transport transport);
 
 }
diff --git a/...g/springframework/boot/actuate/autoconfigure/tracing/otlp/OtlpAutoConfigurationTests.java b/...g/springframework/boot/actuate/autoconfigure/tracing/otlp/OtlpAutoConfigurationTests.java
@@ -172,7 +172,7 @@ static class ConnectionDetailsConfiguration {
 
 		@Bean
 		OtlpTracingConnectionDetails otlpTracingConnectionDetails() {
-			return () -> "http://localhost:12345/v1/traces";
+			return (transport) -> "http://localhost:12345/v1/traces";
 		}
 
 	}

diff --git a/...tlp/GrafanaOpenTelemetryTracingDockerComposeConnectionDetailsFactoryIntegrationTests.java b/...tlp/GrafanaOpenTelemetryTracingDockerComposeConnectionDetailsFactoryIntegrationTests.java
@@ -16,6 +16,7 @@
 
 package org.springframework.boot.docker.compose.service.connection.otlp;
 
+import org.springframework.boot.actuate.autoconfigure.opentelemetry.otlp.Transport;
 import org.springframework.boot.actuate.autoconfigure.tracing.otlp.OtlpTracingConnectionDetails;
 import org.springframework.boot.docker.compose.service.connection.test.DockerComposeTest;
 import org.springframework.boot.testsupport.container.TestImage;
@@ -32,7 +33,8 @@ class GrafanaOpenTelemetryTracingDockerComposeConnectionDetailsFactoryIntegratio
 
 	@DockerComposeTest(composeFile = "otlp-compose.yaml", image = TestImage.GRAFANA_OTEL_LGTM)
 	void runCreatesConnectionDetails(OtlpTracingConnectionDetails connectionDetails) {
-		assertThat(connectionDetails.getUrl()).startsWith("http://").endsWith("/v1/traces");
+		assertThat(connectionDetails.getUrl(Transport.HTTP)).startsWith("http://").endsWith("/v1/traces");
+		assertThat(connectionDetails.getUrl(Transport.GRPC)).startsWith("http://").endsWith("/v1/traces");
 	}
 
 }
diff --git a/...ction/otlp/OpenTelemetryTracingDockerComposeConnectionDetailsFactoryIntegrationTests.java b/...ction/otlp/OpenTelemetryTracingDockerComposeConnectionDetailsFactoryIntegrationTests.java
@@ -16,6 +16,7 @@
 
 package org.springframework.boot.docker.compose.service.connection.otlp;
 
+import org.springframework.boot.actuate.autoconfigure.opentelemetry.otlp.Transport;
 import org.springframework.boot.actuate.autoconfigure.tracing.otlp.OtlpTracingConnectionDetails;
 import org.springframework.boot.docker.compose.service.connection.test.DockerComposeTest;
 import org.springframework.boot.testsupport.container.TestImage;
@@ -32,7 +33,8 @@ class OpenTelemetryTracingDockerComposeConnectionDetailsFactoryIntegrationTests
 
 	@DockerComposeTest(composeFile = "otlp-compose.yaml", image = TestImage.OPENTELEMETRY)
 	void runCreatesConnectionDetails(OtlpTracingConnectionDetails connectionDetails) {
-		assertThat(connectionDetails.getUrl()).startsWith("http://").endsWith("/v1/traces");
+		assertThat(connectionDetails.getUrl(Transport.HTTP)).startsWith("http://").endsWith("/v1/traces");
+		assertThat(connectionDetails.getUrl(Transport.GRPC)).startsWith("http://").endsWith("/v1/traces");
 	}
 
 }
diff --git a/...sources/org/springframework/boot/docker/compose/service/connection/otlp/otlp-compose.yaml b/...sources/org/springframework/boot/docker/compose/service/connection/otlp/otlp-compose.yaml
@@ -2,4 +2,5 @@ services:
   otlp:
     image: '{imageName}'
     ports:
+      - '4317'
       - '4318'
diff --git a/...se/service/connection/otlp/OpenTelemetryTracingDockerComposeConnectionDetailsFactory.java b/...se/service/connection/otlp/OpenTelemetryTracingDockerComposeConnectionDetailsFactory.java
@@ -16,6 +16,7 @@
 
 package org.springframework.boot.docker.compose.service.connection.otlp;
 
+import org.springframework.boot.actuate.autoconfigure.opentelemetry.otlp.Transport;
 import org.springframework.boot.actuate.autoconfigure.tracing.otlp.OtlpTracingConnectionDetails;
 import org.springframework.boot.docker.compose.core.RunningService;
 import org.springframework.boot.docker.compose.service.connection.DockerComposeConnectionDetailsFactory;
@@ -26,14 +27,17 @@
  * {@link OtlpTracingConnectionDetails} for an OTLP service.
  *
  * @author Eddú Meléndez
+ * @author Moritz Halbritter
  */
 class OpenTelemetryTracingDockerComposeConnectionDetailsFactory
 		extends DockerComposeConnectionDetailsFactory<OtlpTracingConnectionDetails> {
 
 	private static final String[] OPENTELEMETRY_IMAGE_NAMES = { "otel/opentelemetry-collector-contrib",
 			"grafana/otel-lgtm" };
 
-	private static final int OTLP_PORT = 4318;
+	private static final int OTLP_GRPC_PORT = 4317;
+
+	private static final int OTLP_HTTP_PORT = 4318;
 
 	OpenTelemetryTracingDockerComposeConnectionDetailsFactory() {
 		super(OPENTELEMETRY_IMAGE_NAMES,
@@ -50,17 +54,24 @@ private static final class OpenTelemetryTracingDockerComposeConnectionDetails ex
 
 		private final String host;
 
-		private final int port;
+		private final int grpcPort;
+
+		private final int httPort;
 
 		private OpenTelemetryTracingDockerComposeConnectionDetails(RunningService source) {
 			super(source);
 			this.host = source.host();
-			this.port = source.ports().get(OTLP_PORT);
+			this.grpcPort = source.ports().get(OTLP_GRPC_PORT);
+			this.httPort = source.ports().get(OTLP_HTTP_PORT);
 		}
 
 		@Override
-		public String getUrl() {
-			return "http://%s:%d/v1/traces".formatted(this.host, this.port);
+		public String getUrl(Transport transport) {
+			int port = switch (transport) {
+				case HTTP -> this.httPort;
+				case GRPC -> this.grpcPort;
+			};
+			return "http://%s:%d/v1/traces".formatted(this.host, port);
 		}
 
 	}

diff --git a/...on/otlp/GrafanaOpenTelemetryTracingContainerConnectionDetailsFactoryIntegrationTests.java b/...on/otlp/GrafanaOpenTelemetryTracingContainerConnectionDetailsFactoryIntegrationTests.java
@@ -22,6 +22,7 @@
 import org.testcontainers.junit.jupiter.Testcontainers;
 
 import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.boot.actuate.autoconfigure.opentelemetry.otlp.Transport;
 import org.springframework.boot.actuate.autoconfigure.tracing.otlp.OtlpAutoConfiguration;
 import org.springframework.boot.actuate.autoconfigure.tracing.otlp.OtlpTracingConnectionDetails;
 import org.springframework.boot.autoconfigure.ImportAutoConfiguration;
@@ -50,7 +51,10 @@ class GrafanaOpenTelemetryTracingContainerConnectionDetailsFactoryIntegrationTes
 
 	@Test
 	void connectionCanBeMadeToOpenTelemetryContainer() {
-		assertThat(this.connectionDetails.getUrl()).isEqualTo("%s/v1/traces".formatted(container.getOtlpHttpUrl()));
+		assertThat(this.connectionDetails.getUrl(Transport.HTTP))
+			.isEqualTo("%s/v1/traces".formatted(container.getOtlpHttpUrl()));
+		assertThat(this.connectionDetails.getUrl(Transport.GRPC))
+			.isEqualTo("%s/v1/traces".formatted(container.getOtlpGrpcUrl()));
 	}
 
 	@Configuration(proxyBeanMethods = false)

diff --git a/...onnection/otlp/OpenTelemetryTracingContainerConnectionDetailsFactoryIntegrationTests.java b/...onnection/otlp/OpenTelemetryTracingContainerConnectionDetailsFactoryIntegrationTests.java
@@ -22,6 +22,7 @@
 import org.testcontainers.junit.jupiter.Testcontainers;
 
 import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.boot.actuate.autoconfigure.opentelemetry.otlp.Transport;
 import org.springframework.boot.actuate.autoconfigure.tracing.otlp.OtlpAutoConfiguration;
 import org.springframework.boot.actuate.autoconfigure.tracing.otlp.OtlpTracingConnectionDetails;
 import org.springframework.boot.autoconfigure.ImportAutoConfiguration;
@@ -43,15 +44,18 @@ class OpenTelemetryTracingContainerConnectionDetailsFactoryIntegrationTests {
 
 	@Container
 	@ServiceConnection
-	static final GenericContainer<?> container = TestImage.OPENTELEMETRY.genericContainer().withExposedPorts(4318);
+	static final GenericContainer<?> container = TestImage.OPENTELEMETRY.genericContainer()
+		.withExposedPorts(4317, 4318);
 
 	@Autowired
 	private OtlpTracingConnectionDetails connectionDetails;
 
 	@Test
 	void connectionCanBeMadeToOpenTelemetryContainer() {
-		assertThat(this.connectionDetails.getUrl())
+		assertThat(this.connectionDetails.getUrl(Transport.HTTP))
 			.isEqualTo("http://" + container.getHost() + ":" + container.getMappedPort(4318) + "/v1/traces");
+		assertThat(this.connectionDetails.getUrl(Transport.GRPC))
+			.isEqualTo("http://" + container.getHost() + ":" + container.getMappedPort(4317) + "/v1/traces");
 	}
 
 	@Configuration(proxyBeanMethods = false)

diff --git a/...service/connection/otlp/GrafanaOpenTelemetryTracingContainerConnectionDetailsFactory.java b/...service/connection/otlp/GrafanaOpenTelemetryTracingContainerConnectionDetailsFactory.java
@@ -18,6 +18,7 @@
 
 import org.testcontainers.grafana.LgtmStackContainer;
 
+import org.springframework.boot.actuate.autoconfigure.opentelemetry.otlp.Transport;
 import org.springframework.boot.actuate.autoconfigure.tracing.otlp.OtlpTracingConnectionDetails;
 import org.springframework.boot.testcontainers.service.connection.ContainerConnectionDetailsFactory;
 import org.springframework.boot.testcontainers.service.connection.ContainerConnectionSource;
@@ -52,8 +53,12 @@ private OpenTelemetryTracingContainerConnectionDetails(ContainerConnectionSource
 		}
 
 		@Override
-		public String getUrl() {
-			return "%s/v1/traces".formatted(getContainer().getOtlpHttpUrl());
+		public String getUrl(Transport transport) {
+			String url = switch (transport) {
+				case HTTP -> getContainer().getOtlpHttpUrl();
+				case GRPC -> getContainer().getOtlpGrpcUrl();
+			};
+			return "%s/v1/traces".formatted(url);
 		}
 
 	}

diff --git a/...ainers/service/connection/otlp/OpenTelemetryTracingContainerConnectionDetailsFactory.java b/...ainers/service/connection/otlp/OpenTelemetryTracingContainerConnectionDetailsFactory.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2012-2023 the original author or authors.
+ * 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.
@@ -19,6 +19,7 @@
 import org.testcontainers.containers.Container;
 import org.testcontainers.containers.GenericContainer;
 
+import org.springframework.boot.actuate.autoconfigure.opentelemetry.otlp.Transport;
 import org.springframework.boot.actuate.autoconfigure.tracing.otlp.OtlpTracingConnectionDetails;
 import org.springframework.boot.testcontainers.service.connection.ContainerConnectionDetailsFactory;
 import org.springframework.boot.testcontainers.service.connection.ContainerConnectionSource;
@@ -35,6 +36,10 @@
 class OpenTelemetryTracingContainerConnectionDetailsFactory
 		extends ContainerConnectionDetailsFactory<Container<?>, OtlpTracingConnectionDetails> {
 
+	private static final int OTLP_GRPC_PORT = 4317;
+
+	private static final int OTLP_HTTP_PORT = 4318;
+
 	OpenTelemetryTracingContainerConnectionDetailsFactory() {
 		super("otel/opentelemetry-collector-contrib",
 				"org.springframework.boot.actuate.autoconfigure.tracing.otlp.OtlpAutoConfiguration");
@@ -54,8 +59,12 @@ private OpenTelemetryTracingContainerConnectionDetails(ContainerConnectionSource
 		}
 
 		@Override
-		public String getUrl() {
-			return "http://%s:%d/v1/traces".formatted(getContainer().getHost(), getContainer().getMappedPort(4318));
+		public String getUrl(Transport transport) {
+			int port = switch (transport) {
+				case HTTP -> OTLP_HTTP_PORT;
+				case GRPC -> OTLP_GRPC_PORT;
+			};
+			return "http://%s:%d/v1/traces".formatted(getContainer().getHost(), getContainer().getMappedPort(port));
 		}
 
 	}
-Original file line number
+Diff line change
@@ Expand Up / @@ -172,7 +172,7 @@ static class ConnectionDetailsConfiguration { @@
     		@Bean
     		OtlpTracingConnectionDetails otlpTracingConnectionDetails() {
-    			return () -> "http://localhost:12345/v1/traces";
+    			return (transport) -> "http://localhost:12345/v1/traces";
     		}
     	}
@@ Expand Down @@