Make url.scheme opt in for HTTP client metrics and make `server.por…

…t` required (#9784)

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

@@ -145,7 +145,6 @@ InternalNetworkAttributesExtractor<REQUEST, RESPONSE> buildNetworkExtractor() {
 
   InternalServerAttributesExtractor<REQUEST> buildServerExtractor() {
     return new InternalServerAttributesExtractor<>(
-        new ClientSideServerPortCondition<>(httpAttributesGetter),
         serverAddressAndPortExtractor,
         SemconvStability.emitStableHttpSemconv(),
         SemconvStability.emitOldHttpSemconv(),

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

@@ -7,7 +7,6 @@
 
 import static io.opentelemetry.instrumentation.api.instrumenter.http.HttpMessageBodySizeUtil.getHttpRequestBodySize;
 import static io.opentelemetry.instrumentation.api.instrumenter.http.HttpMessageBodySizeUtil.getHttpResponseBodySize;
-import static io.opentelemetry.instrumentation.api.instrumenter.http.HttpMetricsUtil.mergeClientAttributes;
 import static java.util.logging.Level.FINE;
 
 import io.opentelemetry.api.common.Attributes;
@@ -84,7 +83,7 @@ public void onEnd(Context context, Attributes endAttributes, long endNanos) {
       return;
     }
 
-    Attributes sizeAttributes = mergeClientAttributes(startAttributes, endAttributes);
+    Attributes sizeAttributes = startAttributes.toBuilder().putAll(endAttributes).build();
 
     Long requestBodySize = getHttpRequestBodySize(endAttributes, startAttributes);
     if (requestBodySize != null) {

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

@@ -6,7 +6,6 @@
 package io.opentelemetry.instrumentation.api.instrumenter.http;
 
 import static io.opentelemetry.instrumentation.api.instrumenter.http.HttpMetricsUtil.createStableDurationHistogramBuilder;
-import static io.opentelemetry.instrumentation.api.instrumenter.http.HttpMetricsUtil.mergeClientAttributes;
 import static java.util.logging.Level.FINE;
 
 import com.google.auto.value.AutoValue;
@@ -91,7 +90,7 @@ public void onEnd(Context context, Attributes endAttributes, long endNanos) {
       return;
     }
 
-    Attributes attributes = mergeClientAttributes(state.startAttributes(), endAttributes);
+    Attributes attributes = state.startAttributes().toBuilder().putAll(endAttributes).build();
 
     if (stableDuration != null) {
       stableDuration.record((endNanos - state.startTimeNanos()) / NANOS_PER_S, attributes, context);

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

@@ -31,8 +31,7 @@ static void applyStableClientDurationAdvice(DoubleHistogramBuilder builder) {
                 SemanticAttributes.NETWORK_PROTOCOL_NAME,
                 SemanticAttributes.NETWORK_PROTOCOL_VERSION,
                 SemanticAttributes.SERVER_ADDRESS,
-                SemanticAttributes.SERVER_PORT,
-                SemanticAttributes.URL_SCHEME));
+                SemanticAttributes.SERVER_PORT));
   }
 
   @SuppressWarnings("deprecation") // until old http semconv are dropped in 2.0
@@ -68,7 +67,6 @@ static void applyClientRequestSizeAdvice(LongHistogramBuilder builder) {
                 SemanticAttributes.NETWORK_PROTOCOL_VERSION,
                 SemanticAttributes.SERVER_ADDRESS,
                 SemanticAttributes.SERVER_PORT,
-                SemanticAttributes.URL_SCHEME,
                 // old attributes
                 SemanticAttributes.HTTP_METHOD,
                 SemanticAttributes.HTTP_STATUS_CODE,

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

@@ -8,12 +8,9 @@
 import static java.util.Arrays.asList;
 import static java.util.Collections.unmodifiableList;
 
-import io.opentelemetry.api.common.Attributes;
-import io.opentelemetry.api.common.AttributesBuilder;
 import io.opentelemetry.api.metrics.DoubleHistogramBuilder;
 import io.opentelemetry.api.metrics.Meter;
 import io.opentelemetry.extension.incubator.metrics.ExtendedDoubleHistogramBuilder;
-import io.opentelemetry.semconv.SemanticAttributes;
 import java.util.List;
 
 final class HttpMetricsUtil {
@@ -34,17 +31,5 @@ static DoubleHistogramBuilder createStableDurationHistogramBuilder(
     return durationBuilder;
   }
 
-  static Attributes mergeClientAttributes(Attributes startAttributes, Attributes endAttributes) {
-    AttributesBuilder builder = startAttributes.toBuilder().putAll(endAttributes);
-    String url = startAttributes.get(SemanticAttributes.URL_FULL);
-    if (url != null) {
-      int index = url.indexOf("://");
-      if (index > 0) {
-        builder.put(SemanticAttributes.URL_SCHEME, url.substring(0, index));
-      }
-    }
-    return builder.build();
-  }
-
   private HttpMetricsUtil() {}
 }

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

@@ -160,7 +160,6 @@ InternalNetworkAttributesExtractor<REQUEST, RESPONSE> buildNetworkExtractor() {
 
   InternalServerAttributesExtractor<REQUEST> buildServerExtractor() {
     return new InternalServerAttributesExtractor<>(
-        new ServerSideServerPortCondition<>(httpAttributesGetter),
         serverAddressPortExtractor,
         SemconvStability.emitStableHttpSemconv(),
         SemconvStability.emitOldHttpSemconv(),

instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/net/NetClientAttributesExtractor.java

@@ -58,7 +58,6 @@ private NetClientAttributesExtractor(NetClientAttributesGetter<REQUEST, RESPONSE
             SemconvStability.emitOldHttpSemconv());
     internalServerExtractor =
         new InternalServerAttributesExtractor<>(
-            (port, request) -> true,
             serverAddressAndPortExtractor,
             SemconvStability.emitStableHttpSemconv(),
             SemconvStability.emitOldHttpSemconv(),

instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/net/NetServerAttributesExtractor.java

@@ -60,7 +60,6 @@ private NetServerAttributesExtractor(NetServerAttributesGetter<REQUEST, RESPONSE
             SemconvStability.emitOldHttpSemconv());
     internalServerExtractor =
         new InternalServerAttributesExtractor<>(
-            (port, request) -> true,
             serverAddressAndPortExtractor,
             SemconvStability.emitStableHttpSemconv(),
             SemconvStability.emitOldHttpSemconv(),

instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/network/ServerAttributesExtractor.java

@@ -51,10 +51,8 @@ ServerAttributesExtractor<REQUEST, RESPONSE> createForServerSide(
   ServerAttributesExtractor(
       ServerAttributesGetter<REQUEST, RESPONSE> getter,
       InternalServerAttributesExtractor.Mode mode) {
-    // the ServerAttributesExtractor will always emit new semconv
     internalExtractor =
         new InternalServerAttributesExtractor<>(
-            (port, request) -> true,
             new ServerAddressAndPortExtractor<>(getter, AddressAndPortExtractor.noop()),
             SemconvStability.emitStableHttpSemconv(),
             SemconvStability.emitOldHttpSemconv(),

instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/network/internal/InternalServerAttributesExtractor.java

@@ -10,27 +10,23 @@
 import io.opentelemetry.api.common.AttributeKey;
 import io.opentelemetry.api.common.AttributesBuilder;
 import io.opentelemetry.semconv.SemanticAttributes;
-import java.util.function.BiPredicate;
 
 /**
  * This class is internal and is hence not for public use. Its APIs are unstable and can change at
  * any time.
  */
 public final class InternalServerAttributesExtractor<REQUEST> {
 
-  private final BiPredicate<Integer, REQUEST> captureServerPortCondition;
   private final AddressAndPortExtractor<REQUEST> addressAndPortExtractor;
   private final boolean emitStableUrlAttributes;
   private final boolean emitOldHttpAttributes;
   private final Mode oldSemconvMode;
 
   public InternalServerAttributesExtractor(
-      BiPredicate<Integer, REQUEST> captureServerPortCondition,
       AddressAndPortExtractor<REQUEST> addressAndPortExtractor,
       boolean emitStableUrlAttributes,
       boolean emitOldHttpAttributes,
       Mode oldSemconvMode) {
-    this.captureServerPortCondition = captureServerPortCondition;
     this.addressAndPortExtractor = addressAndPortExtractor;
     this.emitStableUrlAttributes = emitStableUrlAttributes;
     this.emitOldHttpAttributes = emitOldHttpAttributes;
@@ -48,9 +44,7 @@ public void onStart(AttributesBuilder attributes, REQUEST request) {
         internalSet(attributes, oldSemconvMode.address, serverAddressAndPort.address);
       }
 
-      if (serverAddressAndPort.port != null
-          && serverAddressAndPort.port > 0
-          && captureServerPortCondition.test(serverAddressAndPort.port, request)) {
+      if (serverAddressAndPort.port != null && serverAddressAndPort.port > 0) {
         if (emitStableUrlAttributes) {
           internalSet(attributes, SemanticAttributes.SERVER_PORT, (long) serverAddressAndPort.port);
         }

instrumentation-api-semconv/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractorTest.java

@@ -122,7 +122,7 @@ void normal() {
     request.put("protocolName", "http");
     request.put("protocolVersion", "1.1");
     request.put("serverAddress", "github.com");
-    request.put("serverPort", "123");
+    request.put("serverPort", "80");
 
     Map<String, String> response = new HashMap<>();
     response.put("statusCode", "202");
@@ -148,7 +148,7 @@ void normal() {
                 AttributeKey.stringArrayKey("http.request.header.custom_request_header"),
                 asList("123", "456")),
             entry(SemanticAttributes.NET_PEER_NAME, "github.com"),
-            entry(SemanticAttributes.NET_PEER_PORT, 123L),
+            entry(SemanticAttributes.NET_PEER_PORT, 80L),
             entry(HttpAttributes.HTTP_REQUEST_RESEND_COUNT, 2L));
 
     AttributesBuilder endAttributes = Attributes.builder();
@@ -259,34 +259,6 @@ void extractNetHostAndPortFromNetAttributesGetter() {
             entry(SemanticAttributes.NET_PEER_PORT, 777L));
   }
 
-  @ParameterizedTest
-  @ArgumentsSource(DefaultPeerPortArgumentSource.class)
-  void defaultPeerPort(int peerPort, String url) {
-    Map<String, String> request = new HashMap<>();
-    request.put("urlFull", url);
-    request.put("serverPort", String.valueOf(peerPort));
-
-    AttributesExtractor<Map<String, String>, Map<String, String>> extractor =
-        HttpClientAttributesExtractor.create(new TestHttpClientAttributesGetter());
-
-    AttributesBuilder attributes = Attributes.builder();
-    extractor.onStart(attributes, Context.root(), request);
-
-    assertThat(attributes.build()).doesNotContainKey(SemanticAttributes.NET_PEER_PORT);
-  }
-
-  static class DefaultPeerPortArgumentSource implements ArgumentsProvider {
-
-    @Override
-    public Stream<? extends Arguments> provideArguments(ExtensionContext context) {
-      return Stream.of(
-          arguments(80, "http://github.com"),
-          arguments(80, "HTTP://GITHUB.COM"),
-          arguments(443, "https://github.com"),
-          arguments(443, "HTTPS://GITHUB.COM"));
-    }
-  }
-
   @Test
   void zeroResends() {
     Map<String, String> request = new HashMap<>();

instrumentation-api-semconv/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerAttributesExtractorTest.java

@@ -124,7 +124,7 @@ void normal() {
     request.put("header.content-length", "10");
     request.put("route", "/repositories/{id}");
     request.put("header.user-agent", "okhttp 3.x");
-    request.put("header.host", "github.com");
+    request.put("header.host", "github.com:80");
     request.put("header.forwarded", "for=1.1.1.1;proto=https");
     request.put("header.custom-request-header", "123,456");
     request.put("networkTransport", "tcp");
@@ -151,6 +151,7 @@ void normal() {
     assertThat(startAttributes.build())
         .containsOnly(
             entry(SemanticAttributes.NET_HOST_NAME, "github.com"),
+            entry(SemanticAttributes.NET_HOST_PORT, 80L),
             entry(SemanticAttributes.HTTP_METHOD, "POST"),
             entry(SemanticAttributes.HTTP_SCHEME, "https"),
             entry(SemanticAttributes.HTTP_TARGET, "/repositories/1?details=true"),
@@ -235,33 +236,6 @@ void extractNetHostAndPortFromHostHeader() {
             entry(SemanticAttributes.NET_HOST_PORT, 777L));
   }
 
-  @ParameterizedTest
-  @ArgumentsSource(DefaultHostPortArgumentSource.class)
-  void defaultHostPort(int hostPort, String scheme) {
-    Map<String, Object> request = new HashMap<>();
-    request.put("urlScheme", scheme);
-    request.put("serverPort", hostPort);
-
-    AttributesExtractor<Map<String, Object>, Map<String, Object>> extractor =
-        HttpServerAttributesExtractor.builder(new TestHttpServerAttributesGetter())
-            .setCapturedRequestHeaders(emptyList())
-            .setCapturedResponseHeaders(emptyList())
-            .build();
-
-    AttributesBuilder attributes = Attributes.builder();
-    extractor.onStart(attributes, Context.root(), request);
-
-    assertThat(attributes.build()).doesNotContainKey(SemanticAttributes.NET_HOST_PORT);
-  }
-
-  static class DefaultHostPortArgumentSource implements ArgumentsProvider {
-
-    @Override
-    public Stream<? extends Arguments> provideArguments(ExtensionContext context) {
-      return Stream.of(arguments(80, "http"), arguments(443, "https"));
-    }
-  }
-
   @ParameterizedTest
   @ArgumentsSource(PathAndQueryArgumentSource.class)
   void computeTargetFromPathAndQuery(String path, String query, String expectedTarget) {