From 10430777c43fb88ef022cb6e269a7704c9fe4690 Mon Sep 17 00:00:00 2001 From: Mateusz Rzeszutek Date: Mon, 19 Jun 2023 11:06:46 +0200 Subject: [PATCH] Make server.socket.* attributes on the HTTP server side opt-in --- .../http/HttpClientAttributesExtractor.java | 3 +- .../http/HttpServerAttributesExtractor.java | 10 ++- .../HttpServerAttributesExtractorBuilder.java | 21 ++++- .../net/NetClientAttributesExtractor.java | 3 +- .../net/NetServerAttributesExtractor.java | 3 +- .../network/ServerAttributesExtractor.java | 3 +- .../InternalServerAttributesExtractor.java | 11 ++- .../HttpServerAttributesExtractorTest.java | 1 + ...verAttributesExtractorBothSemconvTest.java | 1 + ...rAttributesExtractorStableSemconvTest.java | 90 +++++++++++-------- 10 files changed, 99 insertions(+), 47 deletions(-) diff --git a/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractor.java b/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractor.java index 816cc66fcd99..2c51b4226aaa 100644 --- a/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractor.java +++ b/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractor.java @@ -98,7 +98,8 @@ public static HttpClientAttributesExtractorBuilder HttpServerAttributesExtractorBuilder httpAttributesGetter, NetServerAttributesGetter netAttributesGetter, List capturedRequestHeaders, - List capturedResponseHeaders) { + List capturedResponseHeaders, + boolean captureServerSocketAttributes) { this( httpAttributesGetter, netAttributesGetter, capturedRequestHeaders, capturedResponseHeaders, + captureServerSocketAttributes, HttpRouteHolder::getRoute); } @@ -93,6 +95,7 @@ public static HttpServerAttributesExtractorBuilder netAttributesGetter, List capturedRequestHeaders, List capturedResponseHeaders, + boolean captureServerSocketAttributes, Function httpRouteHolderGetter) { super(httpAttributesGetter, capturedRequestHeaders, capturedResponseHeaders); HttpNetNamePortGetter namePortGetter = @@ -119,7 +122,10 @@ public static HttpServerAttributesExtractorBuilder( netAttributesGetter, diff --git a/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerAttributesExtractorBuilder.java b/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerAttributesExtractorBuilder.java index eccb8ba08175..06ad4ad9922f 100644 --- a/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerAttributesExtractorBuilder.java +++ b/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerAttributesExtractorBuilder.java @@ -19,6 +19,7 @@ public final class HttpServerAttributesExtractorBuilder { final NetServerAttributesGetter netAttributesGetter; List capturedRequestHeaders = emptyList(); List capturedResponseHeaders = emptyList(); + boolean captureServerSocketAttributes = false; HttpServerAttributesExtractorBuilder( HttpServerAttributesGetter httpAttributesGetter, @@ -64,12 +65,30 @@ public HttpServerAttributesExtractorBuilder setCapturedRespon return this; } + /** + * Configures the extractor to capture the optional {@code server.socket.address} and {@code + * server.socket.port} attributes, which are not collected by default. + * + * @param captureServerSocketAttributes {@code true} if the extractor should collect the optional + * {@code server.socket.address} and {@code server.socket.port} attributes. + */ + @CanIgnoreReturnValue + public HttpServerAttributesExtractorBuilder setCaptureServerSocketAttributes( + boolean captureServerSocketAttributes) { + this.captureServerSocketAttributes = captureServerSocketAttributes; + return this; + } + /** * Returns a new {@link HttpServerAttributesExtractor} with the settings of this {@link * HttpServerAttributesExtractorBuilder}. */ public AttributesExtractor build() { return new HttpServerAttributesExtractor<>( - httpAttributesGetter, netAttributesGetter, capturedRequestHeaders, capturedResponseHeaders); + httpAttributesGetter, + netAttributesGetter, + capturedRequestHeaders, + capturedResponseHeaders, + captureServerSocketAttributes); } } diff --git a/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/net/NetClientAttributesExtractor.java b/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/net/NetClientAttributesExtractor.java index 016ca5e24d79..49b8c91c206f 100644 --- a/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/net/NetClientAttributesExtractor.java +++ b/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/net/NetClientAttributesExtractor.java @@ -53,7 +53,8 @@ private NetClientAttributesExtractor(NetClientAttributesGetter( getter, diff --git a/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/network/ServerAttributesExtractor.java b/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/network/ServerAttributesExtractor.java index c622da9adb64..159962d93e0e 100644 --- a/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/network/ServerAttributesExtractor.java +++ b/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/network/ServerAttributesExtractor.java @@ -41,7 +41,8 @@ public static ServerAttributesExtractor c /* emitStableUrlAttributes= */ true, /* emitOldHttpAttributes= */ false, // this param does not matter when old semconv is off - InternalServerAttributesExtractor.Mode.HOST); + InternalServerAttributesExtractor.Mode.HOST, + /* captureServerSocketAttributes= */ true); } @Override diff --git a/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/network/internal/InternalServerAttributesExtractor.java b/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/network/internal/InternalServerAttributesExtractor.java index 68e4ef1db9ce..742f1e334679 100644 --- a/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/network/internal/InternalServerAttributesExtractor.java +++ b/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/network/internal/InternalServerAttributesExtractor.java @@ -27,6 +27,7 @@ public final class InternalServerAttributesExtractor { private final boolean emitStableUrlAttributes; private final boolean emitOldHttpAttributes; private final Mode oldSemconvMode; + private final boolean captureServerSocketAttributes; public InternalServerAttributesExtractor( ServerAttributesGetter getter, @@ -34,13 +35,15 @@ public InternalServerAttributesExtractor( FallbackNamePortGetter fallbackNamePortGetter, boolean emitStableUrlAttributes, boolean emitOldHttpAttributes, - Mode oldSemconvMode) { + Mode oldSemconvMode, + boolean captureServerSocketAttributes) { this.getter = getter; this.captureServerPortCondition = captureServerPortCondition; this.fallbackNamePortGetter = fallbackNamePortGetter; this.emitStableUrlAttributes = emitStableUrlAttributes; this.emitOldHttpAttributes = emitOldHttpAttributes; this.oldSemconvMode = oldSemconvMode; + this.captureServerSocketAttributes = captureServerSocketAttributes; } public void onStart(AttributesBuilder attributes, REQUEST request) { @@ -73,7 +76,7 @@ public void onEnd(AttributesBuilder attributes, REQUEST request, @Nullable RESPO String serverSocketAddress = getter.getServerSocketAddress(request, response); if (serverSocketAddress != null && !serverSocketAddress.equals(serverAddress)) { - if (emitStableUrlAttributes) { + if (emitStableUrlAttributes && captureServerSocketAttributes) { internalSet(attributes, NetworkAttributes.SERVER_SOCKET_ADDRESS, serverSocketAddress); } if (emitOldHttpAttributes) { @@ -85,7 +88,7 @@ public void onEnd(AttributesBuilder attributes, REQUEST request, @Nullable RESPO if (serverSocketPort != null && serverSocketPort > 0 && !serverSocketPort.equals(serverPort)) { - if (emitStableUrlAttributes) { + if (emitStableUrlAttributes && captureServerSocketAttributes) { internalSet(attributes, NetworkAttributes.SERVER_SOCKET_PORT, (long) serverSocketPort); } if (emitOldHttpAttributes) { @@ -95,7 +98,7 @@ public void onEnd(AttributesBuilder attributes, REQUEST request, @Nullable RESPO String serverSocketDomain = getter.getServerSocketDomain(request, response); if (serverSocketDomain != null && !serverSocketDomain.equals(serverAddress)) { - if (emitStableUrlAttributes) { + if (emitStableUrlAttributes && captureServerSocketAttributes) { internalSet(attributes, NetworkAttributes.SERVER_SOCKET_DOMAIN, serverSocketDomain); } if (emitOldHttpAttributes && oldSemconvMode.socketDomain != null) { diff --git a/instrumentation-api-semconv/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerAttributesExtractorTest.java b/instrumentation-api-semconv/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerAttributesExtractorTest.java index ab2fcc33f3fd..6fd9fa5fbac7 100644 --- a/instrumentation-api-semconv/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerAttributesExtractorTest.java +++ b/instrumentation-api-semconv/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerAttributesExtractorTest.java @@ -162,6 +162,7 @@ void normal() { new TestNetServerAttributesGetter(), singletonList("Custom-Request-Header"), singletonList("Custom-Response-Header"), + false, routeFromContext); AttributesBuilder startAttributes = Attributes.builder(); diff --git a/instrumentation-api-semconv/src/testBothHttpSemconv/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerAttributesExtractorBothSemconvTest.java b/instrumentation-api-semconv/src/testBothHttpSemconv/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerAttributesExtractorBothSemconvTest.java index 8f926a62da85..18b5121b64b6 100644 --- a/instrumentation-api-semconv/src/testBothHttpSemconv/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerAttributesExtractorBothSemconvTest.java +++ b/instrumentation-api-semconv/src/testBothHttpSemconv/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerAttributesExtractorBothSemconvTest.java @@ -156,6 +156,7 @@ void normal() { new TestNetServerAttributesGetter(), singletonList("Custom-Request-Header"), singletonList("Custom-Response-Header"), + false, routeFromContext); AttributesBuilder startAttributes = Attributes.builder(); diff --git a/instrumentation-api-semconv/src/testStableHttpSemconv/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerAttributesExtractorStableSemconvTest.java b/instrumentation-api-semconv/src/testStableHttpSemconv/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerAttributesExtractorStableSemconvTest.java index ea26ec3591ad..75d956f167a9 100644 --- a/instrumentation-api-semconv/src/testStableHttpSemconv/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerAttributesExtractorStableSemconvTest.java +++ b/instrumentation-api-semconv/src/testStableHttpSemconv/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerAttributesExtractorStableSemconvTest.java @@ -38,103 +38,119 @@ class HttpServerAttributesExtractorStableSemconvTest { static class TestHttpServerAttributesGetter - implements HttpServerAttributesGetter, Map> { + implements HttpServerAttributesGetter, Map> { @Override - public String getHttpRequestMethod(Map request) { - return (String) request.get("method"); + public String getHttpRequestMethod(Map request) { + return request.get("method"); } @Override - public String getUrlScheme(Map request) { - return (String) request.get("scheme"); + public String getUrlScheme(Map request) { + return request.get("scheme"); } @Nullable @Override - public String getUrlPath(Map request) { - return (String) request.get("path"); + public String getUrlPath(Map request) { + return request.get("path"); } @Nullable @Override - public String getUrlQuery(Map request) { - return (String) request.get("query"); + public String getUrlQuery(Map request) { + return request.get("query"); } @Override - public String getHttpRoute(Map request) { - return (String) request.get("route"); + public String getHttpRoute(Map request) { + return request.get("route"); } @Override - public List getHttpRequestHeader(Map request, String name) { - String values = (String) request.get("header." + name); + public List getHttpRequestHeader(Map request, String name) { + String values = request.get("header." + name); return values == null ? emptyList() : asList(values.split(",")); } @Override public Integer getHttpResponseStatusCode( - Map request, Map response, @Nullable Throwable error) { - String value = (String) response.get("statusCode"); + Map request, Map response, @Nullable Throwable error) { + String value = response.get("statusCode"); return value == null ? null : Integer.parseInt(value); } @Override public List getHttpResponseHeader( - Map request, Map response, String name) { - String values = (String) response.get("header." + name); + Map request, Map response, String name) { + String values = response.get("header." + name); return values == null ? emptyList() : asList(values.split(",")); } } static class TestNetServerAttributesGetter - implements NetServerAttributesGetter, Map> { + implements NetServerAttributesGetter, Map> { @Nullable @Override public String getNetworkTransport( - Map request, @Nullable Map response) { - return (String) request.get("transport"); + Map request, @Nullable Map response) { + return request.get("transport"); } @Nullable @Override public String getNetworkType( - Map request, @Nullable Map response) { - return (String) request.get("type"); + Map request, @Nullable Map response) { + return request.get("type"); } @Nullable @Override public String getNetworkProtocolName( - Map request, Map response) { - return (String) request.get("protocolName"); + Map request, Map response) { + return request.get("protocolName"); } @Nullable @Override public String getNetworkProtocolVersion( - Map request, Map response) { - return (String) request.get("protocolVersion"); + Map request, Map response) { + return request.get("protocolVersion"); } @Nullable @Override - public String getServerAddress(Map request) { - return (String) request.get("hostName"); + public String getServerAddress(Map request) { + return request.get("hostName"); } @Nullable @Override - public Integer getServerPort(Map request) { - return (Integer) request.get("hostPort"); + public Integer getServerPort(Map request) { + String value = request.get("hostPort"); + return value == null ? null : Integer.parseInt(value); + } + + @Nullable + @Override + public String getServerSocketAddress( + Map request, @Nullable Map response) { + return request.get("serverSocketAddress"); + } + + @Nullable + @Override + public Integer getServerSocketPort( + Map request, @Nullable Map response) { + String value = request.get("serverSocketPort"); + return value == null ? null : Integer.parseInt(value); } } @Test void normal() { - Map request = new HashMap<>(); + Map request = new HashMap<>(); request.put("method", "POST"); request.put("url", "http://github.com"); request.put("path", "/repositories/1"); @@ -150,20 +166,23 @@ void normal() { request.put("type", "ipv4"); request.put("protocolName", "http"); request.put("protocolVersion", "2.0"); + request.put("serverSocketAddress", "1.2.3.4"); + request.put("serverSocketPort", "42"); - Map response = new HashMap<>(); + Map response = new HashMap<>(); response.put("statusCode", "202"); response.put("header.content-length", "20"); response.put("header.custom-response-header", "654,321"); Function routeFromContext = ctx -> "/repositories/{repoId}"; - HttpServerAttributesExtractor, Map> extractor = + HttpServerAttributesExtractor, Map> extractor = new HttpServerAttributesExtractor<>( new TestHttpServerAttributesGetter(), new TestNetServerAttributesGetter(), singletonList("Custom-Request-Header"), singletonList("Custom-Response-Header"), + false, routeFromContext); AttributesBuilder startAttributes = Attributes.builder(); @@ -212,9 +231,8 @@ void skipNetworkTransportIfDefaultForProtocol( request.put("transport", observedTransport); AttributesExtractor, Map> extractor = - HttpClientAttributesExtractor.create( - new HttpClientAttributesExtractorStableSemconvTest.TestHttpClientAttributesGetter(), - new HttpClientAttributesExtractorStableSemconvTest.TestNetClientAttributesGetter()); + HttpServerAttributesExtractor.create( + new TestHttpServerAttributesGetter(), new TestNetServerAttributesGetter()); AttributesBuilder attributes = Attributes.builder(); extractor.onStart(attributes, Context.root(), request);