From 9cd36289a8be6fa359e8b31f932330e43808b92d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gast=C3=B3n=20Panizza?= Date: Thu, 16 Feb 2023 15:26:45 -0300 Subject: [PATCH 1/6] Add Headers as Attributes for Otel traces --- pom.xml | 2 +- .../listener/server/HttpListenerConfig.java | 19 +++++++ .../http/internal/listener/HttpListener.java | 2 +- .../HttpListenerCurrentSpanCustomizer.java | 43 ++++++++++++--- .../http/internal/request/HttpRequester.java | 3 +- .../internal/request/HttpRequesterConfig.java | 20 +++++++ .../internal/request/TracingSettings.java | 55 +++++++++++++++++++ .../tracing/HttpCurrentSpanCustomizer.java | 7 +++ .../HttpRequestCurrentSpanCustomizer.java | 35 +++++++++++- .../HttpCurrentSpanCustomizerTestCase.java | 8 ++- 10 files changed, 178 insertions(+), 16 deletions(-) create mode 100644 src/main/java/org/mule/extension/http/internal/request/TracingSettings.java diff --git a/pom.xml b/pom.xml index 79e1d7c92..a282f3c23 100644 --- a/pom.xml +++ b/pom.xml @@ -15,7 +15,7 @@ org.mule.connectors mule-http-connector mule-extension - 1.8.0-SNAPSHOT + 1.8.1-SNAPSHOT HTTP Connector A Mule extension that provides HTTP server and client functionality diff --git a/src/main/java/org/mule/extension/http/api/listener/server/HttpListenerConfig.java b/src/main/java/org/mule/extension/http/api/listener/server/HttpListenerConfig.java index 4ca26d55f..bb4d05cc4 100644 --- a/src/main/java/org/mule/extension/http/api/listener/server/HttpListenerConfig.java +++ b/src/main/java/org/mule/extension/http/api/listener/server/HttpListenerConfig.java @@ -9,6 +9,7 @@ import static java.util.Optional.empty; import static java.util.Optional.of; import static org.mule.runtime.api.meta.ExpressionSupport.NOT_SUPPORTED; +import static org.mule.runtime.api.meta.ExpressionSupport.SUPPORTED; import static org.mule.runtime.api.util.Preconditions.checkArgument; import org.mule.extension.http.api.listener.headers.HttpHeadersException; @@ -66,12 +67,25 @@ public class HttpListenerConfig implements Initialisable { @Expression(NOT_SUPPORTED) private boolean rejectInvalidTransferEncoding; + /** + * W-12558102 + * Indicates what headers should not be exported as attributes when generating Open Telemetry traces. By default, common headers associated with credentials are skipped. + * + * @since 1.8.0 + */ + @Parameter + @Optional(defaultValue = "client_id, client_secret, Authorization") + @Expression(SUPPORTED) + private String skipHeadersOnTracing; + private HttpHeadersValidator httpHeaderValidators; @Override public void initialise() throws InitialisationException { basePath = sanitizePathWithStartSlash(this.basePath); httpHeaderValidators = new InvalidTransferEncodingValidator(rejectInvalidTransferEncoding); + //W-12558102 + skipHeadersOnTracing = this.skipHeadersOnTracing; } public ListenerPath getFullListenerPath(String listenerPath) { @@ -99,4 +113,9 @@ public java.util.Optional getInterceptor() { public void validateHeaders(MultiMap headers) throws HttpHeadersException { httpHeaderValidators.validateHeaders(headers); } + + //W-12558102 + public String getSkipHeadersOnTracing() { + return skipHeadersOnTracing; + } } diff --git a/src/main/java/org/mule/extension/http/internal/listener/HttpListener.java b/src/main/java/org/mule/extension/http/internal/listener/HttpListener.java index 0da45c500..3f7d9addc 100644 --- a/src/main/java/org/mule/extension/http/internal/listener/HttpListener.java +++ b/src/main/java/org/mule/extension/http/internal/listener/HttpListener.java @@ -441,7 +441,7 @@ public void handleRequest(HttpRequestContext requestContext, HttpResponseReadyCa forwardCompatibilityHelper.get().getDistributedTraceContextManager(context); distributedTraceContextManager.setRemoteTraceContextMap(headers); getHttpListenerCurrentSpanCustomizer(result.getAttributes().get(), server.getServerAddress().getIp(), - server.getServerAddress().getPort()) + server.getServerAddress().getPort(), config.getSkipHeadersOnTracing()) .customizeSpan(distributedTraceContextManager); } diff --git a/src/main/java/org/mule/extension/http/internal/listener/profiling/tracing/HttpListenerCurrentSpanCustomizer.java b/src/main/java/org/mule/extension/http/internal/listener/profiling/tracing/HttpListenerCurrentSpanCustomizer.java index 9934327c2..647c4a060 100644 --- a/src/main/java/org/mule/extension/http/internal/listener/profiling/tracing/HttpListenerCurrentSpanCustomizer.java +++ b/src/main/java/org/mule/extension/http/internal/listener/profiling/tracing/HttpListenerCurrentSpanCustomizer.java @@ -15,9 +15,14 @@ import org.mule.extension.http.api.HttpRequestAttributes; import org.mule.extension.http.internal.request.profiling.tracing.HttpCurrentSpanCustomizer; import org.mule.sdk.api.runtime.source.DistributedTraceContextManager; +import org.mule.runtime.api.util.MultiMap; +import com.google.common.collect.ArrayListMultimap; +import com.google.common.collect.ListMultimap; import java.net.URI; import java.net.URISyntaxException; +import java.util.List; +import java.util.Arrays; import org.slf4j.Logger; @@ -41,16 +46,24 @@ public class HttpListenerCurrentSpanCustomizer extends HttpCurrentSpanCustomizer private final String host; private final int port; - private HttpListenerCurrentSpanCustomizer(HttpRequestAttributes attributes, String host, int port) { + //W-12558102 + private final List skipAttributesList; + + private HttpListenerCurrentSpanCustomizer(HttpRequestAttributes attributes, String host, int port, String skipAttributes) { this.attributes = attributes; this.host = host; this.port = port; + //W-12558102 + this.skipAttributesList = Arrays.asList(skipAttributes.split(",", -1)); + this.skipAttributesList.replaceAll(String::trim); } public static HttpCurrentSpanCustomizer getHttpListenerCurrentSpanCustomizer(HttpRequestAttributes attributes, String host, - int port) { - return new HttpListenerCurrentSpanCustomizer(attributes, host, port); + int port, + //W-... gp + String skipAttributes) { + return new HttpListenerCurrentSpanCustomizer(attributes, host, port, skipAttributes); } @Override @@ -62,11 +75,9 @@ public void customizeSpan(DistributedTraceContextManager distributedTraceContext distributedTraceContextManager.addCurrentSpanAttribute(NET_HOST_NAME, host); distributedTraceContextManager.addCurrentSpanAttribute(NET_HOST_PORT, valueOf(getURI().getPort())); distributedTraceContextManager.addCurrentSpanAttribute(HTTP_SCHEME, attributes.getScheme()); - String userAgent = attributes.getHeaders().get(USER_AGENT); - - if (userAgent != null) { - distributedTraceContextManager.addCurrentSpanAttribute(HTTP_USER_AGENT, userAgent); - } + //W-12558102 - Parsing HTTP headers as Span Attributes + MultiMap headers = getHeaders(); + headers.keySet().forEach(key -> distributedTraceContextManager.addCurrentSpanAttribute(key, headers.get(key))); } catch (Throwable e) { LOGGER.warn("Error on setting listener span attributes.", e); @@ -110,4 +121,20 @@ protected String getSpanKind() { protected String getSpanName() { return attributes.getListenerPath(); } + + /** + * W-12558102 + * This provides a transparent way to obtain the definitive list of headers to add in the spans associated with Otel tracing. + * This list will have all headers except those that have been skipped via the skipHeadersOnTracing property. + */ + @Override + public MultiMap getHeaders() { + MultiMap modifiedHeaders = new MultiMap(); + attributes.getHeaders().keySet().forEach(key -> { + if (!skipAttributesList.contains(key)) { + modifiedHeaders.put(key, attributes.getHeaders().get(key)); + } + }); + return modifiedHeaders; + } } diff --git a/src/main/java/org/mule/extension/http/internal/request/HttpRequester.java b/src/main/java/org/mule/extension/http/internal/request/HttpRequester.java index 055e78e80..17b6f4d94 100644 --- a/src/main/java/org/mule/extension/http/internal/request/HttpRequester.java +++ b/src/main/java/org/mule/extension/http/internal/request/HttpRequester.java @@ -147,7 +147,8 @@ public void doRequest(HttpExtensionClient client, HttpRequesterConfig config, St authentication, injectedHeaders, requestCreator, distributedTraceContextManager); - getHttpRequesterCurrentSpanCustomizer(httpRequester).customizeSpan(distributedTraceContextManager); + getHttpRequesterCurrentSpanCustomizer(httpRequester, config.getSkipHeadersOnTracing()) + .customizeSpan(distributedTraceContextManager); doRequestWithRetry(client, config, uri, method, streamingMode, sendBodyMode, followRedirects, authentication, resolvedTimeout, responseValidator, transformationService, requestCreator, checkRetry, muleContext, scheduler, diff --git a/src/main/java/org/mule/extension/http/internal/request/HttpRequesterConfig.java b/src/main/java/org/mule/extension/http/internal/request/HttpRequesterConfig.java index 9d6866d19..de98d18e9 100644 --- a/src/main/java/org/mule/extension/http/internal/request/HttpRequesterConfig.java +++ b/src/main/java/org/mule/extension/http/internal/request/HttpRequesterConfig.java @@ -50,6 +50,11 @@ public class HttpRequesterConfig implements Initialisable, HttpRequesterCookieCo @Placement(order = 3) private ResponseSettings responseSettings; + //W-12558102 + @ParameterGroup(name = "Tracing Settings") + @Placement(order = 4) + private TracingSettings tracingSettings; + @Inject private MuleContext muleContext; private CookieManager cookieManager; @@ -93,6 +98,11 @@ public Integer getResponseTimeout() { return responseSettings.getResponseTimeout(); } + //W-12558102 + public String getSkipHeadersOnTracing() { + return tracingSettings.getSkipHeadersOnTracing(); + } + @Override public boolean isEnableCookies() { return requestSettings.isEnableCookies(); @@ -116,6 +126,8 @@ public static class Builder { private RequestUrlConfiguration urlConfiguration; private RequestSettings requestSettings; private ResponseSettings responseSettings; + //W-12558102 + private TracingSettings tracingSettings; private MuleContext muleContext; private Builder() {} @@ -135,6 +147,12 @@ public Builder withResponseSettings(ResponseSettings responseSettings) { return this; } + //W-12558102 + public Builder withTracingSettings(TracingSettings tracingSettings) { + this.tracingSettings = tracingSettings; + return this; + } + public Builder withMuleContext(MuleContext muleContext) { this.muleContext = muleContext; return this; @@ -149,6 +167,8 @@ public HttpRequesterConfig build() { config.urlConfiguration = this.urlConfiguration; config.requestSettings = this.requestSettings; config.responseSettings = this.responseSettings; + //W-12558102 + config.tracingSettings = this.tracingSettings; config.muleContext = this.muleContext; return config; } diff --git a/src/main/java/org/mule/extension/http/internal/request/TracingSettings.java b/src/main/java/org/mule/extension/http/internal/request/TracingSettings.java new file mode 100644 index 000000000..5d2ca8b05 --- /dev/null +++ b/src/main/java/org/mule/extension/http/internal/request/TracingSettings.java @@ -0,0 +1,55 @@ +/* + * Copyright (c) MuleSoft, Inc. All rights reserved. http://www.mulesoft.com + * The software in this package is published under the terms of the CPAL v1.0 + * license, a copy of which has been included with this distribution in the + * LICENSE.txt file. + */ +package org.mule.extension.http.internal.request; + +import static org.mule.runtime.api.meta.ExpressionSupport.SUPPORTED; + +import org.mule.runtime.extension.api.annotation.Expression; +import org.mule.runtime.extension.api.annotation.param.Optional; +import org.mule.runtime.extension.api.annotation.param.Parameter; + +/** + * Groups parameters which configure how a request is traced + * + * @since 1.0 + */ +public final class TracingSettings { + + /** + * Indicates what headers should not be exported as attributes when generating Open Telemetry traces. By default, common headers associated with credentials are skipped. + * + * @since 1.8.0 + */ + @Parameter + @Optional(defaultValue = "client_id, client_secret, Authorization") + @Expression(SUPPORTED) + private String skipHeadersOnTracing; + + public String getSkipHeadersOnTracing() { + return skipHeadersOnTracing; + } + + public static Builder builder() { + return new Builder(); + } + + public static class Builder { + + private String skipHeadersOnTracing; + + public Builder withFollowRedirects(String skipHeadersOnTracing) { + this.skipHeadersOnTracing = skipHeadersOnTracing; + return this; + } + + public TracingSettings build() { + TracingSettings settings = new TracingSettings(); + settings.skipHeadersOnTracing = this.skipHeadersOnTracing; + return settings; + } + } +} diff --git a/src/main/java/org/mule/extension/http/internal/request/profiling/tracing/HttpCurrentSpanCustomizer.java b/src/main/java/org/mule/extension/http/internal/request/profiling/tracing/HttpCurrentSpanCustomizer.java index 0e01447fd..ccdbccd05 100644 --- a/src/main/java/org/mule/extension/http/internal/request/profiling/tracing/HttpCurrentSpanCustomizer.java +++ b/src/main/java/org/mule/extension/http/internal/request/profiling/tracing/HttpCurrentSpanCustomizer.java @@ -10,6 +10,7 @@ import static org.slf4j.LoggerFactory.getLogger; import org.mule.sdk.api.runtime.source.DistributedTraceContextManager; +import org.mule.runtime.api.util.MultiMap; import java.net.URI; import java.net.URISyntaxException; @@ -78,4 +79,10 @@ protected String getSpanName() { * @return the span kind */ protected abstract String getSpanKind(); + + /** + * W-12558102 + * @return the headers for the http span. + */ + public abstract MultiMap getHeaders(); } diff --git a/src/main/java/org/mule/extension/http/internal/request/profiling/tracing/HttpRequestCurrentSpanCustomizer.java b/src/main/java/org/mule/extension/http/internal/request/profiling/tracing/HttpRequestCurrentSpanCustomizer.java index 838f91222..a99b09bac 100644 --- a/src/main/java/org/mule/extension/http/internal/request/profiling/tracing/HttpRequestCurrentSpanCustomizer.java +++ b/src/main/java/org/mule/extension/http/internal/request/profiling/tracing/HttpRequestCurrentSpanCustomizer.java @@ -16,9 +16,12 @@ import org.mule.runtime.http.api.domain.HttpProtocol; import org.mule.runtime.http.api.domain.message.request.HttpRequest; import org.mule.sdk.api.runtime.source.DistributedTraceContextManager; +import org.mule.runtime.api.util.MultiMap; import org.slf4j.Logger; import java.net.URI; +import java.util.List; +import java.util.Arrays; /** * A customizer for the current span for HTTP Requests @@ -38,17 +41,23 @@ public class HttpRequestCurrentSpanCustomizer extends HttpCurrentSpanCustomizer public static final String PROTOCOL_VERSION_1_0 = "1.0"; public static final String PROTOCOL_VERSION_1_1 = "1.1"; + //W-12558102 + private final List skipAttributesList; + /** * @return a new instance of a {@link HttpRequestCurrentSpanCustomizer}. */ - public static HttpCurrentSpanCustomizer getHttpRequesterCurrentSpanCustomizer(HttpRequest httpRequest) { - return new HttpRequestCurrentSpanCustomizer(httpRequest); + public static HttpCurrentSpanCustomizer getHttpRequesterCurrentSpanCustomizer(HttpRequest httpRequest, String skipAttributes) { + return new HttpRequestCurrentSpanCustomizer(httpRequest, skipAttributes); } private HttpRequest httpRequest; - private HttpRequestCurrentSpanCustomizer(HttpRequest httpRequest) { + private HttpRequestCurrentSpanCustomizer(HttpRequest httpRequest, String skipAttributes) { this.httpRequest = httpRequest; + //W-12558102 + this.skipAttributesList = Arrays.asList(skipAttributes.split(",", -1)); + this.skipAttributesList.replaceAll(String::trim); } @Override @@ -59,6 +68,10 @@ public void customizeSpan(DistributedTraceContextManager distributedTraceContext distributedTraceContextManager.addCurrentSpanAttribute(HTTP_URL, getURI().toString()); distributedTraceContextManager.addCurrentSpanAttribute(NET_PEER_PORT, valueOf(getURI().getPort())); distributedTraceContextManager.addCurrentSpanAttribute(NET_PEER_NAME, getURI().getHost()); + //W-12558102 - Parsing HTTP headers as Span Attributes + MultiMap headers = getHeaders(); + headers.keySet().forEach(key -> distributedTraceContextManager.addCurrentSpanAttribute(key, headers.get(key))); + } catch (Throwable e) { LOGGER.warn("Error on setting the request span attributes", e); } @@ -104,4 +117,20 @@ private static String getHttpProtocolVersionFrom(HttpProtocol protocol) { return PROTOCOL_VERSION_1_1; } + + /** + * W-12558102 + * This provides a transparent way to obtain the definitive list of headers to add in the spans associated with Otel tracing. + * This list will have all headers except those that have been skipped via the skipHeadersOnTracing property. + */ + @Override + public MultiMap getHeaders() { + MultiMap modifiedHeaders = new MultiMap(); + httpRequest.getHeaders().keySet().forEach(key -> { + if (!skipAttributesList.contains(key)) { + modifiedHeaders.put(key, httpRequest.getHeaders().get(key)); + } + }); + return modifiedHeaders; + } } diff --git a/src/test/java/org/mule/test/http/internal/profiling/tracing/HttpCurrentSpanCustomizerTestCase.java b/src/test/java/org/mule/test/http/internal/profiling/tracing/HttpCurrentSpanCustomizerTestCase.java index f2a7ad207..cfda6caa4 100644 --- a/src/test/java/org/mule/test/http/internal/profiling/tracing/HttpCurrentSpanCustomizerTestCase.java +++ b/src/test/java/org/mule/test/http/internal/profiling/tracing/HttpCurrentSpanCustomizerTestCase.java @@ -60,6 +60,8 @@ public class HttpCurrentSpanCustomizerTestCase { public static final String EXPECTED_PEER_NAME = "www.expectedhost.com"; public static final int TEST_PORT = 8080; public static final String EXPECTED_SCHEME = HTTPS; + //W- gp + public static final String SKIP_HEADERS_ATTRIBUTES = ""; @Test @Description("The listener span customizer informs the distributed trace context manager the correct attributes/name") @@ -74,7 +76,8 @@ public void listenerSpan() { when(headers.get(USER_AGENT)).thenReturn(EXPECTED_USER_AGENT); when(attributes.getLocalAddress()).thenReturn(LOCAL_ADDRESS); - HttpCurrentSpanCustomizer currentSpanCustomizer = getHttpListenerCurrentSpanCustomizer(attributes, TEST_HOST, TEST_PORT); + HttpCurrentSpanCustomizer currentSpanCustomizer = + getHttpListenerCurrentSpanCustomizer(attributes, TEST_HOST, TEST_PORT, SKIP_HEADERS_ATTRIBUTES); DistributedTraceContextManager distributedTraceContextManager = mock(DistributedTraceContextManager.class); currentSpanCustomizer.customizeSpan(distributedTraceContextManager); @@ -99,7 +102,8 @@ public void requestSpan() throws Exception { when(httpRequest.getUri()).thenReturn(uri); when(httpRequest.getProtocol()).thenReturn(HTTP_1_1); - HttpCurrentSpanCustomizer httpCurrentSpanCustomizer = getHttpRequesterCurrentSpanCustomizer(httpRequest); + HttpCurrentSpanCustomizer httpCurrentSpanCustomizer = + getHttpRequesterCurrentSpanCustomizer(httpRequest, SKIP_HEADERS_ATTRIBUTES); httpCurrentSpanCustomizer.customizeSpan(distributedTraceContextManager); verify(distributedTraceContextManager).setCurrentSpanName(EXPECTED_SPAN_NAME); From 79d51ae902564b5a1d17048c1f78021b6491726c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gast=C3=B3n=20Panizza?= Date: Thu, 16 Feb 2023 16:29:10 -0300 Subject: [PATCH 2/6] Correlate test change with GUS --- .../profiling/tracing/HttpCurrentSpanCustomizerTestCase.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/org/mule/test/http/internal/profiling/tracing/HttpCurrentSpanCustomizerTestCase.java b/src/test/java/org/mule/test/http/internal/profiling/tracing/HttpCurrentSpanCustomizerTestCase.java index cfda6caa4..99fdd716e 100644 --- a/src/test/java/org/mule/test/http/internal/profiling/tracing/HttpCurrentSpanCustomizerTestCase.java +++ b/src/test/java/org/mule/test/http/internal/profiling/tracing/HttpCurrentSpanCustomizerTestCase.java @@ -60,7 +60,7 @@ public class HttpCurrentSpanCustomizerTestCase { public static final String EXPECTED_PEER_NAME = "www.expectedhost.com"; public static final int TEST_PORT = 8080; public static final String EXPECTED_SCHEME = HTTPS; - //W- gp + //W-12558102 public static final String SKIP_HEADERS_ATTRIBUTES = ""; @Test From 99134612f01f3a22d464a8778887686ca247d806 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gast=C3=B3n=20Panizza?= Date: Thu, 16 Feb 2023 16:42:20 -0300 Subject: [PATCH 3/6] Sanitization of code based on review --- pom.xml | 2 +- .../http/api/listener/server/HttpListenerConfig.java | 2 -- .../tracing/HttpListenerCurrentSpanCustomizer.java | 9 ++++----- .../tracing/HttpRequestCurrentSpanCustomizer.java | 5 +++-- 4 files changed, 8 insertions(+), 10 deletions(-) diff --git a/pom.xml b/pom.xml index a282f3c23..79e1d7c92 100644 --- a/pom.xml +++ b/pom.xml @@ -15,7 +15,7 @@ org.mule.connectors mule-http-connector mule-extension - 1.8.1-SNAPSHOT + 1.8.0-SNAPSHOT HTTP Connector A Mule extension that provides HTTP server and client functionality diff --git a/src/main/java/org/mule/extension/http/api/listener/server/HttpListenerConfig.java b/src/main/java/org/mule/extension/http/api/listener/server/HttpListenerConfig.java index bb4d05cc4..ae85bf81f 100644 --- a/src/main/java/org/mule/extension/http/api/listener/server/HttpListenerConfig.java +++ b/src/main/java/org/mule/extension/http/api/listener/server/HttpListenerConfig.java @@ -84,8 +84,6 @@ public class HttpListenerConfig implements Initialisable { public void initialise() throws InitialisationException { basePath = sanitizePathWithStartSlash(this.basePath); httpHeaderValidators = new InvalidTransferEncodingValidator(rejectInvalidTransferEncoding); - //W-12558102 - skipHeadersOnTracing = this.skipHeadersOnTracing; } public ListenerPath getFullListenerPath(String listenerPath) { diff --git a/src/main/java/org/mule/extension/http/internal/listener/profiling/tracing/HttpListenerCurrentSpanCustomizer.java b/src/main/java/org/mule/extension/http/internal/listener/profiling/tracing/HttpListenerCurrentSpanCustomizer.java index 647c4a060..cf400a3af 100644 --- a/src/main/java/org/mule/extension/http/internal/listener/profiling/tracing/HttpListenerCurrentSpanCustomizer.java +++ b/src/main/java/org/mule/extension/http/internal/listener/profiling/tracing/HttpListenerCurrentSpanCustomizer.java @@ -8,7 +8,7 @@ package org.mule.extension.http.internal.listener.profiling.tracing; import static java.lang.String.valueOf; - +import static java.util.Arrays.asList; import static com.google.common.net.HttpHeaders.USER_AGENT; import static org.slf4j.LoggerFactory.getLogger; @@ -16,14 +16,14 @@ import org.mule.extension.http.internal.request.profiling.tracing.HttpCurrentSpanCustomizer; import org.mule.sdk.api.runtime.source.DistributedTraceContextManager; import org.mule.runtime.api.util.MultiMap; -import com.google.common.collect.ArrayListMultimap; -import com.google.common.collect.ListMultimap; import java.net.URI; import java.net.URISyntaxException; import java.util.List; import java.util.Arrays; +import com.google.common.collect.ArrayListMultimap; +import com.google.common.collect.ListMultimap; import org.slf4j.Logger; /** @@ -46,7 +46,6 @@ public class HttpListenerCurrentSpanCustomizer extends HttpCurrentSpanCustomizer private final String host; private final int port; - //W-12558102 private final List skipAttributesList; private HttpListenerCurrentSpanCustomizer(HttpRequestAttributes attributes, String host, int port, String skipAttributes) { @@ -54,7 +53,7 @@ private HttpListenerCurrentSpanCustomizer(HttpRequestAttributes attributes, Stri this.host = host; this.port = port; //W-12558102 - this.skipAttributesList = Arrays.asList(skipAttributes.split(",", -1)); + this.skipAttributesList = asList(skipAttributes.split(",", -1)); this.skipAttributesList.replaceAll(String::trim); } diff --git a/src/main/java/org/mule/extension/http/internal/request/profiling/tracing/HttpRequestCurrentSpanCustomizer.java b/src/main/java/org/mule/extension/http/internal/request/profiling/tracing/HttpRequestCurrentSpanCustomizer.java index a99b09bac..fa0db626c 100644 --- a/src/main/java/org/mule/extension/http/internal/request/profiling/tracing/HttpRequestCurrentSpanCustomizer.java +++ b/src/main/java/org/mule/extension/http/internal/request/profiling/tracing/HttpRequestCurrentSpanCustomizer.java @@ -7,9 +7,9 @@ package org.mule.extension.http.internal.request.profiling.tracing; -import static java.lang.String.valueOf; import static org.mule.runtime.http.api.domain.HttpProtocol.HTTP_0_9; import static org.mule.runtime.http.api.domain.HttpProtocol.HTTP_1_0; +import static java.lang.String.valueOf; import static org.slf4j.LoggerFactory.getLogger; import org.mule.extension.http.internal.listener.profiling.tracing.HttpListenerCurrentSpanCustomizer; @@ -17,12 +17,13 @@ import org.mule.runtime.http.api.domain.message.request.HttpRequest; import org.mule.sdk.api.runtime.source.DistributedTraceContextManager; import org.mule.runtime.api.util.MultiMap; -import org.slf4j.Logger; import java.net.URI; import java.util.List; import java.util.Arrays; +import org.slf4j.Logger; + /** * A customizer for the current span for HTTP Requests * From 8ac3154f17fe566e29b6ab0c042bb22689646f2c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gast=C3=B3n=20Panizza?= Date: Thu, 16 Feb 2023 17:10:25 -0300 Subject: [PATCH 4/6] Refactor of getHeaders() --- .../HttpListenerCurrentSpanCustomizer.java | 20 +--------------- .../tracing/HttpCurrentSpanCustomizer.java | 13 +++++++++- .../HttpRequestCurrentSpanCustomizer.java | 24 +++---------------- 3 files changed, 16 insertions(+), 41 deletions(-) diff --git a/src/main/java/org/mule/extension/http/internal/listener/profiling/tracing/HttpListenerCurrentSpanCustomizer.java b/src/main/java/org/mule/extension/http/internal/listener/profiling/tracing/HttpListenerCurrentSpanCustomizer.java index cf400a3af..b724215ca 100644 --- a/src/main/java/org/mule/extension/http/internal/listener/profiling/tracing/HttpListenerCurrentSpanCustomizer.java +++ b/src/main/java/org/mule/extension/http/internal/listener/profiling/tracing/HttpListenerCurrentSpanCustomizer.java @@ -20,7 +20,6 @@ import java.net.URI; import java.net.URISyntaxException; import java.util.List; -import java.util.Arrays; import com.google.common.collect.ArrayListMultimap; import com.google.common.collect.ListMultimap; @@ -52,7 +51,6 @@ private HttpListenerCurrentSpanCustomizer(HttpRequestAttributes attributes, Stri this.attributes = attributes; this.host = host; this.port = port; - //W-12558102 this.skipAttributesList = asList(skipAttributes.split(",", -1)); this.skipAttributesList.replaceAll(String::trim); } @@ -74,8 +72,7 @@ public void customizeSpan(DistributedTraceContextManager distributedTraceContext distributedTraceContextManager.addCurrentSpanAttribute(NET_HOST_NAME, host); distributedTraceContextManager.addCurrentSpanAttribute(NET_HOST_PORT, valueOf(getURI().getPort())); distributedTraceContextManager.addCurrentSpanAttribute(HTTP_SCHEME, attributes.getScheme()); - //W-12558102 - Parsing HTTP headers as Span Attributes - MultiMap headers = getHeaders(); + MultiMap headers = getHeaders(attributes.getHeaders(), skipAttributesList); headers.keySet().forEach(key -> distributedTraceContextManager.addCurrentSpanAttribute(key, headers.get(key))); } catch (Throwable e) { @@ -121,19 +118,4 @@ protected String getSpanName() { return attributes.getListenerPath(); } - /** - * W-12558102 - * This provides a transparent way to obtain the definitive list of headers to add in the spans associated with Otel tracing. - * This list will have all headers except those that have been skipped via the skipHeadersOnTracing property. - */ - @Override - public MultiMap getHeaders() { - MultiMap modifiedHeaders = new MultiMap(); - attributes.getHeaders().keySet().forEach(key -> { - if (!skipAttributesList.contains(key)) { - modifiedHeaders.put(key, attributes.getHeaders().get(key)); - } - }); - return modifiedHeaders; - } } diff --git a/src/main/java/org/mule/extension/http/internal/request/profiling/tracing/HttpCurrentSpanCustomizer.java b/src/main/java/org/mule/extension/http/internal/request/profiling/tracing/HttpCurrentSpanCustomizer.java index ccdbccd05..b4a984e0e 100644 --- a/src/main/java/org/mule/extension/http/internal/request/profiling/tracing/HttpCurrentSpanCustomizer.java +++ b/src/main/java/org/mule/extension/http/internal/request/profiling/tracing/HttpCurrentSpanCustomizer.java @@ -14,6 +14,7 @@ import java.net.URI; import java.net.URISyntaxException; +import java.util.List; import org.slf4j.Logger; @@ -83,6 +84,16 @@ protected String getSpanName() { /** * W-12558102 * @return the headers for the http span. + * This provides a transparent way to obtain the definitive list of headers to add in the spans associated with Otel tracing. + * This list will have all headers except those that have been skipped via the skipHeadersOnTracing property. */ - public abstract MultiMap getHeaders(); + public MultiMap getHeaders(MultiMap headers, List skipAttributesList) { + MultiMap modifiedHeaders = new MultiMap(); + headers.keySet().forEach(key -> { + if (!skipAttributesList.contains(key)) { + modifiedHeaders.put(key, headers.get(key)); + } + }); + return modifiedHeaders; + }; } diff --git a/src/main/java/org/mule/extension/http/internal/request/profiling/tracing/HttpRequestCurrentSpanCustomizer.java b/src/main/java/org/mule/extension/http/internal/request/profiling/tracing/HttpRequestCurrentSpanCustomizer.java index fa0db626c..c18fadeee 100644 --- a/src/main/java/org/mule/extension/http/internal/request/profiling/tracing/HttpRequestCurrentSpanCustomizer.java +++ b/src/main/java/org/mule/extension/http/internal/request/profiling/tracing/HttpRequestCurrentSpanCustomizer.java @@ -10,6 +10,7 @@ import static org.mule.runtime.http.api.domain.HttpProtocol.HTTP_0_9; import static org.mule.runtime.http.api.domain.HttpProtocol.HTTP_1_0; import static java.lang.String.valueOf; +import static java.util.Arrays.asList; import static org.slf4j.LoggerFactory.getLogger; import org.mule.extension.http.internal.listener.profiling.tracing.HttpListenerCurrentSpanCustomizer; @@ -20,8 +21,6 @@ import java.net.URI; import java.util.List; -import java.util.Arrays; - import org.slf4j.Logger; /** @@ -56,8 +55,7 @@ public static HttpCurrentSpanCustomizer getHttpRequesterCurrentSpanCustomizer(Ht private HttpRequestCurrentSpanCustomizer(HttpRequest httpRequest, String skipAttributes) { this.httpRequest = httpRequest; - //W-12558102 - this.skipAttributesList = Arrays.asList(skipAttributes.split(",", -1)); + this.skipAttributesList = asList(skipAttributes.split(",", -1)); this.skipAttributesList.replaceAll(String::trim); } @@ -69,8 +67,7 @@ public void customizeSpan(DistributedTraceContextManager distributedTraceContext distributedTraceContextManager.addCurrentSpanAttribute(HTTP_URL, getURI().toString()); distributedTraceContextManager.addCurrentSpanAttribute(NET_PEER_PORT, valueOf(getURI().getPort())); distributedTraceContextManager.addCurrentSpanAttribute(NET_PEER_NAME, getURI().getHost()); - //W-12558102 - Parsing HTTP headers as Span Attributes - MultiMap headers = getHeaders(); + MultiMap headers = getHeaders(httpRequest.getHeaders(), skipAttributesList); headers.keySet().forEach(key -> distributedTraceContextManager.addCurrentSpanAttribute(key, headers.get(key))); } catch (Throwable e) { @@ -119,19 +116,4 @@ private static String getHttpProtocolVersionFrom(HttpProtocol protocol) { return PROTOCOL_VERSION_1_1; } - /** - * W-12558102 - * This provides a transparent way to obtain the definitive list of headers to add in the spans associated with Otel tracing. - * This list will have all headers except those that have been skipped via the skipHeadersOnTracing property. - */ - @Override - public MultiMap getHeaders() { - MultiMap modifiedHeaders = new MultiMap(); - httpRequest.getHeaders().keySet().forEach(key -> { - if (!skipAttributesList.contains(key)) { - modifiedHeaders.put(key, httpRequest.getHeaders().get(key)); - } - }); - return modifiedHeaders; - } } From e049bc48c2513981585e66631ba873fa45637ab6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gast=C3=B3n=20Panizza?= Date: Thu, 16 Feb 2023 18:08:21 -0300 Subject: [PATCH 5/6] Enhance test case to include skip headers on tracing --- .../HttpListenerCurrentSpanCustomizer.java | 1 - .../HttpCurrentSpanCustomizerTestCase.java | 24 +++++++++++++++---- 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/src/main/java/org/mule/extension/http/internal/listener/profiling/tracing/HttpListenerCurrentSpanCustomizer.java b/src/main/java/org/mule/extension/http/internal/listener/profiling/tracing/HttpListenerCurrentSpanCustomizer.java index b724215ca..be4cec365 100644 --- a/src/main/java/org/mule/extension/http/internal/listener/profiling/tracing/HttpListenerCurrentSpanCustomizer.java +++ b/src/main/java/org/mule/extension/http/internal/listener/profiling/tracing/HttpListenerCurrentSpanCustomizer.java @@ -58,7 +58,6 @@ private HttpListenerCurrentSpanCustomizer(HttpRequestAttributes attributes, Stri public static HttpCurrentSpanCustomizer getHttpListenerCurrentSpanCustomizer(HttpRequestAttributes attributes, String host, int port, - //W-... gp String skipAttributes) { return new HttpListenerCurrentSpanCustomizer(attributes, host, port, skipAttributes); } diff --git a/src/test/java/org/mule/test/http/internal/profiling/tracing/HttpCurrentSpanCustomizerTestCase.java b/src/test/java/org/mule/test/http/internal/profiling/tracing/HttpCurrentSpanCustomizerTestCase.java index 99fdd716e..68235ed04 100644 --- a/src/test/java/org/mule/test/http/internal/profiling/tracing/HttpCurrentSpanCustomizerTestCase.java +++ b/src/test/java/org/mule/test/http/internal/profiling/tracing/HttpCurrentSpanCustomizerTestCase.java @@ -21,6 +21,7 @@ import static org.mule.extension.http.internal.request.profiling.tracing.HttpRequestCurrentSpanCustomizer.NET_PEER_NAME; import static org.mule.extension.http.internal.request.profiling.tracing.HttpRequestCurrentSpanCustomizer.NET_PEER_PORT; import static org.mule.extension.http.internal.request.profiling.tracing.HttpRequestCurrentSpanCustomizer.getHttpRequesterCurrentSpanCustomizer; +import static org.mule.extension.http.internal.request.profiling.tracing.HttpCurrentSpanCustomizer.SPAN_KIND; import static org.mule.runtime.http.api.domain.HttpProtocol.HTTP_1_1; import static org.mule.test.http.AllureConstants.HttpFeature.HTTP_EXTENSION; @@ -28,6 +29,7 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import static org.mockito.Mockito.never; import static org.mule.test.http.AllureConstants.HttpFeature.HttpStory.TRACING; import io.qameta.allure.Description; @@ -60,8 +62,11 @@ public class HttpCurrentSpanCustomizerTestCase { public static final String EXPECTED_PEER_NAME = "www.expectedhost.com"; public static final int TEST_PORT = 8080; public static final String EXPECTED_SCHEME = HTTPS; - //W-12558102 - public static final String SKIP_HEADERS_ATTRIBUTES = ""; + public static final String EXPECTED_SPAN_KIND = "SERVER"; + public static final String SKIP_HEADERS_ATTRIBUTES = "client_id, client_secret"; + public static final String EXPECTED_SKIPPED_HEADER_KEY = "client_id"; + public static final String EXPECTED_SKIPPED_HEADER_VALUE = "1f6bdf78-ae39-11ed-afa1-0242ac120002"; + public MultiMap headers; @Test @Description("The listener span customizer informs the distributed trace context manager the correct attributes/name") @@ -71,25 +76,30 @@ public void listenerSpan() { when(attributes.getScheme()).thenReturn(HTTPS); when(attributes.getVersion()).thenReturn(HTTP_1_1.asString()); when(attributes.getListenerPath()).thenReturn(LISTENER_PATH); - MultiMap headers = mock(MultiMap.class); + headers = mock(MultiMap.class); + headers.put(EXPECTED_SKIPPED_HEADER_KEY, EXPECTED_SKIPPED_HEADER_VALUE); when(attributes.getHeaders()).thenReturn(headers); when(headers.get(USER_AGENT)).thenReturn(EXPECTED_USER_AGENT); when(attributes.getLocalAddress()).thenReturn(LOCAL_ADDRESS); HttpCurrentSpanCustomizer currentSpanCustomizer = getHttpListenerCurrentSpanCustomizer(attributes, TEST_HOST, TEST_PORT, SKIP_HEADERS_ATTRIBUTES); + DistributedTraceContextManager distributedTraceContextManager = mock(DistributedTraceContextManager.class); currentSpanCustomizer.customizeSpan(distributedTraceContextManager); verify(distributedTraceContextManager).setCurrentSpanName(LISTENER_PATH); verify(distributedTraceContextManager).addCurrentSpanAttribute(HTTP_METHOD, GET.asString()); + verify(distributedTraceContextManager).addCurrentSpanAttribute(SPAN_KIND, EXPECTED_SPAN_KIND); verify(distributedTraceContextManager).addCurrentSpanAttribute(HTTP_FLAVOR, EXPECTED_PROTOCOL_VERSION); verify(distributedTraceContextManager).addCurrentSpanAttribute(HTTP_TARGET, LISTENER_PATH); verify(distributedTraceContextManager).addCurrentSpanAttribute(NET_HOST_NAME, TEST_HOST); verify(distributedTraceContextManager).addCurrentSpanAttribute(NET_HOST_PORT, EXPECTED_PORT); - verify(distributedTraceContextManager).addCurrentSpanAttribute(HTTP_USER_AGENT, EXPECTED_USER_AGENT); verify(distributedTraceContextManager).addCurrentSpanAttribute(HTTP_SCHEME, EXPECTED_SCHEME); + + verify(distributedTraceContextManager, never()).addCurrentSpanAttribute(EXPECTED_SKIPPED_HEADER_KEY, + EXPECTED_SKIPPED_HEADER_VALUE); } @Test @@ -101,6 +111,8 @@ public void requestSpan() throws Exception { when(httpRequest.getMethod()).thenReturn(GET.asString()); when(httpRequest.getUri()).thenReturn(uri); when(httpRequest.getProtocol()).thenReturn(HTTP_1_1); + headers = mock(MultiMap.class); + when(httpRequest.getHeaders()).thenReturn(headers); HttpCurrentSpanCustomizer httpCurrentSpanCustomizer = getHttpRequesterCurrentSpanCustomizer(httpRequest, SKIP_HEADERS_ATTRIBUTES); @@ -112,5 +124,9 @@ public void requestSpan() throws Exception { verify(distributedTraceContextManager).addCurrentSpanAttribute(HTTP_URL, uri.toString()); verify(distributedTraceContextManager).addCurrentSpanAttribute(NET_PEER_PORT, EXPECTED_PORT); verify(distributedTraceContextManager).addCurrentSpanAttribute(NET_PEER_NAME, EXPECTED_PEER_NAME); + + verify(distributedTraceContextManager, never()).addCurrentSpanAttribute(EXPECTED_SKIPPED_HEADER_KEY, + EXPECTED_SKIPPED_HEADER_VALUE); + } } From b42ebc8a3e3f2dd3672b81a7f7f9dee2efef0e83 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gast=C3=B3n=20Panizza?= Date: Fri, 17 Feb 2023 13:53:57 -0300 Subject: [PATCH 6/6] Reverting USER_AGENT to Otel convention --- .../profiling/tracing/HttpListenerCurrentSpanCustomizer.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/main/java/org/mule/extension/http/internal/listener/profiling/tracing/HttpListenerCurrentSpanCustomizer.java b/src/main/java/org/mule/extension/http/internal/listener/profiling/tracing/HttpListenerCurrentSpanCustomizer.java index be4cec365..9771e8911 100644 --- a/src/main/java/org/mule/extension/http/internal/listener/profiling/tracing/HttpListenerCurrentSpanCustomizer.java +++ b/src/main/java/org/mule/extension/http/internal/listener/profiling/tracing/HttpListenerCurrentSpanCustomizer.java @@ -71,6 +71,11 @@ public void customizeSpan(DistributedTraceContextManager distributedTraceContext distributedTraceContextManager.addCurrentSpanAttribute(NET_HOST_NAME, host); distributedTraceContextManager.addCurrentSpanAttribute(NET_HOST_PORT, valueOf(getURI().getPort())); distributedTraceContextManager.addCurrentSpanAttribute(HTTP_SCHEME, attributes.getScheme()); + String userAgent = attributes.getHeaders().get(USER_AGENT); + + if (userAgent != null) { + distributedTraceContextManager.addCurrentSpanAttribute(HTTP_USER_AGENT, userAgent); + } MultiMap headers = getHeaders(attributes.getHeaders(), skipAttributesList); headers.keySet().forEach(key -> distributedTraceContextManager.addCurrentSpanAttribute(key, headers.get(key)));