From 506ae2b894fc147a9c4613442c4c969d6e31666a Mon Sep 17 00:00:00 2001 From: Trask Stalnaker Date: Mon, 4 Oct 2021 09:23:58 -0700 Subject: [PATCH] Flavor extractor (#4274) --- .../http/HttpClientAttributesExtractor.java | 14 ++++++++++++++ .../http/HttpCommonAttributesExtractor.java | 10 ---------- .../http/HttpServerAttributesExtractor.java | 4 ++++ .../http/HttpServerAttributesExtractorTest.java | 3 ++- .../v1_3/ArmeriaHttpServerAttributesExtractor.java | 2 +- .../LibertyDispatcherHttpAttributesExtractor.java | 3 +-- .../ratpack/RatpackHttpAttributesExtractor.java | 2 +- .../v1_0/RestletHttpAttributesExtractor.java | 2 +- .../servlet/ServletHttpAttributesExtractor.java | 4 +--- .../SpringWebMvcHttpAttributesExtractor.java | 3 +-- .../common/TomcatHttpAttributesExtractor.java | 2 +- .../undertow/UndertowHttpAttributesExtractor.java | 2 +- 12 files changed, 28 insertions(+), 23 deletions(-) diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractor.java b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractor.java index 341b57582d33..2476b33044ae 100644 --- a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractor.java +++ b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractor.java @@ -6,6 +6,8 @@ package io.opentelemetry.instrumentation.api.instrumenter.http; import io.opentelemetry.api.common.AttributesBuilder; +import io.opentelemetry.context.Context; +import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter; import io.opentelemetry.semconv.trace.attributes.SemanticAttributes; import org.checkerframework.checker.nullness.qual.Nullable; @@ -34,10 +36,22 @@ protected final void onEnd( @Nullable RESPONSE response, @Nullable Throwable error) { super.onEnd(attributes, request, response, error); + set(attributes, SemanticAttributes.HTTP_FLAVOR, flavor(request, response)); } // Attributes that always exist in a request @Nullable protected abstract String url(REQUEST request); + + // Attributes which are not always available when the request is ready. + + /** + * Extracts the {@code http.flavor} span attribute. + * + *

This is called from {@link Instrumenter#end(Context, Object, Object, Throwable)}, whether + * {@code response} is {@code null} or not. + */ + @Nullable + protected abstract String flavor(REQUEST request, @Nullable RESPONSE response); } diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpCommonAttributesExtractor.java b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpCommonAttributesExtractor.java index 457878edda06..6fcf11fb0273 100644 --- a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpCommonAttributesExtractor.java +++ b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpCommonAttributesExtractor.java @@ -44,7 +44,6 @@ protected void onEnd( attributes, SemanticAttributes.HTTP_REQUEST_CONTENT_LENGTH_UNCOMPRESSED, requestContentLengthUncompressed(request, response)); - set(attributes, SemanticAttributes.HTTP_FLAVOR, flavor(request, response)); if (response != null) { Integer statusCode = statusCode(request, response); if (statusCode != null) { @@ -90,15 +89,6 @@ protected void onEnd( protected abstract Long requestContentLengthUncompressed( REQUEST request, @Nullable RESPONSE response); - /** - * Extracts the {@code http.flavor} span attribute. - * - *

This is called from {@link Instrumenter#end(Context, Object, Object, Throwable)}, whether - * {@code response} is {@code null} or not. - */ - @Nullable - protected abstract String flavor(REQUEST request, @Nullable RESPONSE response); - /** * Extracts the {@code http.status_code} span attribute. * diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerAttributesExtractor.java b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerAttributesExtractor.java index 83d3ae78ff4b..57a659a05e1b 100644 --- a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerAttributesExtractor.java +++ b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerAttributesExtractor.java @@ -27,6 +27,7 @@ public abstract class HttpServerAttributesExtractor protected final void onStart(AttributesBuilder attributes, REQUEST request) { super.onStart(attributes, request); + set(attributes, SemanticAttributes.HTTP_FLAVOR, flavor(request)); set(attributes, SemanticAttributes.HTTP_SCHEME, scheme(request)); set(attributes, SemanticAttributes.HTTP_HOST, host(request)); set(attributes, SemanticAttributes.HTTP_TARGET, target(request)); @@ -46,6 +47,9 @@ protected final void onEnd( // Attributes that always exist in a request + @Nullable + protected abstract String flavor(REQUEST request); + @Nullable protected abstract String target(REQUEST request); diff --git a/instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerAttributesExtractorTest.java b/instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerAttributesExtractorTest.java index 062b702eda7e..2088d9f59c41 100644 --- a/instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerAttributesExtractorTest.java +++ b/instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerAttributesExtractorTest.java @@ -72,7 +72,7 @@ protected Integer statusCode(Map request, Map re } @Override - protected String flavor(Map request, Map response) { + protected String flavor(Map request) { return request.get("flavor"); } @@ -114,6 +114,7 @@ void normal() { extractor.onStart(attributes, request); assertThat(attributes.build()) .containsOnly( + entry(SemanticAttributes.HTTP_FLAVOR, "http/2"), entry(SemanticAttributes.HTTP_METHOD, "POST"), entry(SemanticAttributes.HTTP_TARGET, "/repositories/1"), entry(SemanticAttributes.HTTP_HOST, "github.com:80"), diff --git a/instrumentation/armeria-1.3/library/src/main/java/io/opentelemetry/instrumentation/armeria/v1_3/ArmeriaHttpServerAttributesExtractor.java b/instrumentation/armeria-1.3/library/src/main/java/io/opentelemetry/instrumentation/armeria/v1_3/ArmeriaHttpServerAttributesExtractor.java index 5c58d903d5f1..58bc3c533d71 100644 --- a/instrumentation/armeria-1.3/library/src/main/java/io/opentelemetry/instrumentation/armeria/v1_3/ArmeriaHttpServerAttributesExtractor.java +++ b/instrumentation/armeria-1.3/library/src/main/java/io/opentelemetry/instrumentation/armeria/v1_3/ArmeriaHttpServerAttributesExtractor.java @@ -74,7 +74,7 @@ protected Integer statusCode(RequestContext ctx, RequestLog requestLog) { } @Override - protected String flavor(RequestContext ctx, @Nullable RequestLog requestLog) { + protected String flavor(RequestContext ctx) { SessionProtocol protocol = ctx.sessionProtocol(); if (protocol.isMultiplex()) { return SemanticAttributes.HttpFlavorValues.HTTP_2_0; diff --git a/instrumentation/liberty/liberty-dispatcher/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/liberty/dispatcher/LibertyDispatcherHttpAttributesExtractor.java b/instrumentation/liberty/liberty-dispatcher/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/liberty/dispatcher/LibertyDispatcherHttpAttributesExtractor.java index ccb41f08abac..8d44114e0b7e 100644 --- a/instrumentation/liberty/liberty-dispatcher/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/liberty/dispatcher/LibertyDispatcherHttpAttributesExtractor.java +++ b/instrumentation/liberty/liberty-dispatcher/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/liberty/dispatcher/LibertyDispatcherHttpAttributesExtractor.java @@ -38,8 +38,7 @@ public class LibertyDispatcherHttpAttributesExtractor } @Override - protected @Nullable String flavor( - LibertyRequest libertyRequest, @Nullable LibertyResponse libertyResponse) { + protected @Nullable String flavor(LibertyRequest libertyRequest) { String flavor = libertyRequest.getProtocol(); if (flavor != null) { // remove HTTP/ prefix to comply with semantic conventions diff --git a/instrumentation/ratpack-1.4/library/src/main/java/io/opentelemetry/instrumentation/ratpack/RatpackHttpAttributesExtractor.java b/instrumentation/ratpack-1.4/library/src/main/java/io/opentelemetry/instrumentation/ratpack/RatpackHttpAttributesExtractor.java index 4d8d22b66169..f21a48dd8f3b 100644 --- a/instrumentation/ratpack-1.4/library/src/main/java/io/opentelemetry/instrumentation/ratpack/RatpackHttpAttributesExtractor.java +++ b/instrumentation/ratpack-1.4/library/src/main/java/io/opentelemetry/instrumentation/ratpack/RatpackHttpAttributesExtractor.java @@ -83,7 +83,7 @@ protected Long requestContentLengthUncompressed(Request request, @Nullable Respo @Override @Nullable - protected String flavor(Request request, @Nullable Response response) { + protected String flavor(Request request) { switch (request.getProtocol()) { case "HTTP/1.0": return SemanticAttributes.HttpFlavorValues.HTTP_1_0; diff --git a/instrumentation/restlet/restlet-1.0/library/src/main/java/io/opentelemetry/instrumentation/restlet/v1_0/RestletHttpAttributesExtractor.java b/instrumentation/restlet/restlet-1.0/library/src/main/java/io/opentelemetry/instrumentation/restlet/v1_0/RestletHttpAttributesExtractor.java index c0dc62b77578..6e9020d676f8 100644 --- a/instrumentation/restlet/restlet-1.0/library/src/main/java/io/opentelemetry/instrumentation/restlet/v1_0/RestletHttpAttributesExtractor.java +++ b/instrumentation/restlet/restlet-1.0/library/src/main/java/io/opentelemetry/instrumentation/restlet/v1_0/RestletHttpAttributesExtractor.java @@ -60,7 +60,7 @@ protected String host(Request request) { } @Override - protected @Nullable String flavor(Request request, @Nullable Response response) { + protected @Nullable String flavor(Request request) { String version = (String) request.getAttributes().get("org.restlet.http.version"); switch (version) { case "HTTP/1.0": diff --git a/instrumentation/servlet/servlet-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/ServletHttpAttributesExtractor.java b/instrumentation/servlet/servlet-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/ServletHttpAttributesExtractor.java index fd2bff7727d5..03d2c805a7f3 100644 --- a/instrumentation/servlet/servlet-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/ServletHttpAttributesExtractor.java +++ b/instrumentation/servlet/servlet-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/ServletHttpAttributesExtractor.java @@ -71,9 +71,7 @@ public ServletHttpAttributesExtractor(ServletAccessor accesso } @Override - protected @Nullable String flavor( - ServletRequestContext requestContext, - @Nullable ServletResponseContext responseContext) { + protected @Nullable String flavor(ServletRequestContext requestContext) { String flavor = accessor.getRequestProtocol(requestContext.request()); if (flavor != null) { // remove HTTP/ prefix to comply with semantic conventions diff --git a/instrumentation/spring/spring-webmvc-3.1/library/src/main/java/io/opentelemetry/instrumentation/spring/webmvc/SpringWebMvcHttpAttributesExtractor.java b/instrumentation/spring/spring-webmvc-3.1/library/src/main/java/io/opentelemetry/instrumentation/spring/webmvc/SpringWebMvcHttpAttributesExtractor.java index 12891c46b193..c4776fa0a6ae 100644 --- a/instrumentation/spring/spring-webmvc-3.1/library/src/main/java/io/opentelemetry/instrumentation/spring/webmvc/SpringWebMvcHttpAttributesExtractor.java +++ b/instrumentation/spring/spring-webmvc-3.1/library/src/main/java/io/opentelemetry/instrumentation/spring/webmvc/SpringWebMvcHttpAttributesExtractor.java @@ -35,8 +35,7 @@ final class SpringWebMvcHttpAttributesExtractor } @Override - protected @Nullable String flavor( - HttpServletRequest request, @Nullable HttpServletResponse response) { + protected @Nullable String flavor(HttpServletRequest request) { return request.getProtocol(); } diff --git a/instrumentation/tomcat/tomcat-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/tomcat/common/TomcatHttpAttributesExtractor.java b/instrumentation/tomcat/tomcat-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/tomcat/common/TomcatHttpAttributesExtractor.java index da583bf25b23..386be84e62ab 100644 --- a/instrumentation/tomcat/tomcat-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/tomcat/common/TomcatHttpAttributesExtractor.java +++ b/instrumentation/tomcat/tomcat-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/tomcat/common/TomcatHttpAttributesExtractor.java @@ -61,7 +61,7 @@ protected String method(Request request) { } @Override - protected @Nullable String flavor(Request request, @Nullable Response response) { + protected @Nullable String flavor(Request request) { String flavor = request.protocol().toString(); if (flavor != null) { // remove HTTP/ prefix to comply with semantic conventions diff --git a/instrumentation/undertow-1.4/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/undertow/UndertowHttpAttributesExtractor.java b/instrumentation/undertow-1.4/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/undertow/UndertowHttpAttributesExtractor.java index 9352914a8ba3..82f8549df2a7 100644 --- a/instrumentation/undertow-1.4/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/undertow/UndertowHttpAttributesExtractor.java +++ b/instrumentation/undertow-1.4/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/undertow/UndertowHttpAttributesExtractor.java @@ -36,7 +36,7 @@ protected String method(HttpServerExchange exchange) { } @Override - protected String flavor(HttpServerExchange exchange, @Nullable HttpServerExchange unused) { + protected String flavor(HttpServerExchange exchange) { String flavor = exchange.getProtocol().toString(); // remove HTTP/ prefix to comply with semantic conventions if (flavor.startsWith("HTTP/")) {