Peer service testing (#14963)

Co-authored-by: otelbot <197425009+otelbot@users.noreply.github.com>
Co-authored-by: Lauri Tulmin <ltulmin@splunk.com>

Author: 3 people

conventions/src/main/kotlin/io.opentelemetry.instrumentation.javaagent-testing.gradle.kts

@@ -94,7 +94,8 @@ class JavaagentTestArgumentsProvider(
     "-Dio.opentelemetry.javaagent.slf4j.simpleLogger.log.io.grpc.internal.ManagedChannelImplBuilder=INFO",
     "-Dio.opentelemetry.javaagent.slf4j.simpleLogger.log.io.perfmark.PerfMark=INFO",
     "-Dio.opentelemetry.javaagent.slf4j.simpleLogger.log.io.grpc.Context=INFO",
-    "-Dotel.java.experimental.span-attributes.copy-from-baggage.include=test-baggage-key-1,test-baggage-key-2"
+    "-Dotel.java.experimental.span-attributes.copy-from-baggage.include=test-baggage-key-1,test-baggage-key-2",
+    "-Dotel.instrumentation.common.peer-service-mapping=127.0.0.1=test-peer-service,localhost=test-peer-service,192.0.2.1=test-peer-service"
   )
 }
 

instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/http/HttpClientPeerServiceAttributesExtractor.java

@@ -12,6 +12,10 @@
 import io.opentelemetry.instrumentation.api.incubator.semconv.net.internal.UrlParser;
 import io.opentelemetry.instrumentation.api.instrumenter.AttributesExtractor;
 import io.opentelemetry.instrumentation.api.semconv.http.HttpClientAttributesGetter;
+import io.opentelemetry.instrumentation.api.semconv.http.internal.HostAddressAndPortExtractor;
+import io.opentelemetry.instrumentation.api.semconv.network.internal.AddressAndPort;
+import io.opentelemetry.instrumentation.api.semconv.network.internal.AddressAndPortExtractor;
+import io.opentelemetry.instrumentation.api.semconv.network.internal.ServerAddressAndPortExtractor;
 import java.util.function.Supplier;
 import javax.annotation.Nullable;
 
@@ -26,26 +30,33 @@ public final class HttpClientPeerServiceAttributesExtractor<REQUEST, RESPONSE>
   // copied from PeerIncubatingAttributes
   private static final AttributeKey<String> PEER_SERVICE = AttributeKey.stringKey("peer.service");
 
+  private final AddressAndPortExtractor<REQUEST> addressAndPortExtractor;
   private final HttpClientAttributesGetter<REQUEST, RESPONSE> attributesGetter;
   private final PeerServiceResolver peerServiceResolver;
 
   // visible for tests
   HttpClientPeerServiceAttributesExtractor(
+      AddressAndPortExtractor<REQUEST> addressAndPortExtractor,
       HttpClientAttributesGetter<REQUEST, RESPONSE> attributesGetter,
       PeerServiceResolver peerServiceResolver) {
+    this.addressAndPortExtractor = addressAndPortExtractor;
     this.attributesGetter = attributesGetter;
     this.peerServiceResolver = peerServiceResolver;
   }
 
   /**
    * Returns a new {@link HttpClientPeerServiceAttributesExtractor} that will use the passed {@code
-   * attributesGetter} instance to determine the value of the {@code peer.service} attribute.
+   * attributesGetter} to extract server address and port (with fallback to the HTTP Host header).
    */
   public static <REQUEST, RESPONSE>
       HttpClientPeerServiceAttributesExtractor<REQUEST, RESPONSE> create(
           HttpClientAttributesGetter<REQUEST, RESPONSE> attributesGetter,
           PeerServiceResolver peerServiceResolver) {
-    return new HttpClientPeerServiceAttributesExtractor<>(attributesGetter, peerServiceResolver);
+    AddressAndPortExtractor<REQUEST> addressAndPortExtractor =
+        new ServerAddressAndPortExtractor<>(
+            attributesGetter, new HostAddressAndPortExtractor<>(attributesGetter));
+    return new HttpClientPeerServiceAttributesExtractor<>(
+        addressAndPortExtractor, attributesGetter, peerServiceResolver);
   }
 
   @Override
@@ -65,10 +76,11 @@ public void onEnd(
       return;
     }
 
-    String serverAddress = attributesGetter.getServerAddress(request);
-    Integer serverPort = attributesGetter.getServerPort(request);
+    AddressAndPort addressAndPort = addressAndPortExtractor.extract(request);
+
     Supplier<String> pathSupplier = () -> getUrlPath(attributesGetter, request);
