Redact query string values for http client spans (#13114)

Co-authored-by: Steve Rao <raozihao.rzh@alibaba-inc.com>
Co-authored-by: Lauri Tulmin <tulmin@gmail.com>
Co-authored-by: Lauri Tulmin <ltulmin@splunk.com>
Co-authored-by: Trask Stalnaker <trask.stalnaker@gmail.com>

instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/builder/internal/DefaultHttpClientInstrumenterBuilder.java

@@ -20,6 +20,7 @@ import io.opentelemetry.instrumentation.api.instrumenter.InstrumenterBuilder;
 import io.opentelemetry.instrumentation.api.instrumenter.SpanKindExtractor;
 import io.opentelemetry.instrumentation.api.instrumenter.SpanNameExtractor;
 import io.opentelemetry.instrumentation.api.instrumenter.SpanStatusExtractor;
+import io.opentelemetry.instrumentation.api.internal.Experimental;
 import io.opentelemetry.instrumentation.api.semconv.http.HttpClientAttributesExtractor;
 import io.opentelemetry.instrumentation.api.semconv.http.HttpClientAttributesExtractorBuilder;
 import io.opentelemetry.instrumentation.api.semconv.http.HttpClientAttributesGetter;
@@ -177,6 +178,18 @@ public final class DefaultHttpClientInstrumenterBuilder<REQUEST, RESPONSE> {
     return this;
   }
 
+  /**
+   * Configures the instrumentation to redact sensitive URL parameters.
+   *
+   * @param redactQueryParameters {@code true} if the sensitive URL parameters have to be redacted.
+   */
+  @CanIgnoreReturnValue
+  public DefaultHttpClientInstrumenterBuilder<REQUEST, RESPONSE> setRedactQueryParameters(
+      boolean redactQueryParameters) {
+    Experimental.setRedactQueryParameters(httpAttributesExtractorBuilder, redactQueryParameters);
+    return this;
+  }
+
   /** Sets custom {@link SpanNameExtractor} via transform function. */
   @CanIgnoreReturnValue
   public DefaultHttpClientInstrumenterBuilder<REQUEST, RESPONSE> setSpanNameExtractor(
@@ -225,6 +238,7 @@ public final class DefaultHttpClientInstrumenterBuilder<REQUEST, RESPONSE> {
           .addAttributesExtractor(HttpExperimentalAttributesExtractor.create(attributesGetter))
           .addOperationMetrics(HttpClientExperimentalMetrics.get());
     }
+
     builderCustomizer.accept(builder);
 
     if (headerSetter != null) {
@@ -248,6 +262,7 @@ public final class DefaultHttpClientInstrumenterBuilder<REQUEST, RESPONSE> {
     set(
         config::shouldEmitExperimentalHttpClientTelemetry,
         this::setEmitExperimentalHttpClientMetrics);
+    set(config::redactQueryParameters, this::setRedactQueryParameters);
     return this;
   }
 

instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/config/internal/CommonConfig.java

@@ -31,6 +31,7 @@ public final class CommonConfig {
   private final boolean statementSanitizationEnabled;
   private final boolean emitExperimentalHttpClientTelemetry;
   private final boolean emitExperimentalHttpServerTelemetry;
+  private final boolean redactQueryParameters;
   private final String loggingTraceIdKey;
   private final String loggingSpanIdKey;
   private final String loggingTraceFlagsKey;
@@ -57,6 +58,9 @@ public final class CommonConfig {
         config.getBoolean("otel.instrumentation.common.db-statement-sanitizer.enabled", true);
     emitExperimentalHttpClientTelemetry =
         config.getBoolean("otel.instrumentation.http.client.emit-experimental-telemetry", false);
+    redactQueryParameters =
+        config.getBoolean(
+            "otel.instrumentation.http.client.experimental.redact-query-parameters", true);
     emitExperimentalHttpServerTelemetry =
         config.getBoolean("otel.instrumentation.http.server.emit-experimental-telemetry", false);
     enduserConfig = new EnduserConfig(config);
@@ -111,6 +115,10 @@ public final class CommonConfig {
     return emitExperimentalHttpServerTelemetry;
   }
 
+  public boolean redactQueryParameters() {
+    return redactQueryParameters;
+  }
+
   public String getTraceIdKey() {
     return loggingTraceIdKey;
   }

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

@@ -0,0 +1,36 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.instrumentation.api.internal;
+
+import io.opentelemetry.instrumentation.api.semconv.http.HttpClientAttributesExtractorBuilder;
+import java.util.function.BiConsumer;
+import javax.annotation.Nullable;
+
+/**
+ * This class is internal and is hence not for public use. Its APIs are unstable and can change at
+ * any time.
+ */
+public final class Experimental {
+
+  @Nullable
+  private static volatile BiConsumer<HttpClientAttributesExtractorBuilder<?, ?>, Boolean>
+      redactHttpClientQueryParameters;
+
+  private Experimental() {}
+
+  public static void setRedactQueryParameters(
+      HttpClientAttributesExtractorBuilder<?, ?> builder, boolean redactQueryParameters) {
+    if (redactHttpClientQueryParameters != null) {
+      redactHttpClientQueryParameters.accept(builder, redactQueryParameters);
+    }
+  }
+
+  public static void internalSetRedactHttpClientQueryParameters(
+      BiConsumer<HttpClientAttributesExtractorBuilder<?, ?>, Boolean>
+          redactHttpClientQueryParameters) {
+    Experimental.redactHttpClientQueryParameters = redactHttpClientQueryParameters;
+  }
+}

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

@@ -17,6 +17,9 @@ import io.opentelemetry.instrumentation.api.semconv.network.internal.InternalNet
 import io.opentelemetry.instrumentation.api.semconv.network.internal.InternalServerAttributesExtractor;
 import io.opentelemetry.semconv.HttpAttributes;
 import io.opentelemetry.semconv.UrlAttributes;
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.Set;
 import java.util.function.ToIntFunction;
 import javax.annotation.Nullable;
 
@@ -32,6 +35,9 @@ public final class HttpClientAttributesExtractor<REQUEST, RESPONSE>
         REQUEST, RESPONSE, HttpClientAttributesGetter<REQUEST, RESPONSE>>
     implements SpanKeyProvider {
 
+  private static final Set<String> PARAMS_TO_REDACT =
+      new HashSet<>(Arrays.asList("AWSAccessKeyId", "Signature", "sig", "X-Goog-Signature"));
+
   /**
    * Creates the HTTP client attributes extractor with default configuration.
    *
@@ -54,6 +60,7 @@ public final class HttpClientAttributesExtractor<REQUEST, RESPONSE>
   private final InternalNetworkAttributesExtractor<REQUEST, RESPONSE> internalNetworkExtractor;
   private final InternalServerAttributesExtractor<REQUEST> internalServerExtractor;
   private final ToIntFunction<Context> resendCountIncrementer;
+  private final boolean redactQueryParameters;
 
   HttpClientAttributesExtractor(HttpClientAttributesExtractorBuilder<REQUEST, RESPONSE> builder) {
     super(
@@ -65,6 +72,7 @@ public final class HttpClientAttributesExtractor<REQUEST, RESPONSE>
     internalNetworkExtractor = builder.buildNetworkExtractor();
     internalServerExtractor = builder.buildServerExtractor();
     resendCountIncrementer = builder.resendCountIncrementer;
+    redactQueryParameters = builder.redactQueryParameters;
   }
 
   @Override
@@ -104,11 +112,21 @@ public final class HttpClientAttributesExtractor<REQUEST, RESPONSE>
   }
 
   @Nullable
-  private static String stripSensitiveData(@Nullable String url) {
+  private String stripSensitiveData(@Nullable String url) {
     if (url == null || url.isEmpty()) {
       return url;
     }
 
+    url = redactUserInfo(url);
+
+    if (redactQueryParameters) {
+      url = redactQueryParameters(url);
+    }
+
+    return url;
+  }
+
+  private static String redactUserInfo(String url) {
     int schemeEndIndex = url.indexOf(':');
 
     if (schemeEndIndex == -1) {
@@ -145,4 +163,57 @@ public final class HttpClientAttributesExtractor<REQUEST, RESPONSE>
     }
     return url.substring(0, schemeEndIndex + 3) + "REDACTED:REDACTED" + url.substring(atIndex);
   }
+
+  private static String redactQueryParameters(String url) {
+    int questionMarkIndex = url.indexOf('?');
+    if (questionMarkIndex == -1 || !containsParamToRedact(url)) {
+      return url;
+    }
+
+    StringBuilder urlAfterQuestionMark = new StringBuilder();
+
+    // To build a parameter name until we reach the '=' character
+    // If the parameter name is a one to redact, we will redact the value
+    StringBuilder currentParamName = new StringBuilder();
+
+    for (int i = questionMarkIndex + 1; i < url.length(); i++) {
+      char currentChar = url.charAt(i);
+
+      if (currentChar == '=') {
+        urlAfterQuestionMark.append('=');
+        if (PARAMS_TO_REDACT.contains(currentParamName.toString())) {
+          urlAfterQuestionMark.append("REDACTED");
+          // skip over parameter value
+          for (; i + 1 < url.length(); i++) {
+            char c = url.charAt(i + 1);
+            if (c == '&' || c == '#') {
+              break;
+            }
+          }
+        }
+      } else if (currentChar == '&') { // New parameter delimiter
+        urlAfterQuestionMark.append(currentChar);
+        // To avoid creating a new StringBuilder for each new parameter
+        currentParamName.setLength(0);
+      } else if (currentChar == '#') { // Reference delimiter
+        urlAfterQuestionMark.append(url.substring(i));
+        break;
+      } else {
+        // param values can be appended to currentParamName here but it's not an issue
+        currentParamName.append(currentChar);
+        urlAfterQuestionMark.append(currentChar);
+      }
+    }
+
+    return url.substring(0, questionMarkIndex) + "?" + urlAfterQuestionMark;
+  }
+
+  private static boolean containsParamToRedact(String urlpart) {
+    for (String param : PARAMS_TO_REDACT) {
+      if (urlpart.contains(param)) {
+        return true;
+      }
+    }
+    return false;
+  }
 }

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

@@ -11,6 +11,7 @@ import com.google.errorprone.annotations.CanIgnoreReturnValue;
 import io.opentelemetry.context.Context;
 import io.opentelemetry.instrumentation.api.instrumenter.AttributesExtractor;
 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.network.internal.AddressAndPortExtractor;
 import io.opentelemetry.instrumentation.api.semconv.network.internal.InternalNetworkAttributesExtractor;
@@ -37,6 +38,12 @@ public final class HttpClientAttributesExtractorBuilder<REQUEST, RESPONSE> {
   List<String> capturedResponseHeaders = emptyList();
   Set<String> knownMethods = HttpConstants.KNOWN_METHODS;
   ToIntFunction<Context> resendCountIncrementer = HttpClientRequestResendCount::getAndIncrement;
+  boolean redactQueryParameters;
+
+  static {
+    Experimental.internalSetRedactHttpClientQueryParameters(
+        (builder, redact) -> builder.redactQueryParameters = redact);
+  }
 
   HttpClientAttributesExtractorBuilder(
       HttpClientAttributesGetter<REQUEST, RESPONSE> httpAttributesGetter) {

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

@@ -23,12 +23,14 @@ import static java.util.Collections.emptyList;
 import static java.util.Collections.emptyMap;
 import static java.util.Collections.singletonList;
 import static org.assertj.core.api.Assertions.entry;
+import static org.junit.jupiter.params.provider.Arguments.arguments;
 
 import io.opentelemetry.api.common.AttributeKey;
 import io.opentelemetry.api.common.Attributes;
 import io.opentelemetry.api.common.AttributesBuilder;
 import io.opentelemetry.context.Context;
 import io.opentelemetry.instrumentation.api.instrumenter.AttributesExtractor;
+import io.opentelemetry.instrumentation.api.internal.Experimental;
 import io.opentelemetry.instrumentation.api.internal.HttpConstants;
 import java.net.ConnectException;
 import java.util.HashMap;
@@ -36,9 +38,13 @@ import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.function.ToIntFunction;
+import java.util.stream.Stream;
 import javax.annotation.Nullable;
 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;
 import org.junit.jupiter.params.provider.ValueSource;
 
@@ -200,6 +206,93 @@ class HttpClientAttributesExtractorTest {
             entry(NETWORK_PEER_PORT, 456L));
   }
 
+  @ParameterizedTest
+  @ArgumentsSource(UrlSourceToRedact.class)
+  void shouldRedactUserInfoAndQueryParameters(String url, String expectedResult) {
+    Map<String, String> request = new HashMap<>();
+    request.put("urlFull", url);
+
+    HttpClientAttributesExtractorBuilder<Map<String, String>, Map<String, String>> builder =
+        HttpClientAttributesExtractor.builder(new TestHttpClientAttributesGetter());
+    Experimental.setRedactQueryParameters(builder, true);
+    AttributesExtractor<Map<String, String>, Map<String, String>> extractor = builder.build();
+
+    AttributesBuilder attributes = Attributes.builder();
+    extractor.onStart(attributes, Context.root(), request);
+
+    assertThat(attributes.build()).containsOnly(entry(URL_FULL, expectedResult));
+  }
+
+  static final class UrlSourceToRedact implements ArgumentsProvider {
+
+    @Override
+    public Stream<? extends Arguments> provideArguments(ExtensionContext context) {
+      return Stream.of(
+          arguments("https://user1:secret@github.com", "https://REDACTED:REDACTED@github.com"),
+          arguments(
+              "https://user1:secret@github.com/path/",
+              "https://REDACTED:REDACTED@github.com/path/"),
+          arguments(
+              "https://user1:secret@github.com#test.html",
+              "https://REDACTED:REDACTED@github.com#test.html"),
+          arguments(
+              "https://user1:secret@github.com?foo=b@r",
+              "https://REDACTED:REDACTED@github.com?foo=b@r"),
+          arguments(
+              "https://user1:secret@github.com/p@th?foo=b@r",
+              "https://REDACTED:REDACTED@github.com/p@th?foo=b@r"),
+          arguments("https://github.com/p@th?foo=b@r", "https://github.com/p@th?foo=b@r"),
+          arguments("https://github.com#t@st.html", "https://github.com#t@st.html"),
+          arguments("user1:secret@github.com", "user1:secret@github.com"),
+          arguments("https://github.com@", "https://github.com@"),
+          arguments(
+              "https://service.com?paramA=valA&paramB=valB",
+              "https://service.com?paramA=valA&paramB=valB"),
+          arguments(
+              "https://service.com?AWSAccessKeyId=AKIAIOSFODNN7",
+              "https://service.com?AWSAccessKeyId=REDACTED"),
+          arguments(
+              "https://service.com?Signature=39Up9jzHkxhuIhFE9594DJxe7w6cIRCg0V6ICGS0%3A377",
+              "https://service.com?Signature=REDACTED"),
+          arguments(
+              "https://service.com?sig=39Up9jzHkxhuIhFE9594DJxe7w6cIRCg0V6ICGS0",
+              "https://service.com?sig=REDACTED"),
+          arguments(
+              "https://service.com?X-Goog-Signature=39Up9jzHkxhuIhFE9594DJxe7w6cIRCg0V6ICGS0",
+              "https://service.com?X-Goog-Signature=REDACTED"),
+          arguments(
+              "https://service.com?paramA=valA&AWSAccessKeyId=AKIAIOSFODNN7&paramB=valB",
+              "https://service.com?paramA=valA&AWSAccessKeyId=REDACTED&paramB=valB"),
+          arguments(
+              "https://service.com?AWSAccessKeyId=AKIAIOSFODNN7&paramA=valA",
+              "https://service.com?AWSAccessKeyId=REDACTED&paramA=valA"),
+          arguments(
+              "https://service.com?paramA=valA&AWSAccessKeyId=AKIAIOSFODNN7",
+              "https://service.com?paramA=valA&AWSAccessKeyId=REDACTED"),
+          arguments(
+              "https://service.com?AWSAccessKeyId=AKIAIOSFODNN7&AWSAccessKeyId=ZGIAIOSFODNN7",
+              "https://service.com?AWSAccessKeyId=REDACTED&AWSAccessKeyId=REDACTED"),
+          arguments(
+              "https://service.com?AWSAccessKeyId=AKIAIOSFODNN7#ref",
+              "https://service.com?AWSAccessKeyId=REDACTED#ref"),
+          arguments(
+              "https://service.com?AWSAccessKeyId=AKIAIOSFODNN7&aa&bb",
+              "https://service.com?AWSAccessKeyId=REDACTED&aa&bb"),
+          arguments(
+              "https://service.com?aa&bb&AWSAccessKeyId=AKIAIOSFODNN7",
+              "https://service.com?aa&bb&AWSAccessKeyId=REDACTED"),
+          arguments(
+              "https://service.com?AWSAccessKeyId=AKIAIOSFODNN7&&",
+              "https://service.com?AWSAccessKeyId=REDACTED&&"),
+          arguments(
+              "https://service.com?&&AWSAccessKeyId=AKIAIOSFODNN7",
+              "https://service.com?&&AWSAccessKeyId=REDACTED"),
+          arguments(
+              "https://service.com?AWSAccessKeyId=AKIAIOSFODNN7&a&b#fragment",
+              "https://service.com?AWSAccessKeyId=REDACTED&a&b#fragment"));
+    }
+  }
+
   @ParameterizedTest
   @ArgumentsSource(ValidRequestMethodsProvider.class)
   void shouldExtractKnownMethods(String requestMethod) {

instrumentation/spring/spring-boot-autoconfigure/src/main/resources/META-INF/additional-spring-configuration-metadata.json

@@ -311,6 +311,12 @@
       "description": "Enable the capture of experimental HTTP client telemetry. Add the <code>http.request.body.size</code> and <code>http.response.body.size> attributes to spans, and record the <code>http.client.request.size</code> and <code>http.client.response.size</code> metrics.",
       "defaultValue": false
     },
+    {
+      "name": "otel.instrumentation.http.client.experimental.redact-query-parameters",
+      "type": "java.lang.Boolean",
+      "description": "Redact sensitive URL parameters. See https://opentelemetry.io/docs/specs/semconv/http/http-spans.",
+      "defaultValue": true
+    },
     {
       "name": "otel.instrumentation.http.known-methods",
       "type": "java.util.List<java.lang.String>",

smoke-tests-otel-starter/spring-boot-common/src/main/java/io/opentelemetry/spring/smoketest/AbstractOtelSpringStarterSmokeTest.java

@@ -293,4 +293,23 @@ class AbstractOtelSpringStarterSmokeTest extends AbstractSpringStarterSmokeTest
                     span.hasKind(SpanKind.SERVER).hasAttribute(HttpAttributes.HTTP_ROUTE, "/ping"),
                 span -> withSpanAssert(span)));
   }
+
+  @Test
+  void shouldRedactSomeUrlParameters() {
+    testing.clearAllExportedData();
+
+    RestTemplate restTemplate = restTemplateBuilder.rootUri("http://localhost:" + port).build();
+    restTemplate.getForObject(
+        "/test?X-Goog-Signature=39Up9jzHkxhuIhFE9594DJxe7w6cIRCg0V6ICGS0", String.class);
+
+    testing.waitAndAssertTraces(
+        traceAssert ->
+            traceAssert.hasSpansSatisfyingExactly(
+                span ->
+                    HttpSpanDataAssert.create(span)
+                        .assertClientGetRequest("/test?X-Goog-Signature=REDACTED"),
+                span ->
+                    span.hasKind(SpanKind.SERVER)
+                        .hasAttribute(HttpAttributes.HTTP_ROUTE, "/test")));
+  }
 }

smoke-tests-otel-starter/spring-boot-common/src/main/java/io/opentelemetry/spring/smoketest/OtelSpringStarterSmokeTestController.java

@@ -15,6 +15,7 @@ import org.springframework.web.bind.annotation.RestController;
 public class OtelSpringStarterSmokeTestController {
 
   public static final String PING = "/ping";
+  public static final String TEST = "/test";
   public static final String TEST_HISTOGRAM = "histogram-test-otel-spring-starter";
   public static final String METER_SCOPE_NAME = "scope";
   private final LongHistogram histogram;
@@ -33,4 +34,9 @@ public class OtelSpringStarterSmokeTestController {
     component.withSpanMethod("from-controller");
     return "pong";
   }
+
+  @GetMapping(TEST)
+  public String testUrlToRedact() {
+    return "ok";
+  }
 }