Optimize tomcat uri construction

instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/internal/UriBuilder.java

@@ -0,0 +1,53 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.instrumentation.api.internal;
+
+import org.checkerframework.checker.nullness.qual.Nullable;
+
+// internal until decisions made on
+// https://github.com/open-telemetry/opentelemetry-java-instrumentation/issues/3700
+public class UriBuilder {
+
+  // TODO (trask) investigate and document implications of URI encoding, see
+  // https://github.com/open-telemetry/opentelemetry-java-instrumentation/pull/4008#discussion_r698027851
+  //
+  // note: currently path must be empty or start with "/" but that can be relaxed if needed
+  public static String uri(
+      String scheme, String host, int serverPort, String path, @Nullable String query) {
+
+    boolean isDefaultPort =
+        (scheme.equals("http") && serverPort == 80)
+            || (scheme.equals("https") && serverPort == 443);
+
+    // +3 is space for "://"
+    int length = scheme.length() + 3 + host.length() + path.length();
+    if (!isDefaultPort && serverPort != -1) {
+      // +6 is space for ":" and max port number (65535)
+      length += 6;
+    }
+    if (query != null) {
+      // the +1 is space for "?"
+      length += 1 + query.length();
+    }
+
+    StringBuilder url = new StringBuilder(length);
+    url.append(scheme);
+    url.append("://");
+    url.append(host);
+    if (!isDefaultPort && serverPort != -1) {
+      url.append(':');
+      url.append(serverPort);
+    }
+    url.append(path);
+    if (query != null) {
+      url.append('?');
+      url.append(query);
+    }
+    return url.toString();
+  }
+
+  private UriBuilder() {}
+}

instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/internal/UriBuilderTest.java

@@ -0,0 +1,63 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.instrumentation.api.internal;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.net.URI;
+import java.net.URISyntaxException;
+import java.util.stream.Stream;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtensionContext;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.ArgumentsProvider;
+import org.junit.jupiter.params.provider.ArgumentsSource;
+
+public class UriBuilderTest {
+
+  @ParameterizedTest
+  @ArgumentsSource(Parameters.class)
+  public void test(String scheme, String host, int port, String path, String query)
+      throws URISyntaxException {
+
+    assertThat(UriBuilder.uri(scheme, host, port, path, query))
+        .isEqualTo(new URI(scheme, null, host, port, path, query, null).toString());
+  }
+
+  // can't use parameterized test above because URI.toString() encodes the port when it is supplied,
+  // even it's the default port
+  @Test
+  public void testHttpDefaultPort() {
+    assertThat(UriBuilder.uri("http", "myhost", 80, "/mypath", "myquery"))
+        .isEqualTo("http://myhost/mypath?myquery");
+  }
+
+  // can't use parameterized test above because URI.toString() encodes the port when it is supplied,
+  // even it's the default port
+  @Test
+  public void testHttpsDefaultPort() {
+    assertThat(UriBuilder.uri("https", "myhost", 443, "/mypath", "myquery"))
+        .isEqualTo("https://myhost/mypath?myquery");
+  }
+
+  private static class Parameters implements ArgumentsProvider {
+    @Override
+    public Stream<? extends Arguments> provideArguments(ExtensionContext context) {
+      return Stream.of(
+          Arguments.of("http", "myhost", -1, "/mypath", "myquery"), // test default http port
+          Arguments.of("http", "myhost", 8080, "/mypath", "myquery"), // test non-default http port
+          Arguments.of("https", "myhost", -1, "/mypath", "myquery"), // test default https port
+          Arguments.of(
+              "https", "myhost", 8443, "/mypath", "myquery"), // test non-default https port
+          Arguments.of("http", "myhost", -1, "/", "myquery"), // test root path
+          Arguments.of("http", "myhost", -1, "", "myquery"), // test empty path
+          Arguments.of("http", "myhost", -1, "/mypath", ""), // test empty query string
+          Arguments.of("http", "myhost", -1, "/mypath", null) // test null query string
+          );
+    }
+  }
+}

instrumentation/tomcat/tomcat-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/tomcat/common/TomcatTracer.java

@@ -9,10 +9,10 @@
 
 import io.opentelemetry.context.Context;
 import io.opentelemetry.context.propagation.TextMapGetter;
+import io.opentelemetry.instrumentation.api.internal.UriBuilder;
 import io.opentelemetry.instrumentation.api.servlet.AppServerBridge;
 import io.opentelemetry.instrumentation.api.servlet.ServerSpanNaming;
 import io.opentelemetry.instrumentation.api.tracer.HttpServerTracer;
-import java.net.URI;
 import java.util.Collections;
 import org.apache.coyote.ActionCode;
 import org.apache.coyote.Request;
@@ -88,19 +88,7 @@ protected String url(Request request) {
     String path = request.requestURI().toString();
     String query = request.queryString().toString();
 
-    try {
-      return new URI(scheme, null, host, serverPort, path, query, null).toString();
-    } catch (Exception e) {
-      logger.warn(
-          "Malformed url? scheme: {}, host: {}, port: {}, path: {}, query: {}",
-          scheme,
-          host,
-          serverPort,
-          path,
-          query,
-          e);
-    }
-    return null;
+    return UriBuilder.uri(scheme, host, serverPort, path, query);
   }
 
   @Override