Review fixes

Author: JonasKunz

exporters/common/src/main/java/io/opentelemetry/exporter/internal/metrics/ExporterInstrumentation.java

@@ -6,11 +6,14 @@
 package io.opentelemetry.exporter.internal.metrics;
 
 import io.opentelemetry.api.common.Attributes;
+import io.opentelemetry.api.common.AttributesBuilder;
 import io.opentelemetry.api.metrics.MeterProvider;
 import io.opentelemetry.sdk.common.InternalTelemetryVersion;
 import io.opentelemetry.sdk.internal.SemConvAttributes;
 import io.opentelemetry.sdk.internal.Signal;
 import io.opentelemetry.sdk.internal.StandardComponentId;
+import java.net.URI;
+import java.net.URISyntaxException;
 import java.util.function.Supplier;
 import javax.annotation.Nullable;
 
@@ -42,16 +45,40 @@ public ExporterInstrumentation(
             signal == Signal.PROFILE
                 ? NoopExporterMetrics.INSTANCE
                 : new SemConvExporterMetrics(
-                    meterProviderSupplier,
-                    signal,
-                    componentId,
-                    ServerAttributesUtil.extractServerAttributes(endpoint));
+                    meterProviderSupplier, signal, componentId, extractServerAttributes(endpoint));
         break;
       default:
         throw new IllegalStateException("Unhandled case: " + schema);
     }
   }
 
+  // visible for testing
+  static Attributes extractServerAttributes(String httpEndpoint) {
+    try {
+      URI parsed = new URI(httpEndpoint);
+      AttributesBuilder builder = Attributes.builder();
+      String host = parsed.getHost();
+      if (host != null) {
+        builder.put(SemConvAttributes.SERVER_ADDRESS, host);
+      }
+      int port = parsed.getPort();
+      if (port == -1) {
+        String scheme = parsed.getScheme();
+        if ("https".equals(scheme)) {
+          port = 443;
+        } else if ("http".equals(scheme)) {
+          port = 80;
+        }
+      }
+      if (port != -1) {
+        builder.put(SemConvAttributes.SERVER_PORT, port);
+      }
+      return builder.build();
+    } catch (URISyntaxException e) {
+      return Attributes.empty();
+    }
+  }
+
   public Recording startRecordingExport(int itemCount) {
     return new Recording(implementation.startRecordingExport(itemCount));
   }

exporters/common/src/main/java/io/opentelemetry/exporter/internal/metrics/ServerAttributesUtil.java

@@ -12,6 +12,7 @@
 
 import io.opentelemetry.api.common.Attributes;
 import io.opentelemetry.exporter.internal.marshal.Marshaler;
+import io.opentelemetry.internal.testing.slf4j.SuppressLogger;
 import io.opentelemetry.sdk.common.InternalTelemetryVersion;
 import io.opentelemetry.sdk.internal.ComponentId;
 import io.opentelemetry.sdk.internal.SemConvAttributes;

exporters/common/src/test/java/io/opentelemetry/exporter/internal/grpc/GrpcExporterTest.java

@@ -13,6 +13,7 @@
 
 import io.opentelemetry.api.common.Attributes;
 import io.opentelemetry.exporter.internal.marshal.Marshaler;
+import io.opentelemetry.internal.testing.slf4j.SuppressLogger;
 import io.opentelemetry.sdk.common.InternalTelemetryVersion;
 import io.opentelemetry.sdk.internal.ComponentId;
 import io.opentelemetry.sdk.internal.SemConvAttributes;
@@ -208,4 +209,30 @@ void testInternalTelemetry(StandardComponentId.ExporterType exporterType) {
                                           .hasBucketCounts(1))));
     }
   }
+
+  private static class FakeHttpResponse implements HttpSender.Response {
+
+    final int statusCode;
+    final String statusMessage;
+
+    FakeHttpResponse(int statusCode, String statusMessage) {
+      this.statusCode = statusCode;
+      this.statusMessage = statusMessage;
+    }
+
+    @Override
+    public int statusCode() {
+      return statusCode;
+    }
+
+    @Override
+    public String statusMessage() {
+      return statusMessage;
+    }
+
+    @Override
+    public byte[] responseBody() throws IOException {
+      return new byte[0];
+    }
+  }
 }

