Log a warning if OTLP endpoint port is likely incorrect given the pro…

…tocol (#6813)
open-telemetry · Oct 22, 2024 · 45b8c13 · 45b8c13
1 parent f52554b
commit 45b8c13
Show file tree

Hide file tree

Showing 2 changed files with 73 additions and 2 deletions.
diff --git a/exporters/otlp/all/src/main/java/io/opentelemetry/exporter/otlp/internal/OtlpConfigUtil.java b/exporters/otlp/all/src/main/java/io/opentelemetry/exporter/otlp/internal/OtlpConfigUtil.java
@@ -25,6 +25,7 @@
 import java.util.Map;
 import java.util.function.BiConsumer;
 import java.util.function.Consumer;
+import java.util.logging.Logger;
 import javax.annotation.Nullable;
 
 /**
@@ -33,6 +34,8 @@
  */
 public final class OtlpConfigUtil {
 
+  private static final Logger logger = Logger.getLogger(OtlpConfigUtil.class.getName());
+
   public static final String DATA_TYPE_TRACES = "traces";
   public static final String DATA_TYPE_METRICS = "metrics";
   public static final String DATA_TYPE_LOGS = "logs";
@@ -263,7 +266,7 @@ private static URL createUrl(URL context, String spec) {
   }
 
   @Nullable
-  private static URL validateEndpoint(@Nullable String endpoint, boolean allowPath) {
+  private static URL validateEndpoint(@Nullable String endpoint, boolean isHttpProtobuf) {
     if (endpoint == null) {
       return null;
     }
@@ -285,10 +288,28 @@ private static URL validateEndpoint(@Nullable String endpoint, boolean allowPath
       throw new ConfigurationException(
           "OTLP endpoint must not have a fragment: " + endpointUrl.getRef());
     }
-    if (!allowPath && (!endpointUrl.getPath().isEmpty() && !endpointUrl.getPath().equals("/"))) {
+    if (!isHttpProtobuf
+        && (!endpointUrl.getPath().isEmpty() && !endpointUrl.getPath().equals("/"))) {
       throw new ConfigurationException(
           "OTLP endpoint must not have a path: " + endpointUrl.getPath());
     }
+    if ((endpointUrl.getPort() == 4317 && isHttpProtobuf)
+        || (endpointUrl.getPort() == 4318 && !isHttpProtobuf)) {
+      int expectedPort = isHttpProtobuf ? 4318 : 4317;
+      String protocol = isHttpProtobuf ? PROTOCOL_HTTP_PROTOBUF : PROTOCOL_GRPC;
+      logger.warning(
+          "OTLP exporter endpoint port is likely incorrect for protocol version \""
+              + protocol
+              + "\". The endpoint "
+              + endpointUrl
+              + " has port "
+              + endpointUrl.getPort()
+              + ". Typically, the \""
+              + protocol
+              + "\" version of OTLP uses port "
+              + expectedPort
+              + ".");
+    }
     return endpointUrl;
   }
 

diff --git a/...rs/otlp/all/src/test/java/io/opentelemetry/exporter/otlp/internal/OtlpConfigUtilTest.java b/...rs/otlp/all/src/test/java/io/opentelemetry/exporter/otlp/internal/OtlpConfigUtilTest.java
@@ -11,11 +11,14 @@
 import static io.opentelemetry.exporter.otlp.internal.OtlpConfigUtil.PROTOCOL_GRPC;
 import static io.opentelemetry.exporter.otlp.internal.OtlpConfigUtil.PROTOCOL_HTTP_PROTOBUF;
 import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.assertThat;
+import static java.util.stream.Collectors.toList;
 import static org.assertj.core.api.Assertions.assertThatCode;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
 
 import com.google.common.collect.ImmutableMap;
+import io.github.netmikey.logunit.api.LogCapturer;
 import io.opentelemetry.exporter.internal.ExporterBuilderUtil;
+import io.opentelemetry.internal.testing.slf4j.SuppressLogger;
 import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties;
 import io.opentelemetry.sdk.autoconfigure.spi.ConfigurationException;
 import io.opentelemetry.sdk.autoconfigure.spi.internal.DefaultConfigProperties;
@@ -25,13 +28,24 @@
 import io.opentelemetry.sdk.metrics.export.AggregationTemporalitySelector;
 import io.opentelemetry.sdk.metrics.export.DefaultAggregationSelector;
 import java.util.Collections;
+import java.util.List;
 import java.util.Map;
 import java.util.concurrent.atomic.AtomicReference;
+import java.util.stream.Stream;
+import javax.annotation.Nullable;
 import org.assertj.core.api.ThrowableAssert.ThrowingCallable;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+import org.slf4j.event.LoggingEvent;
 
 class OtlpConfigUtilTest {
 
+  @RegisterExtension
+  final LogCapturer logs = LogCapturer.create().captureForType(OtlpConfigUtil.class);
+
   private static final String GENERIC_ENDPOINT_KEY = "otel.exporter.otlp.endpoint";
   private static final String TRACES_ENDPOINT_KEY = "otel.exporter.otlp.traces.endpoint";
   private static final String METRICS_ENDPOINT_KEY = "otel.exporter.otlp.metrics.endpoint";
@@ -122,6 +136,42 @@ void configureOtlpExporterBuilder_InvalidEndpoints() {
         .hasMessageContaining("OTLP endpoint must not have a path:");
   }
 
+  @SuppressLogger(OtlpConfigUtil.class)
+  @ParameterizedTest
+  @MethodSource("misalignedOtlpPortArgs")
+  void configureOtlpExporterBuilder_MisalignedOtlpPort(
+      String protocol, String endpoint, @Nullable String expectedLog) {
+    configureEndpoint(
+        DATA_TYPE_TRACES,
+        ImmutableMap.of(GENERIC_ENDPOINT_KEY, endpoint, "otel.exporter.otlp.protocol", protocol));
+
+    List<String> logMessages =
+        logs.getEvents().stream().map(LoggingEvent::getMessage).collect(toList());
+    if (expectedLog == null) {
+      assertThat(logMessages).isEmpty();
+    } else {
+      assertThat(logMessages).contains(expectedLog);
+    }
+  }
+
+  private static Stream<Arguments> misalignedOtlpPortArgs() {
+    return Stream.of(
+        Arguments.of("http/protobuf", "http://localhost:4318/path", null),
+        Arguments.of("http/protobuf", "http://localhost:8080/path", null),
+        Arguments.of("http/protobuf", "http://localhost/path", null),
+        Arguments.of(
+            "http/protobuf",
+            "http://localhost:4317/path",
+            "OTLP exporter endpoint port is likely incorrect for protocol version \"http/protobuf\". The endpoint http://localhost:4317/path has port 4317. Typically, the \"http/protobuf\" version of OTLP uses port 4318."),
+        Arguments.of("grpc", "http://localhost:4317/", null),
+        Arguments.of("grpc", "http://localhost:8080/", null),
+        Arguments.of("grpc", "http://localhost/", null),
+        Arguments.of(
+            "grpc",
+            "http://localhost:4318/",
+            "OTLP exporter endpoint port is likely incorrect for protocol version \"grpc\". The endpoint http://localhost:4318/ has port 4318. Typically, the \"grpc\" version of OTLP uses port 4317."));
+  }
+
   private static ThrowingCallable configureEndpointCallable(Map<String, String> properties) {
     return () -> configureEndpoint(DATA_TYPE_TRACES, properties);
   }