-    String peerService = mapToPeerService(serverAddress, serverPort, pathSupplier);
+    String peerService =
+        mapToPeerService(addressAndPort.getAddress(), addressAndPort.getPort(), pathSupplier);
     if (peerService != null) {
       attributes.put(PEER_SERVICE, peerService);
     }

instrumentation-api-incubator/src/test/java/io/opentelemetry/instrumentation/api/incubator/semconv/http/HttpClientPeerServiceAttributesExtractorTest.java

@@ -6,10 +6,12 @@
 package io.opentelemetry.instrumentation.api.incubator.semconv.http;
 
 import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.assertThat;
+import static java.util.Collections.singletonList;
 import static java.util.Collections.singletonMap;
 import static org.assertj.core.api.Assertions.entry;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.when;
 
 import io.opentelemetry.api.common.Attributes;
@@ -36,7 +38,7 @@ void shouldNotSetAnyValueIfNetExtractorReturnsNulls() {
         PeerServiceResolver.create(singletonMap("1.2.3.4", "myService"));
 
     HttpClientPeerServiceAttributesExtractor<String, String> underTest =
-        new HttpClientPeerServiceAttributesExtractor<>(
+        HttpClientPeerServiceAttributesExtractor.create(
             httpAttributesExtractor, peerServiceResolver);
 
     Context context = Context.root();
@@ -57,7 +59,7 @@ void shouldNotSetAnyValueIfPeerNameDoesNotMatch() {
         PeerServiceResolver.create(singletonMap("example.com", "myService"));
 
     HttpClientPeerServiceAttributesExtractor<String, String> underTest =
-        new HttpClientPeerServiceAttributesExtractor<>(
+        HttpClientPeerServiceAttributesExtractor.create(
             httpAttributesExtractor, peerServiceResolver);
 
     when(httpAttributesExtractor.getServerAddress(any())).thenReturn("example2.com");
@@ -85,7 +87,7 @@ void shouldSetPeerNameIfItMatches() {
     PeerServiceResolver peerServiceResolver = PeerServiceResolver.create(peerServiceMapping);
 
     HttpClientPeerServiceAttributesExtractor<String, String> underTest =
-        new HttpClientPeerServiceAttributesExtractor<>(
+        HttpClientPeerServiceAttributesExtractor.create(
             httpAttributesExtractor, peerServiceResolver);
 
     when(httpAttributesExtractor.getServerAddress(any())).thenReturn("example.com");
@@ -103,4 +105,34 @@ void shouldSetPeerNameIfItMatches() {
     assertThat(endAttributes.build())
         .containsOnly(entry(PeerIncubatingAttributes.PEER_SERVICE, "myService"));
   }
+
+  @Test
+  void shouldFallbackToHostHeaderWhenServerAddressIsNull() {
+    // given
+    PeerServiceResolver peerServiceResolver =
+        PeerServiceResolver.create(singletonMap("example.com", "myService"));
+
+    HttpClientPeerServiceAttributesExtractor<String, String> underTest =
+        HttpClientPeerServiceAttributesExtractor.create(
+            httpAttributesExtractor, peerServiceResolver);
+
+    // server address is null, should fallback to Host header
+    when(httpAttributesExtractor.getServerAddress(any())).thenReturn(null);
+    when(httpAttributesExtractor.getServerPort(any())).thenReturn(null);
+    when(httpAttributesExtractor.getHttpRequestHeader(any(), eq("host")))
+        .thenReturn(singletonList("example.com:8080"));
+
+    Context context = Context.root();
+
+    // when
+    AttributesBuilder startAttributes = Attributes.builder();
+    underTest.onStart(startAttributes, context, "request");
+    AttributesBuilder endAttributes = Attributes.builder();
+    underTest.onEnd(endAttributes, context, "request", "response", null);
+
+    // then
+    assertThat(startAttributes.build()).isEmpty();
+    assertThat(endAttributes.build())
+        .containsOnly(entry(PeerIncubatingAttributes.PEER_SERVICE, "myService"));
+  }
 }

instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/semconv/http/HostAddressAndPortExtractor.java

@@ -13,6 +13,7 @@
 import io.opentelemetry.instrumentation.api.instrumenter.InstrumenterBuilder;
 import io.opentelemetry.instrumentation.api.internal.Experimental;
 import io.opentelemetry.instrumentation.api.internal.HttpConstants;
+import io.opentelemetry.instrumentation.api.semconv.http.internal.HostAddressAndPortExtractor;
 import io.opentelemetry.instrumentation.api.semconv.network.internal.AddressAndPortExtractor;
 import io.opentelemetry.instrumentation.api.semconv.network.internal.InternalNetworkAttributesExtractor;
 import io.opentelemetry.instrumentation.api.semconv.network.internal.InternalServerAttributesExtractor;

instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/semconv/http/HttpClientAttributesExtractorBuilder.java

@@ -0,0 +1,59 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.instrumentation.api.semconv.http.internal;
+
+import io.opentelemetry.instrumentation.api.semconv.http.HttpCommonAttributesGetter;
+import io.opentelemetry.instrumentation.api.semconv.network.internal.AddressAndPortExtractor;
+import java.util.List;
+import javax.annotation.Nullable;
+
+/**
+ * Extracts server address and port from the HTTP Host header.
+ *
+ * <p>This class is internal and is hence not for public use. Its APIs are unstable and can change
+ * at any time.
+ */
+public final class HostAddressAndPortExtractor<REQUEST>
+    implements AddressAndPortExtractor<REQUEST> {
+
+  private final HttpCommonAttributesGetter<REQUEST, ?> getter;
+
+  public HostAddressAndPortExtractor(HttpCommonAttributesGetter<REQUEST, ?> getter) {
+    this.getter = getter;
+  }
+
+  @Override
+  public void extract(AddressPortSink sink, REQUEST request) {
+    String host = firstHeaderValue(getter.getHttpRequestHeader(request, "host"));
+    if (host == null) {
+      return;
+    }
+
+    int hostHeaderSeparator = host.indexOf(':');
+    if (hostHeaderSeparator == -1) {
+      sink.setAddress(host);
+    } else {
+      sink.setAddress(host.substring(0, hostHeaderSeparator));
+      setPort(sink, host, hostHeaderSeparator + 1, host.length());
+    }
+  }
+
+  @Nullable
+  private static String firstHeaderValue(List<String> values) {
+    return values.isEmpty() ? null : values.get(0);
+  }
+
+  private static void setPort(AddressPortSink sink, String header, int start, int end) {
+    if (start == end) {
+      return;
+    }
+    try {
+      sink.setPort(Integer.parseInt(header.substring(start, end)));
+    } catch (NumberFormatException ignored) {
+      // malformed port, ignoring
+    }
+  }
+}

instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/semconv/http/internal/HostAddressAndPortExtractor.java

@@ -3,7 +3,7 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-package io.opentelemetry.instrumentation.api.semconv.http;
+package io.opentelemetry.instrumentation.api.semconv.http.internal;
 
 import static java.util.Collections.emptyList;
 import static java.util.Collections.singletonList;
@@ -12,6 +12,7 @@
 import static org.mockito.Mockito.verifyNoMoreInteractions;
 import static org.mockito.Mockito.when;
 
+import io.opentelemetry.instrumentation.api.semconv.http.HttpCommonAttributesGetter;
 import io.opentelemetry.instrumentation.api.semconv.network.internal.AddressAndPortExtractor;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;

instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/semconv/http/HostAddressAndPortExtractorTest.java renamed to instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/semconv/http/internal/HostAddressAndPortExtractorTest.java

@@ -19,4 +19,9 @@ class DubboAgentTest extends AbstractDubboTest {
   protected InstrumentationExtension testing() {
     return testing;
   }
+
+  @Override
+  protected boolean hasPeerService() {
+    return true;
+  }
 }

instrumentation/apache-dubbo-2.7/javaagent/src/testDubbo/java/io/opentelemetry/javaagent/instrumentation/apachedubbo/v2_7/DubboAgentTest.java

@@ -19,4 +19,9 @@ class DubboAgentTraceChainTest extends AbstractDubboTraceChainTest {
   protected InstrumentationExtension testing() {
     return testing;
   }
+
+  @Override
+  protected boolean hasPeerService() {
+    return true;
+  }
 }

instrumentation/apache-dubbo-2.7/javaagent/src/testDubbo/java/io/opentelemetry/javaagent/instrumentation/apachedubbo/v2_7/DubboAgentTraceChainTest.java

@@ -18,4 +18,9 @@ class DubboTest extends AbstractDubboTest {
   protected InstrumentationExtension testing() {
     return testing;
   }
+
+  @Override
+  protected boolean hasPeerService() {
+    return false;
+  }
 }