exporters/common/src/test/java/io/opentelemetry/exporter/internal/http/FakeHttpResponse.java

@@ -5,6 +5,7 @@
 
 package io.opentelemetry.exporter.internal.metrics;
 
+import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.assertThat;
 import static org.mockito.Mockito.atLeastOnce;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.verify;
@@ -16,13 +17,15 @@
 import io.opentelemetry.sdk.common.CompletableResultCode;
 import io.opentelemetry.sdk.common.InternalTelemetryVersion;
 import io.opentelemetry.sdk.internal.ComponentId;
+import io.opentelemetry.sdk.internal.SemConvAttributes;
 import io.opentelemetry.sdk.internal.StandardComponentId;
 import io.opentelemetry.sdk.metrics.InstrumentType;
 import io.opentelemetry.sdk.metrics.SdkMeterProvider;
 import io.opentelemetry.sdk.metrics.data.AggregationTemporality;
 import io.opentelemetry.sdk.metrics.export.CollectionRegistration;
 import io.opentelemetry.sdk.metrics.export.MetricReader;
 import java.util.function.Supplier;
+import org.junit.jupiter.api.Test;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.EnumSource;
 import org.mockito.Mockito;
@@ -103,4 +106,54 @@ void noopMeterProvider(InternalTelemetryVersion schemaVersion) {
     instrumentation.startRecordingExport(42).finishSuccessful();
     verify(meterProviderSupplier, atLeastOnce()).get();
   }
+
+  @Test
+  void serverAttributesInvalidUrl() {
+    assertThat(ExporterInstrumentation.extractServerAttributes("^")).isEmpty();
+  }
+
+  @Test
+  void serverAttributesEmptyUrl() {
+    assertThat(ExporterInstrumentation.extractServerAttributes("")).isEmpty();
+  }
+
+  @Test
+  void serverAttributesHttps() {
+    assertThat(ExporterInstrumentation.extractServerAttributes("https://example.com/foo/bar?a=b"))
+        .hasSize(2)
+        .containsEntry(SemConvAttributes.SERVER_ADDRESS, "example.com")
+        .containsEntry(SemConvAttributes.SERVER_PORT, 443);
+
+    assertThat(
+            ExporterInstrumentation.extractServerAttributes("https://example.com:1234/foo/bar?a=b"))
+        .hasSize(2)
+        .containsEntry(SemConvAttributes.SERVER_ADDRESS, "example.com")
+        .containsEntry(SemConvAttributes.SERVER_PORT, 1234);
+  }
+
+  @Test
+  void serverAttributesHttp() {
+    assertThat(ExporterInstrumentation.extractServerAttributes("http://example.com/foo/bar?a=b"))
+        .hasSize(2)
+        .containsEntry(SemConvAttributes.SERVER_ADDRESS, "example.com")
+        .containsEntry(SemConvAttributes.SERVER_PORT, 80);
+
+    assertThat(
+            ExporterInstrumentation.extractServerAttributes("http://example.com:1234/foo/bar?a=b"))
+        .hasSize(2)
+        .containsEntry(SemConvAttributes.SERVER_ADDRESS, "example.com")
+        .containsEntry(SemConvAttributes.SERVER_PORT, 1234);
+  }
+
+  @Test
+  void serverAttributesUnknownScheme() {
+    assertThat(ExporterInstrumentation.extractServerAttributes("custom://foo"))
+        .hasSize(1)
+        .containsEntry(SemConvAttributes.SERVER_ADDRESS, "foo");
+
+    assertThat(ExporterInstrumentation.extractServerAttributes("custom://foo:1234"))
+        .hasSize(2)
+        .containsEntry(SemConvAttributes.SERVER_ADDRESS, "foo")
+        .containsEntry(SemConvAttributes.SERVER_PORT, 1234);
+  }
 }