From 192afd39147b0996a6b1ed183fa1116ba1e2ef35 Mon Sep 17 00:00:00 2001 From: Alex Campelo Date: Tue, 25 Jul 2023 15:47:13 -0700 Subject: [PATCH 1/7] use semantic convention APIs for opentelemetry Motivation: Today the tags are manually handcrafted with strings, if the opentelemetry api changes, the tags will be outdated, using the instrumenter available from the io.opentelemetry.instrumentation:opentelemetry-instrumentation-api-semconv we can make it all dynamic based on the specification without understanding the specification and its tags. Result: API will become more stable and evolve easier together with opentelemetry, no more handcrafted strings and the conventions will be driven by the instrumentation API. --- gradle.properties | 2 +- servicetalk-opentelemetry-http/build.gradle | 4 +- .../http/OpenTelemetryHttpRequestFilter.java | 98 +++++++++++++------ .../http/OpenTelemetryHttpServerFilter.java | 91 +++++++++++------ .../http/OpentelemetryOptions.java | 98 +++++++++++++++++++ .../http/RequestHeadersPropagatorGetter.java | 45 +++++++++ ...va => RequestHeadersPropagatorSetter.java} | 20 ++-- .../http/RequestTagExtractor.java | 78 --------------- .../opentelemetry/http/ScopeTracker.java | 51 +++------- ...etalkHttpClientCommonAttributesGetter.java | 82 ++++++++++++++++ ...etalkHttpServerCommonAttributesGetter.java | 94 ++++++++++++++++++ .../ServicetalkNetClientAttributesGetter.java | 81 +++++++++++++++ .../ServicetalkNetServerAttributesGetter.java | 80 +++++++++++++++ .../http/ServicetalkSpanStatusExtractor.java | 57 +++++++++++ .../OpenTelemetryHttpRequestFilterTest.java | 69 ++++++++++++- .../OpenTelemetryHttpServerFilterTest.java | 89 +++++++++++++---- 16 files changed, 834 insertions(+), 205 deletions(-) create mode 100644 servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/OpentelemetryOptions.java create mode 100755 servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/RequestHeadersPropagatorGetter.java rename servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/{ResponseTagExtractor.java => RequestHeadersPropagatorSetter.java} (52%) mode change 100644 => 100755 delete mode 100644 servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/RequestTagExtractor.java create mode 100644 servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ServicetalkHttpClientCommonAttributesGetter.java create mode 100644 servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ServicetalkHttpServerCommonAttributesGetter.java create mode 100644 servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ServicetalkNetClientAttributesGetter.java create mode 100644 servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ServicetalkNetServerAttributesGetter.java create mode 100644 servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ServicetalkSpanStatusExtractor.java diff --git a/gradle.properties b/gradle.properties index 787c7252d1..31bc451773 100644 --- a/gradle.properties +++ b/gradle.properties @@ -54,6 +54,7 @@ jacksonVersion=2.14.3 openTracingVersion=0.33.0 zipkinReporterVersion=2.16.4 opentelemetryVersion=1.28.0 +opentelemetryApiVersion=1.28.0-alpha # gRPC protobufGradlePluginVersion=0.9.4 @@ -73,7 +74,6 @@ assertJCoreVersion=3.24.2 hamcrestVersion=2.2 mockitoCoreVersion=4.11.0 spotbugsPluginVersion=5.0.13 -opentelemetryInstrumentationVersion=1.9.2-alpha apacheDirectoryServerVersion=1.5.7 commonsLangVersion=2.6 diff --git a/servicetalk-opentelemetry-http/build.gradle b/servicetalk-opentelemetry-http/build.gradle index c131c41eb7..cc77bb4a7d 100755 --- a/servicetalk-opentelemetry-http/build.gradle +++ b/servicetalk-opentelemetry-http/build.gradle @@ -24,6 +24,7 @@ dependencies { api project(":servicetalk-http-api") api "io.opentelemetry:opentelemetry-api:$opentelemetryVersion" + implementation("io.opentelemetry.instrumentation:opentelemetry-instrumentation-api-semconv:$opentelemetryApiVersion") implementation project(":servicetalk-annotations") implementation project(":servicetalk-http-utils") @@ -36,7 +37,8 @@ dependencies { testImplementation project(":servicetalk-http-netty") testImplementation project(":servicetalk-test-resources") testImplementation "io.opentelemetry:opentelemetry-sdk-testing:$opentelemetryVersion" - testImplementation "io.opentelemetry.instrumentation:opentelemetry-log4j-2.13.2:$opentelemetryInstrumentationVersion" + testRuntimeOnly("io.opentelemetry.instrumentation:opentelemetry-log4j-context-data-2.17-autoconfigure:" + + "$opentelemetryApiVersion") testImplementation "org.junit.jupiter:junit-jupiter-api" testImplementation "org.assertj:assertj-core:$assertJCoreVersion" testImplementation "org.mockito:mockito-core:$mockitoCoreVersion" diff --git a/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/OpenTelemetryHttpRequestFilter.java b/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/OpenTelemetryHttpRequestFilter.java index c95aae4f9d..99029467d1 100755 --- a/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/OpenTelemetryHttpRequestFilter.java +++ b/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/OpenTelemetryHttpRequestFilter.java @@ -20,6 +20,8 @@ import io.servicetalk.concurrent.api.Single; import io.servicetalk.http.api.FilterableStreamingHttpClient; import io.servicetalk.http.api.FilterableStreamingHttpConnection; +import io.servicetalk.http.api.HttpRequestMetaData; +import io.servicetalk.http.api.HttpResponseMetaData; import io.servicetalk.http.api.StreamingHttpClientFilter; import io.servicetalk.http.api.StreamingHttpClientFilterFactory; import io.servicetalk.http.api.StreamingHttpConnectionFilter; @@ -27,15 +29,22 @@ import io.servicetalk.http.api.StreamingHttpRequest; import io.servicetalk.http.api.StreamingHttpRequester; import io.servicetalk.http.api.StreamingHttpResponse; -import io.servicetalk.transport.api.HostAndPort; import io.opentelemetry.api.GlobalOpenTelemetry; import io.opentelemetry.api.OpenTelemetry; import io.opentelemetry.api.trace.Span; -import io.opentelemetry.api.trace.SpanKind; import io.opentelemetry.api.trace.Tracer; import io.opentelemetry.context.Context; import io.opentelemetry.context.Scope; +import io.opentelemetry.instrumentation.api.instrumenter.AttributesExtractor; +import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter; +import io.opentelemetry.instrumentation.api.instrumenter.InstrumenterBuilder; +import io.opentelemetry.instrumentation.api.instrumenter.SpanNameExtractor; +import io.opentelemetry.instrumentation.api.instrumenter.http.HttpClientAttributesExtractor; +import io.opentelemetry.instrumentation.api.instrumenter.http.HttpClientMetrics; +import io.opentelemetry.instrumentation.api.instrumenter.http.HttpSpanNameExtractor; +import io.opentelemetry.instrumentation.api.instrumenter.net.NetClientAttributesExtractor; +import io.opentelemetry.semconv.trace.attributes.SemanticAttributes; import java.util.function.UnaryOperator; @@ -54,17 +63,42 @@ public final class OpenTelemetryHttpRequestFilter extends AbstractOpenTelemetryFilter implements StreamingHttpClientFilterFactory, StreamingHttpConnectionFilterFactory { - private final String componentName; + private final Instrumenter instrumenter; /** * Create a new instance. * - * @param openTelemetry the {@link OpenTelemetry}. - * @param componentName The component name used during building new spans. + * @param openTelemetry the {@link OpenTelemetry}. + * @param componentName The component name used during building new spans. + * @param opentelemetryOptions extra options to create the opentelemetry filter. */ - public OpenTelemetryHttpRequestFilter(final OpenTelemetry openTelemetry, String componentName) { + public OpenTelemetryHttpRequestFilter(final OpenTelemetry openTelemetry, String componentName, + OpentelemetryOptions opentelemetryOptions) { super(openTelemetry); - this.componentName = componentName.trim(); + SpanNameExtractor serverSpanNameExtractor = + HttpSpanNameExtractor.create(ServicetalkHttpClientCommonAttributesGetter.INSTANCE); + InstrumenterBuilder clientInstrumenterBuilder = + Instrumenter.builder(openTelemetry, INSTRUMENTATION_SCOPE_NAME, serverSpanNameExtractor); + clientInstrumenterBuilder.setSpanStatusExtractor(ServicetalkSpanStatusExtractor.INSTANCE); + + clientInstrumenterBuilder + .addAttributesExtractor(HttpClientAttributesExtractor + .builder(ServicetalkHttpClientCommonAttributesGetter.INSTANCE, + ServicetalkNetClientAttributesGetter.INSTANCE) + .setCapturedRequestHeaders(opentelemetryOptions.getCaptureRequestHeaders()) + .setCapturedResponseHeaders(opentelemetryOptions.getCaptureResponseHeaders()) + .build()) + .addAttributesExtractor( + NetClientAttributesExtractor.create(ServicetalkNetClientAttributesGetter.INSTANCE)); + if (opentelemetryOptions.isEnableMetrics()) { + clientInstrumenterBuilder.addOperationMetrics(HttpClientMetrics.get()); + } + if (!componentName.trim().isEmpty()) { + clientInstrumenterBuilder.addAttributesExtractor( + AttributesExtractor.constant(SemanticAttributes.PEER_SERVICE, componentName)); + } + instrumenter = + clientInstrumenterBuilder.buildClientInstrumenter(RequestHeadersPropagatorSetter.INSTANCE); } /** @@ -73,7 +107,27 @@ public OpenTelemetryHttpRequestFilter(final OpenTelemetry openTelemetry, String * @param componentName The component name used during building new spans. */ public OpenTelemetryHttpRequestFilter(String componentName) { - this(GlobalOpenTelemetry.get(), componentName); + this(GlobalOpenTelemetry.get(), componentName, OpentelemetryOptions.newBuilder().build()); + } + + /** + * Create a new instance, searching for any instance of an opentelemetry available. + * + * @param openTelemetry the {@link OpenTelemetry}. + * @param componentName The component name used during building new spans. + */ + public OpenTelemetryHttpRequestFilter(final OpenTelemetry openTelemetry, String componentName) { + this(openTelemetry, componentName, OpentelemetryOptions.newBuilder().build()); + } + + /** + * Create a new instance, searching for any instance of an opentelemetry available. + * + * @param componentName The component name used during building new spans. + * @param opentelemetryOptions extra options to create the opentelemetry filter + */ + public OpenTelemetryHttpRequestFilter(String componentName, OpentelemetryOptions opentelemetryOptions) { + this(GlobalOpenTelemetry.get(), componentName, opentelemetryOptions); } /** @@ -81,7 +135,7 @@ public OpenTelemetryHttpRequestFilter(String componentName) { * using the hostname as the component name. */ public OpenTelemetryHttpRequestFilter() { - this(GlobalOpenTelemetry.get(), ""); + this(GlobalOpenTelemetry.get(), "", OpentelemetryOptions.newBuilder().build()); } @Override @@ -108,18 +162,13 @@ public Single request(final StreamingHttpRequest request) private Single trackRequest(final StreamingHttpRequester delegate, final StreamingHttpRequest request) { - Context context = Context.current(); - final Span span = RequestTagExtractor.reportTagsAndStart(tracer - .spanBuilder(getSpanName(request)) - .setParent(context) - .setSpanKind(SpanKind.CLIENT), request); - - final Scope scope = span.makeCurrent(); - final ScopeTracker tracker = new ScopeTracker(scope, span); + final Context parentContext = Context.current(); + Context context = instrumenter.start(parentContext, request); + + final Scope scope = context.makeCurrent(); + final ScopeTracker tracker = new ScopeTracker(scope, context, request, instrumenter); Single response; try { - propagators.getTextMapPropagator().inject(Context.current(), request.headers(), - HeadersPropagatorSetter.INSTANCE); response = delegate.request(request); } catch (Throwable t) { tracker.onError(t); @@ -127,15 +176,4 @@ private Single trackRequest(final StreamingHttpRequester } return tracker.track(response); } - - private String getSpanName(StreamingHttpRequest request) { - if (!componentName.isEmpty()) { - return componentName; - } - HostAndPort hostAndPort = request.effectiveHostAndPort(); - if (hostAndPort != null) { - return hostAndPort.hostName(); - } - return request.requestTarget(); - } } diff --git a/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/OpenTelemetryHttpServerFilter.java b/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/OpenTelemetryHttpServerFilter.java index a425be810f..b97ccda49d 100755 --- a/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/OpenTelemetryHttpServerFilter.java +++ b/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/OpenTelemetryHttpServerFilter.java @@ -19,6 +19,7 @@ import io.servicetalk.concurrent.api.Publisher; import io.servicetalk.concurrent.api.Single; import io.servicetalk.http.api.HttpRequestMetaData; +import io.servicetalk.http.api.HttpResponseMetaData; import io.servicetalk.http.api.HttpServiceContext; import io.servicetalk.http.api.StreamingHttpRequest; import io.servicetalk.http.api.StreamingHttpResponse; @@ -30,10 +31,16 @@ import io.opentelemetry.api.GlobalOpenTelemetry; import io.opentelemetry.api.OpenTelemetry; import io.opentelemetry.api.trace.Span; -import io.opentelemetry.api.trace.SpanKind; import io.opentelemetry.api.trace.Tracer; import io.opentelemetry.context.Context; import io.opentelemetry.context.Scope; +import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter; +import io.opentelemetry.instrumentation.api.instrumenter.InstrumenterBuilder; +import io.opentelemetry.instrumentation.api.instrumenter.SpanNameExtractor; +import io.opentelemetry.instrumentation.api.instrumenter.http.HttpServerAttributesExtractor; +import io.opentelemetry.instrumentation.api.instrumenter.http.HttpServerMetrics; +import io.opentelemetry.instrumentation.api.instrumenter.http.HttpSpanNameExtractor; +import io.opentelemetry.instrumentation.api.instrumenter.net.NetServerAttributesExtractor; import java.util.function.UnaryOperator; @@ -52,21 +59,62 @@ */ public final class OpenTelemetryHttpServerFilter extends AbstractOpenTelemetryFilter implements StreamingHttpServiceFilterFactory { + private final Instrumenter instrumenter; /** * Create a new instance. * - * @param openTelemetry the {@link OpenTelemetry}. + * @param openTelemetry the {@link OpenTelemetry}. + * @param opentelemetryOptions extra options to create the opentelemetry filter. */ - public OpenTelemetryHttpServerFilter(final OpenTelemetry openTelemetry) { + public OpenTelemetryHttpServerFilter(final OpenTelemetry openTelemetry, OpentelemetryOptions opentelemetryOptions) { super(openTelemetry); + SpanNameExtractor serverSpanNameExtractor = + HttpSpanNameExtractor.create(ServicetalkHttpServerCommonAttributesGetter.INSTANCE); + InstrumenterBuilder serverInstrumenterBuilder = + Instrumenter.builder(openTelemetry, INSTRUMENTATION_SCOPE_NAME, serverSpanNameExtractor); + serverInstrumenterBuilder.setSpanStatusExtractor(ServicetalkSpanStatusExtractor.INSTANCE); + + serverInstrumenterBuilder + .addAttributesExtractor(HttpServerAttributesExtractor + .builder(ServicetalkHttpServerCommonAttributesGetter.INSTANCE, + ServicetalkNetServerAttributesGetter.INSTANCE) + .setCapturedRequestHeaders(opentelemetryOptions.getCaptureRequestHeaders()) + .setCapturedResponseHeaders(opentelemetryOptions.getCaptureResponseHeaders()) + .build()) + .addAttributesExtractor( + NetServerAttributesExtractor.create(ServicetalkNetServerAttributesGetter.INSTANCE)); + if (opentelemetryOptions.isEnableMetrics()) { + serverInstrumenterBuilder.addOperationMetrics(HttpServerMetrics.get()); + } + + instrumenter = + serverInstrumenterBuilder.buildServerInstrumenter(RequestHeadersPropagatorGetter.INSTANCE); } /** * Create a new Instance, searching for any instance of an opentelemetry available. */ public OpenTelemetryHttpServerFilter() { - this(GlobalOpenTelemetry.get()); + this(GlobalOpenTelemetry.get(), OpentelemetryOptions.newBuilder().build()); + } + + /** + * Create a new instance. + * + * @param opentelemetryOptions extra options to create the opentelemetry filter + */ + public OpenTelemetryHttpServerFilter(OpentelemetryOptions opentelemetryOptions) { + this(GlobalOpenTelemetry.get(), opentelemetryOptions); + } + + /** + * Create a new instance. + * + * @param openTelemetry the {@link OpenTelemetry}. + */ + public OpenTelemetryHttpServerFilter(final OpenTelemetry openTelemetry) { + this(openTelemetry, OpentelemetryOptions.newBuilder().build()); } @Override @@ -85,26 +133,15 @@ private Single trackRequest(final StreamingHttpService de final HttpServiceContext ctx, final StreamingHttpRequest request, final StreamingHttpResponseFactory responseFactory) { - final Context context = Context.root(); - io.opentelemetry.context.Context tracingContext = - propagators.getTextMapPropagator().extract(context, request.headers(), HeadersPropagatorGetter.INSTANCE); - final Span span = RequestTagExtractor.reportTagsAndStart(tracer - .spanBuilder(getOperationName(request)) - .setParent(tracingContext) - .setSpanKind(SpanKind.SERVER), request); + final Context parentContext = Context.current(); + if (!instrumenter.shouldStart(parentContext, request)) { + return delegate.handle(ctx, request, responseFactory); + } + Context context = instrumenter.start(parentContext, request); - final Scope scope = span.makeCurrent(); - final ScopeTracker tracker = new ScopeTracker(scope, span) { - @Override - protected void tagStatusCode() { - super.tagStatusCode(); - if (metaData != null) { - propagators.getTextMapPropagator().inject(Context.current(), metaData.headers(), - HeadersPropagatorSetter.INSTANCE); - } - } - }; + final Scope scope = context.makeCurrent(); + final ScopeTracker tracker = new ScopeTracker(scope, context, request, instrumenter); Single response; try { response = delegate.handle(ctx, request, responseFactory); @@ -114,14 +151,4 @@ protected void tagStatusCode() { } return tracker.track(response); } - - /** - * Get the operation name to build the span with. - * - * @param metaData The {@link HttpRequestMetaData}. - * @return the operation name to build the span with. - */ - private static String getOperationName(HttpRequestMetaData metaData) { - return metaData.method().name() + ' ' + metaData.path(); - } } diff --git a/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/OpentelemetryOptions.java b/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/OpentelemetryOptions.java new file mode 100644 index 0000000000..fa57ed5520 --- /dev/null +++ b/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/OpentelemetryOptions.java @@ -0,0 +1,98 @@ +/* + * Copyright © 2023 Apple Inc. and the ServiceTalk project authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.servicetalk.opentelemetry.http; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.Objects; + +/** + * A set of options for creating the opentelemetry filter. + */ +public final class OpentelemetryOptions { + + private final List captureRequestHeaders; + private final List captureResponseHeaders; + private final boolean enableMetrics; + + OpentelemetryOptions(List captureRequestHeaders, List captureResponseHeaders, + boolean enableMetrics) { + this.captureRequestHeaders = captureRequestHeaders; + this.captureResponseHeaders = captureResponseHeaders; + this.enableMetrics = enableMetrics; + } + + public static Builder newBuilder() { + return new Builder(); + } + + public List getCaptureRequestHeaders() { + return Collections.unmodifiableList(captureRequestHeaders); + } + + public List getCaptureResponseHeaders() { + return Collections.unmodifiableList(captureResponseHeaders); + } + + public boolean isEnableMetrics() { + return enableMetrics; + } + + /** + * a Builder of {@link OpentelemetryOptions}. + */ + public static class Builder { + private List captureRequestHeaders = new ArrayList<>(); + private List captureResponseHeaders = new ArrayList<>(); + private boolean enableMetrics; + + /** + * set the headers to be captured as extra tags. + * @param captureRequestHeaders extra headers to be captured in client/server requests and added as tags. + * @return an instance of itself + */ + public OpentelemetryOptions.Builder captureRequestHeaders(List captureRequestHeaders) { + this.captureRequestHeaders.addAll(Objects.requireNonNull(captureRequestHeaders)); + return this; + } + + /** + * set the headers to be captured as extra tags. + * @param captureResponseHeaders extra headers to be captured in client/server response and added as tags. + * @return an instance of itself + */ + public OpentelemetryOptions.Builder captureResponseHeaders(List captureResponseHeaders) { + this.captureResponseHeaders.addAll(Objects.requireNonNull(captureResponseHeaders)); + return this; + } + + /** + * whether to enable span metrics. + * @param enableMetrics whether to enable opentelemetry metrics. + * @return an instance of itself + */ + public OpentelemetryOptions.Builder enableMetrics(boolean enableMetrics) { + this.enableMetrics = enableMetrics; + return this; + } + + public OpentelemetryOptions build() { + return new OpentelemetryOptions(captureRequestHeaders, captureResponseHeaders, enableMetrics); + } + } +} diff --git a/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/RequestHeadersPropagatorGetter.java b/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/RequestHeadersPropagatorGetter.java new file mode 100755 index 0000000000..1ee1ed7195 --- /dev/null +++ b/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/RequestHeadersPropagatorGetter.java @@ -0,0 +1,45 @@ +/* + * Copyright © 2022 Apple Inc. and the ServiceTalk project authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.servicetalk.opentelemetry.http; + +import io.servicetalk.http.api.HttpRequestMetaData; + +import io.opentelemetry.context.propagation.TextMapGetter; + +import javax.annotation.Nullable; + +final class RequestHeadersPropagatorGetter implements TextMapGetter { + + static final TextMapGetter INSTANCE = new RequestHeadersPropagatorGetter(); + + private RequestHeadersPropagatorGetter() { + } + + @Override + public Iterable keys(final HttpRequestMetaData carrier) { + return HeadersPropagatorGetter.INSTANCE.keys(carrier.headers()); + } + + @Override + @Nullable + public String get(@Nullable HttpRequestMetaData carrier, final String key) { + if (carrier == null) { + return null; + } + return HeadersPropagatorGetter.INSTANCE.get(carrier.headers(), key); + } +} diff --git a/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ResponseTagExtractor.java b/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/RequestHeadersPropagatorSetter.java old mode 100644 new mode 100755 similarity index 52% rename from servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ResponseTagExtractor.java rename to servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/RequestHeadersPropagatorSetter.java index f2a4ca7d08..165de1a602 --- a/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ResponseTagExtractor.java +++ b/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/RequestHeadersPropagatorSetter.java @@ -16,15 +16,23 @@ package io.servicetalk.opentelemetry.http; -import io.servicetalk.http.api.HttpResponseMetaData; +import io.servicetalk.http.api.HttpRequestMetaData; -import io.opentelemetry.api.trace.Span; +import io.opentelemetry.context.propagation.TextMapSetter; -final class ResponseTagExtractor { +import javax.annotation.Nullable; - public static final ResponseTagExtractor INSTANCE = new ResponseTagExtractor(); +final class RequestHeadersPropagatorSetter implements TextMapSetter { - void extract(HttpResponseMetaData responseMetaData, Span span) { - span.setAttribute("http.status_code", responseMetaData.status().code()); + static final TextMapSetter INSTANCE = new RequestHeadersPropagatorSetter(); + + private RequestHeadersPropagatorSetter() { + } + + @Override + public void set(@Nullable final HttpRequestMetaData headers, final String key, final String value) { + if (headers != null) { + HeadersPropagatorSetter.INSTANCE.set(headers.headers(), key, value); + } } } diff --git a/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/RequestTagExtractor.java b/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/RequestTagExtractor.java deleted file mode 100644 index d2c3612cf0..0000000000 --- a/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/RequestTagExtractor.java +++ /dev/null @@ -1,78 +0,0 @@ -/* - * Copyright © 2022 Apple Inc. and the ServiceTalk project authors - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package io.servicetalk.opentelemetry.http; - -import io.servicetalk.http.api.HttpProtocolVersion; -import io.servicetalk.http.api.HttpRequestMetaData; -import io.servicetalk.transport.api.HostAndPort; - -import io.opentelemetry.api.trace.Span; -import io.opentelemetry.api.trace.SpanBuilder; - -import static io.servicetalk.http.api.HttpHeaderNames.USER_AGENT; - -final class RequestTagExtractor { - - private RequestTagExtractor() { - // empty private constructor - } - - private static String getRequestMethod(HttpRequestMetaData req) { - return req.method().name(); - } - - private static String getHttpUrl(HttpRequestMetaData req) { - return req.path() - + (req.rawQuery() == null ? "" : '?' + req.rawQuery()); - } - - static Span reportTagsAndStart(SpanBuilder span, HttpRequestMetaData httpRequestMetaData) { - span.setAttribute("http.url", getHttpUrl(httpRequestMetaData)); - span.setAttribute("http.method", getRequestMethod(httpRequestMetaData)); - span.setAttribute("http.target", getHttpUrl(httpRequestMetaData)); - span.setAttribute("http.route", httpRequestMetaData.rawPath()); - span.setAttribute("http.flavor", getFlavor(httpRequestMetaData.version())); - CharSequence userAgent = httpRequestMetaData.headers().get(USER_AGENT); - if (userAgent != null) { - span.setAttribute("http.user_agent", userAgent.toString()); - } - String scheme = httpRequestMetaData.scheme(); - if (scheme != null) { - span.setAttribute("http.scheme", scheme); - } - HostAndPort hostAndPort = httpRequestMetaData.effectiveHostAndPort(); - if (hostAndPort != null) { - span.setAttribute("net.host.name", hostAndPort.hostName()); - span.setAttribute("net.host.port", hostAndPort.port()); - } - return span.startSpan(); - } - - private static String getFlavor(final HttpProtocolVersion version) { - if (version.major() == 1) { - if (version.minor() == 1) { - return "1.1"; - } - if (version.minor() == 0) { - return "1.0"; - } - } else if (version.major() == 2 && version.minor() == 0) { - return "2.0"; - } - return version.major() + "." + version.minor(); - } -} diff --git a/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ScopeTracker.java b/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ScopeTracker.java index 1171898343..67a61c391b 100755 --- a/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ScopeTracker.java +++ b/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ScopeTracker.java @@ -18,30 +18,36 @@ import io.servicetalk.concurrent.api.Single; import io.servicetalk.concurrent.api.TerminalSignalConsumer; +import io.servicetalk.http.api.HttpRequestMetaData; import io.servicetalk.http.api.HttpResponseMetaData; +import io.servicetalk.http.api.StreamingHttpRequest; import io.servicetalk.http.api.StreamingHttpResponse; import io.servicetalk.http.utils.BeforeFinallyHttpOperator; -import io.opentelemetry.api.trace.Span; -import io.opentelemetry.api.trace.StatusCode; +import io.opentelemetry.context.Context; import io.opentelemetry.context.Scope; +import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter; import javax.annotation.Nullable; -import static io.servicetalk.http.api.HttpResponseStatus.StatusClass.SERVER_ERROR_5XX; import static java.util.Objects.requireNonNull; class ScopeTracker implements TerminalSignalConsumer { private final Scope currentScope; - private final Span span; + private final Context context; + private final StreamingHttpRequest request; + private final Instrumenter instrumenter; @Nullable protected HttpResponseMetaData metaData; - ScopeTracker(Scope currentScope, final Span span) { + ScopeTracker(Scope currentScope, Context context, StreamingHttpRequest request, + Instrumenter instrumenter) { this.currentScope = requireNonNull(currentScope); - this.span = requireNonNull(span); + this.context = requireNonNull(context); + this.request = requireNonNull(request); + this.instrumenter = requireNonNull(instrumenter); } void onResponseMeta(final HttpResponseMetaData metaData) { @@ -51,11 +57,8 @@ void onResponseMeta(final HttpResponseMetaData metaData) { @Override public void onComplete() { assert metaData != null : "can't have succeeded without capturing metadata first"; - tagStatusCode(); try { - if (isError(metaData)) { - span.setStatus(StatusCode.ERROR); - } + instrumenter.end(context, request, metaData, null); } finally { closeAll(); } @@ -64,8 +67,7 @@ public void onComplete() { @Override public void onError(final Throwable throwable) { try { - tagStatusCode(); - span.setStatus(StatusCode.ERROR); + instrumenter.end(context, request, metaData, throwable); } finally { closeAll(); } @@ -74,23 +76,12 @@ public void onError(final Throwable throwable) { @Override public void cancel() { try { - tagStatusCode(); - span.setStatus(StatusCode.ERROR); + instrumenter.end(context, request, metaData, null); } finally { closeAll(); } } - /** - * Determine if a {@link HttpResponseMetaData} should be considered an error from a tracing perspective. - * - * @param metaData The {@link HttpResponseMetaData} to test. - * @return {@code true} if the {@link HttpResponseMetaData} should be considered an error for tracing. - */ - private static boolean isError(final HttpResponseMetaData metaData) { - return metaData.status().statusClass() == SERVER_ERROR_5XX; - } - Single track(Single responseSingle) { return responseSingle.liftSync(new BeforeFinallyHttpOperator(this)) // BeforeFinallyHttpOperator conditionally outputs a Single with a failed @@ -100,17 +91,7 @@ Single track(Single responseSingle .beforeOnSuccess(this::onResponseMeta); } - void tagStatusCode() { - if (metaData != null) { - ResponseTagExtractor.INSTANCE.extract(metaData, span); - } - } - private void closeAll() { - try { - currentScope.close(); - } finally { - span.end(); - } + currentScope.close(); } } diff --git a/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ServicetalkHttpClientCommonAttributesGetter.java b/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ServicetalkHttpClientCommonAttributesGetter.java new file mode 100644 index 0000000000..b9575dd3e5 --- /dev/null +++ b/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ServicetalkHttpClientCommonAttributesGetter.java @@ -0,0 +1,82 @@ +/* + * Copyright © 2023 Apple Inc. and the ServiceTalk project authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.servicetalk.opentelemetry.http; + +import io.servicetalk.http.api.HttpRequestMetaData; +import io.servicetalk.http.api.HttpResponseMetaData; +import io.servicetalk.transport.api.HostAndPort; + +import io.opentelemetry.instrumentation.api.instrumenter.http.HttpClientAttributesGetter; + +import java.util.Collections; +import java.util.List; +import javax.annotation.Nullable; + +final class ServicetalkHttpClientCommonAttributesGetter + implements HttpClientAttributesGetter { + + static final ServicetalkHttpClientCommonAttributesGetter INSTANCE = + new ServicetalkHttpClientCommonAttributesGetter(); + + private ServicetalkHttpClientCommonAttributesGetter() { + } + + @Override + public String getHttpRequestMethod(HttpRequestMetaData httpRequestMetaData) { + return httpRequestMetaData.method().name(); + } + + @Override + public List getHttpRequestHeader(HttpRequestMetaData httpRequestMetaData, String name) { + CharSequence value = httpRequestMetaData.headers().get(name); + if (value != null) { + return Collections.singletonList(value.toString()); + } + return Collections.emptyList(); + } + + @Override + public Integer getHttpResponseStatusCode(HttpRequestMetaData httpRequestMetaData, + HttpResponseMetaData httpResponseMetaData, + @Nullable Throwable error) { + return httpResponseMetaData.status().code(); + } + + @Override + public List getHttpResponseHeader(HttpRequestMetaData httpRequestMetaData, + HttpResponseMetaData httpResponseMetaData, + String name) { + CharSequence value = httpResponseMetaData.headers().get(name); + if (value != null) { + return Collections.singletonList(value.toString()); + } + return Collections.emptyList(); + } + + @Nullable + @Override + public String getUrlFull(HttpRequestMetaData request) { + HostAndPort effectiveHostAndPort = request.effectiveHostAndPort(); + String requestScheme = request.scheme() != null ? request.scheme() + : "http"; + CharSequence hostAndPort = request.headers().contains("host") ? request.headers().get("host") + : effectiveHostAndPort != null ? + String.format("%s:%d", effectiveHostAndPort.hostName(), effectiveHostAndPort.port()) : null; + return hostAndPort != null ? String.format("%s://%s%s", requestScheme, hostAndPort, request.path()) : + request.path(); + } +} diff --git a/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ServicetalkHttpServerCommonAttributesGetter.java b/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ServicetalkHttpServerCommonAttributesGetter.java new file mode 100644 index 0000000000..66c915f197 --- /dev/null +++ b/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ServicetalkHttpServerCommonAttributesGetter.java @@ -0,0 +1,94 @@ +/* + * Copyright © 2023 Apple Inc. and the ServiceTalk project authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.servicetalk.opentelemetry.http; + +import io.servicetalk.http.api.HttpRequestMetaData; +import io.servicetalk.http.api.HttpResponseMetaData; + +import io.opentelemetry.instrumentation.api.instrumenter.http.HttpServerAttributesGetter; + +import java.util.Collections; +import java.util.List; +import javax.annotation.Nullable; + +final class ServicetalkHttpServerCommonAttributesGetter + implements HttpServerAttributesGetter { + + static final ServicetalkHttpServerCommonAttributesGetter INSTANCE = + new ServicetalkHttpServerCommonAttributesGetter(); + + private ServicetalkHttpServerCommonAttributesGetter() { + } + + @Nullable + @Override + public String getUrlScheme(HttpRequestMetaData httpRequestMetaData) { + return httpRequestMetaData.scheme(); + } + + @Nullable + @Override + public String getUrlPath(HttpRequestMetaData httpRequestMetaData) { + return httpRequestMetaData.path(); + } + + @Nullable + @Override + public String getUrlQuery(HttpRequestMetaData httpRequestMetaData) { + return httpRequestMetaData.query(); + } + + @Nullable + @Override + public String getHttpRequestMethod(HttpRequestMetaData httpRequestMetaData) { + return httpRequestMetaData.method().name(); + } + + @Override + public List getHttpRequestHeader(HttpRequestMetaData httpRequestMetaData, String name) { + CharSequence value = httpRequestMetaData.headers().get(name); + if (value != null) { + return Collections.singletonList(value.toString()); + } + return Collections.emptyList(); + } + + @Nullable + @Override + public Integer getHttpResponseStatusCode(HttpRequestMetaData httpRequestMetaData, + HttpResponseMetaData httpResponseMetaData, + @Nullable Throwable error) { + return httpResponseMetaData.status().code(); + } + + @Override + public List getHttpResponseHeader(HttpRequestMetaData httpRequestMetaData, + HttpResponseMetaData httpResponseMetaData, + String name) { + CharSequence value = httpResponseMetaData.headers().get(name); + if (value != null) { + return Collections.singletonList(value.toString()); + } + return Collections.emptyList(); + } + + @Nullable + @Override + public String getHttpRoute(HttpRequestMetaData httpRequestMetaData) { + return httpRequestMetaData.path(); + } +} diff --git a/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ServicetalkNetClientAttributesGetter.java b/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ServicetalkNetClientAttributesGetter.java new file mode 100644 index 0000000000..a26b1efd39 --- /dev/null +++ b/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ServicetalkNetClientAttributesGetter.java @@ -0,0 +1,81 @@ +/* + * Copyright © 2023 Apple Inc. and the ServiceTalk project authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.servicetalk.opentelemetry.http; + +import io.servicetalk.http.api.HttpProtocolVersion; +import io.servicetalk.http.api.HttpRequestMetaData; +import io.servicetalk.http.api.HttpResponseMetaData; +import io.servicetalk.transport.api.HostAndPort; + +import io.opentelemetry.instrumentation.api.instrumenter.net.NetClientAttributesGetter; + +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. + */ +final class ServicetalkNetClientAttributesGetter + implements NetClientAttributesGetter { + static final ServicetalkNetClientAttributesGetter INSTANCE = new ServicetalkNetClientAttributesGetter(); + + private ServicetalkNetClientAttributesGetter() { + } + + @Nullable + @Override + public String getNetworkProtocolName(HttpRequestMetaData request, @Nullable HttpResponseMetaData response) { + if (response == null) { + return null; + } + return "http"; + } + + @Nullable + @Override + public String getNetworkProtocolVersion(HttpRequestMetaData request, + @Nullable HttpResponseMetaData response) { + if (response == null) { + return null; + } + HttpProtocolVersion version = response.version(); + if (version.major() == 1) { + if (version.minor() == 1) { + return "1.1"; + } + if (version.minor() == 0) { + return "1.0"; + } + } else if (version.major() == 2 && version.minor() == 0) { + return "2.0"; + } + return version.major() + "." + version.minor(); + } + + @Override + @Nullable + public String getServerAddress(HttpRequestMetaData request) { + HostAndPort effectiveHostAndPort = request.effectiveHostAndPort(); + return effectiveHostAndPort != null ? effectiveHostAndPort.hostName() : null; + } + + @Override + public Integer getServerPort(HttpRequestMetaData request) { + HostAndPort effectiveHostAndPort = request.effectiveHostAndPort(); + return effectiveHostAndPort != null ? effectiveHostAndPort.port() : null; + } +} diff --git a/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ServicetalkNetServerAttributesGetter.java b/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ServicetalkNetServerAttributesGetter.java new file mode 100644 index 0000000000..bbcb1fc58a --- /dev/null +++ b/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ServicetalkNetServerAttributesGetter.java @@ -0,0 +1,80 @@ +/* + * Copyright © 2023 Apple Inc. and the ServiceTalk project authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.servicetalk.opentelemetry.http; + +import io.servicetalk.http.api.HttpProtocolVersion; +import io.servicetalk.http.api.HttpRequestMetaData; +import io.servicetalk.http.api.HttpResponseMetaData; +import io.servicetalk.transport.api.HostAndPort; + +import io.opentelemetry.instrumentation.api.instrumenter.net.NetServerAttributesGetter; + +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. + */ +final class ServicetalkNetServerAttributesGetter + implements NetServerAttributesGetter { + + static final ServicetalkNetServerAttributesGetter INSTANCE = new ServicetalkNetServerAttributesGetter(); + + private ServicetalkNetServerAttributesGetter() { + } + + @Nullable + @Override + public String getNetworkProtocolName(HttpRequestMetaData request, @Nullable HttpResponseMetaData response) { + + if (response == null) { + return null; + } + return "http"; + } + + @Nullable + @Override + public String getNetworkProtocolVersion(HttpRequestMetaData request, + @Nullable HttpResponseMetaData response) { + HttpProtocolVersion version = request.version(); + if (version.major() == 1) { + if (version.minor() == 1) { + return "1.1"; + } + if (version.minor() == 0) { + return "1.0"; + } + } else if (version.major() == 2 && version.minor() == 0) { + return "2.0"; + } + return version.major() + "." + version.minor(); + } + + @Override + @Nullable + public String getServerAddress(HttpRequestMetaData request) { + HostAndPort effectiveHostAndPort = request.effectiveHostAndPort(); + return effectiveHostAndPort != null ? effectiveHostAndPort.hostName() : null; + } + + @Override + public Integer getServerPort(HttpRequestMetaData request) { + HostAndPort effectiveHostAndPort = request.effectiveHostAndPort(); + return effectiveHostAndPort != null ? effectiveHostAndPort.port() : null; + } +} diff --git a/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ServicetalkSpanStatusExtractor.java b/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ServicetalkSpanStatusExtractor.java new file mode 100644 index 0000000000..3d1b9b2706 --- /dev/null +++ b/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ServicetalkSpanStatusExtractor.java @@ -0,0 +1,57 @@ +/* + * Copyright © 2023 Apple Inc. and the ServiceTalk project authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.servicetalk.opentelemetry.http; + +import io.servicetalk.http.api.HttpRequestMetaData; +import io.servicetalk.http.api.HttpResponseMetaData; + +import io.opentelemetry.api.trace.StatusCode; +import io.opentelemetry.instrumentation.api.instrumenter.SpanStatusBuilder; +import io.opentelemetry.instrumentation.api.instrumenter.SpanStatusExtractor; + +import javax.annotation.Nullable; + +final class ServicetalkSpanStatusExtractor implements SpanStatusExtractor { + + static final ServicetalkSpanStatusExtractor INSTANCE = new ServicetalkSpanStatusExtractor(); + + private ServicetalkSpanStatusExtractor() { + } + + @Override + public void extract( + SpanStatusBuilder spanStatusBuilder, + HttpRequestMetaData request, + @Nullable HttpResponseMetaData status, + @Nullable Throwable error) { + if (error != null) { + spanStatusBuilder.setStatus(StatusCode.ERROR); + } else if (status != null) { + switch (status.status().statusClass()) { + case INFORMATIONAL_1XX: + case SUCCESSFUL_2XX: + case REDIRECTION_3XX: + spanStatusBuilder.setStatus(StatusCode.OK); + break; + default: + spanStatusBuilder.setStatus(StatusCode.ERROR); + } + } else { + SpanStatusExtractor.getDefault().extract(spanStatusBuilder, request, null, null); + } + } +} diff --git a/servicetalk-opentelemetry-http/src/test/java/io/servicetalk/opentelemetry/http/OpenTelemetryHttpRequestFilterTest.java b/servicetalk-opentelemetry-http/src/test/java/io/servicetalk/opentelemetry/http/OpenTelemetryHttpRequestFilterTest.java index 57eafa9b5d..40b09e5236 100755 --- a/servicetalk-opentelemetry-http/src/test/java/io/servicetalk/opentelemetry/http/OpenTelemetryHttpRequestFilterTest.java +++ b/servicetalk-opentelemetry-http/src/test/java/io/servicetalk/opentelemetry/http/OpenTelemetryHttpRequestFilterTest.java @@ -43,6 +43,7 @@ import org.slf4j.LoggerFactory; import java.lang.invoke.MethodHandles; +import java.util.Collections; import static io.servicetalk.concurrent.api.Single.succeeded; import static io.servicetalk.http.netty.HttpClients.forSingleAddress; @@ -128,9 +129,27 @@ void testInjectWithAParent() throws Exception { ta.hasTraceId(serverSpanState.getTraceId())); otelTesting.assertTraces() - .hasTracesSatisfyingExactly(ta -> - assertThat(ta.getSpan(0).getAttributes().get(SemanticAttributes.HTTP_URL)) - .isEqualTo("/path")); + .hasTracesSatisfyingExactly(ta -> { + SpanData span = ta.getSpan(0); + assertThat(span.getAttributes().get(SemanticAttributes.HTTP_URL)) + .isEqualTo("/path"); + assertThat(span.getAttributes().get(SemanticAttributes.HTTP_METHOD)) + .isEqualTo("GET"); + assertThat(span.getAttributes().get(SemanticAttributes.HTTP_RESPONSE_CONTENT_LENGTH)) + .isGreaterThan(0); + assertThat(span.getAttributes().get(SemanticAttributes.NET_PROTOCOL_VERSION)) + .isEqualTo("1.1"); + assertThat(span.getAttributes().get(SemanticAttributes.NET_PROTOCOL_NAME)) + .isEqualTo("http"); + assertThat(span.getAttributes().get(SemanticAttributes.PEER_SERVICE)) + .isEqualTo("testClient"); + assertThat(span.getAttributes() + .get(AttributeKey.stringArrayKey("http.response.header.my_header"))) + .isNull(); + assertThat(span.getAttributes() + .get(AttributeKey.stringArrayKey("http.request.header.some_request_header"))) + .isNull(); + }); } } } @@ -186,6 +205,47 @@ void testInjectWithAParentCreated() throws Exception { } } + @Test + void testCaptureHeader() throws Exception { + final String requestUrl = "/"; + OpenTelemetry openTelemetry = otelTesting.getOpenTelemetry(); + try (ServerContext context = buildServer(openTelemetry, false)) { + try (HttpClient client = forSingleAddress(serverHostAndPort(context)) + .appendClientFilter(new OpenTelemetryHttpRequestFilter(openTelemetry, "testClient", + OpentelemetryOptions.newBuilder() + .captureResponseHeaders(Collections.singletonList("my-header")) + .captureRequestHeaders(Collections.singletonList("some-request-header")) + .build())) + .appendClientFilter(new TestTracingClientLoggerFilter(TRACING_TEST_LOG_LINE_PREFIX)).build()) { + HttpResponse response = client.request(client.get(requestUrl) + .addHeader("some-request-header", "request-header-value")).toFuture().get(); + TestSpanState serverSpanState = response.payloadBody(SPAN_STATE_SERIALIZER); + + verifyTraceIdPresentInLogs(stableAccumulated(1000), requestUrl, + serverSpanState.getTraceId(), serverSpanState.getSpanId(), + TRACING_TEST_LOG_LINE_PREFIX); + assertThat(otelTesting.getSpans()).hasSize(1); + assertThat(otelTesting.getSpans()).extracting("traceId") + .containsExactly(serverSpanState.getTraceId()); + assertThat(otelTesting.getSpans()).extracting("spanId") + .containsAnyOf(serverSpanState.getSpanId()); + otelTesting.assertTraces() + .hasTracesSatisfyingExactly(ta -> ta.hasTraceId(serverSpanState.getTraceId())); + + otelTesting.assertTraces() + .hasTracesSatisfyingExactly(ta -> { + SpanData span = ta.getSpan(0); + assertThat(span.getAttributes() + .get(AttributeKey.stringArrayKey("http.response.header.my_header"))) + .isEqualTo(Collections.singletonList("header-value")); + assertThat(span.getAttributes() + .get(AttributeKey.stringArrayKey("http.request.header.some_request_header"))) + .isEqualTo(Collections.singletonList("request-header-value")); + }); + } + } + } + private static ServerContext buildServer(OpenTelemetry givenOpentelemetry, boolean addFilter) throws Exception { HttpServerBuilder httpServerBuilder = HttpServers.forAddress(localAddress(0)); if (addFilter) { @@ -200,7 +260,8 @@ private static ServerContext buildServer(OpenTelemetry givenOpentelemetry, boole .extract(context, request.headers(), HeadersPropagatorGetter.INSTANCE); Span span = Span.fromContext(tracingContext); return succeeded( - responseFactory.ok().payloadBody(new TestSpanState(span.getSpanContext()), SPAN_STATE_SERIALIZER)); + responseFactory.ok().addHeader("my-header", "header-value") + .payloadBody(new TestSpanState(span.getSpanContext()), SPAN_STATE_SERIALIZER)); }); } diff --git a/servicetalk-opentelemetry-http/src/test/java/io/servicetalk/opentelemetry/http/OpenTelemetryHttpServerFilterTest.java b/servicetalk-opentelemetry-http/src/test/java/io/servicetalk/opentelemetry/http/OpenTelemetryHttpServerFilterTest.java index c9b866affb..4c92279ad5 100644 --- a/servicetalk-opentelemetry-http/src/test/java/io/servicetalk/opentelemetry/http/OpenTelemetryHttpServerFilterTest.java +++ b/servicetalk-opentelemetry-http/src/test/java/io/servicetalk/opentelemetry/http/OpenTelemetryHttpServerFilterTest.java @@ -43,6 +43,7 @@ import java.io.IOException; import java.net.HttpURLConnection; import java.net.URL; +import java.util.Collections; import static io.servicetalk.concurrent.api.Single.succeeded; import static io.servicetalk.http.netty.HttpClients.forSingleAddress; @@ -91,17 +92,23 @@ void testInjectWithNoParent() throws Exception { otelTesting.assertTraces() .hasTracesSatisfyingExactly(ta -> { SpanData span = ta.getSpan(0); - assertThat(span.getAttributes().get(SemanticAttributes.HTTP_URL)) - .isEqualTo("/path"); + assertThat(span.getAttributes().get(SemanticAttributes.HTTP_STATUS_CODE)) + .isEqualTo(200); assertThat(span.getAttributes().get(SemanticAttributes.HTTP_TARGET)) .isEqualTo("/path"); - assertThat(span.getAttributes().get(SemanticAttributes.HTTP_ROUTE)) - .isEqualTo("/path"); - assertThat(span.getAttributes().get(SemanticAttributes.HTTP_FLAVOR)) + assertThat(span.getAttributes().get(SemanticAttributes.NET_PROTOCOL_NAME)) + .isEqualTo("http"); + assertThat(span.getAttributes().get(SemanticAttributes.NET_PROTOCOL_VERSION)) .isEqualTo("1.1"); assertThat(span.getAttributes().get(SemanticAttributes.HTTP_METHOD)) .isEqualTo("GET"); assertThat(span.getName()).isEqualTo("GET /path"); + assertThat(span.getAttributes() + .get(AttributeKey.stringArrayKey("http.response.header.my_header"))) + .isNull(); + assertThat(span.getAttributes() + .get(AttributeKey.stringArrayKey("http.request.header.some_request_header"))) + .isNull(); }); } } @@ -133,7 +140,7 @@ void testInjectWithAParent() throws Exception { .hasTracesSatisfyingExactly(ta -> { assertThat(ta.getSpan(0).getAttributes().get(SemanticAttributes.HTTP_URL)) .isEqualTo("/path"); - assertThat(ta.getSpan(0).getAttributes().get(SemanticAttributes.HTTP_FLAVOR)) + assertThat(ta.getSpan(0).getAttributes().get(SemanticAttributes.NET_PROTOCOL_VERSION)) .isEqualTo("1.1"); }); } @@ -169,28 +176,68 @@ void testInjectWithNewTrace() throws Exception { } finally { span.end(); } - verifyTraceIdPresentInLogs(stableAccumulated(1000), "/", - serverSpanState.getTraceId(), serverSpanState.getSpanId(), - TRACING_TEST_LOG_LINE_PREFIX); - assertThat(otelTesting.getSpans()).hasSize(2); - assertThat(otelTesting.getSpans()).extracting("traceId") - .containsExactly(serverSpanState.getTraceId(), serverSpanState.getTraceId()); + verifyTraceIdPresentInLogs(stableAccumulated(1000), "/", + serverSpanState.getTraceId(), serverSpanState.getSpanId(), + TRACING_TEST_LOG_LINE_PREFIX); + assertThat(otelTesting.getSpans()).hasSize(2); + assertThat(otelTesting.getSpans()).extracting("traceId") + .containsExactly(serverSpanState.getTraceId(), serverSpanState.getTraceId()); otelTesting.assertTraces() .hasTracesSatisfyingExactly(ta -> { assertThat(ta.getSpan(0).getAttributes().get(SemanticAttributes.HTTP_URL)) - .startsWith(url.toString()); - assertThat(ta.getSpan(1).getAttributes().get(SemanticAttributes.HTTP_URL)) - .isEqualTo("/path?query=this&foo=bar"); + .endsWith(url.toString()); + assertThat(ta.getSpan(1).getAttributes().get(SemanticAttributes.HTTP_METHOD)) + .isEqualTo("GET"); assertThat(ta.getSpan(0).getAttributes().get(AttributeKey.stringKey("component"))) .isEqualTo("serviceTalk"); }); } } - private static ServerContext buildServer(OpenTelemetry givenOpentelemetry) throws Exception { + @Test + void testCaptureHeaders() throws Exception { + final String requestUrl = "/path"; + try (ServerContext context = buildServer(otelTesting.getOpenTelemetry(), + OpentelemetryOptions.newBuilder() + .captureResponseHeaders(Collections.singletonList("my-header")) + .captureRequestHeaders(Collections.singletonList("some-request-header")) + .build())) { + try (HttpClient client = forSingleAddress(serverHostAndPort(context)).build()) { + HttpResponse response = client.request(client.get(requestUrl) + .addHeader("some-request-header", "request-header-value")) + .toFuture().get(); + TestSpanState serverSpanState = response.payloadBody(SPAN_STATE_SERIALIZER); + + verifyTraceIdPresentInLogs(stableAccumulated(1000), requestUrl, + serverSpanState.getTraceId(), serverSpanState.getSpanId(), + TRACING_TEST_LOG_LINE_PREFIX); + assertThat(otelTesting.getSpans()).hasSize(1); + assertThat(otelTesting.getSpans()).extracting("traceId") + .containsExactly(serverSpanState.getTraceId()); + assertThat(otelTesting.getSpans()).extracting("spanId") + .containsAnyOf(serverSpanState.getSpanId()); + otelTesting.assertTraces() + .hasTracesSatisfyingExactly(ta -> ta.hasTraceId(serverSpanState.getTraceId())); + + otelTesting.assertTraces() + .hasTracesSatisfyingExactly(ta -> { + SpanData span = ta.getSpan(0); + assertThat( + span.getAttributes().get(AttributeKey.stringArrayKey("http.response.header.my_header"))) + .isEqualTo(Collections.singletonList("header-value")); + assertThat(span.getAttributes() + .get(AttributeKey.stringArrayKey("http.request.header.some_request_header"))) + .isEqualTo(Collections.singletonList("request-header-value")); + }); + } + } + } + + private static ServerContext buildServer(OpenTelemetry givenOpentelemetry, + OpentelemetryOptions opentelemetryOptions) throws Exception { return HttpServers.forAddress(localAddress(0)) - .appendServiceFilter(new OpenTelemetryHttpServerFilter(givenOpentelemetry)) + .appendServiceFilter(new OpenTelemetryHttpServerFilter(givenOpentelemetry, opentelemetryOptions)) .appendServiceFilter(new TestTracingServerLoggerFilter(TRACING_TEST_LOG_LINE_PREFIX)) .listenAndAwait((ctx, request, responseFactory) -> { final ContextPropagators propagators = givenOpentelemetry.getPropagators(); @@ -202,7 +249,13 @@ private static ServerContext buildServer(OpenTelemetry givenOpentelemetry) throw span = Span.fromContext(tracingContext); } return succeeded( - responseFactory.ok().payloadBody(new TestSpanState(span.getSpanContext()), SPAN_STATE_SERIALIZER)); + responseFactory.ok() + .addHeader("my-header", "header-value") + .payloadBody(new TestSpanState(span.getSpanContext()), SPAN_STATE_SERIALIZER)); }); } + + private static ServerContext buildServer(OpenTelemetry givenOpentelemetry) throws Exception { + return buildServer(givenOpentelemetry, OpentelemetryOptions.newBuilder().build()); + } } From 8a845010c51b5bb56ad060ae0f5d7708745cf377 Mon Sep 17 00:00:00 2001 From: Alex Campelo Date: Fri, 11 Aug 2023 10:07:44 -0700 Subject: [PATCH 2/7] HttpProtocolVersion.fullVersion() and addressing comments --- gradle.properties | 2 +- .../http/api/HttpProtocolVersion.java | 18 +++++++++++++++ .../http/api/HttpProtocolVersionTest.java | 3 +++ servicetalk-opentelemetry-http/build.gradle | 5 ++-- .../http/OpenTelemetryHttpRequestFilter.java | 10 ++++---- .../http/OpenTelemetryHttpServerFilter.java | 10 ++++---- .../http/OpentelemetryOptions.java | 23 +++++++++++-------- ...ervicetalkHttpClientAttributesGetter.java} | 8 +++---- ...ervicetalkHttpServerAttributesGetter.java} | 13 ++++------- .../ServicetalkNetClientAttributesGetter.java | 15 ++---------- .../ServicetalkNetServerAttributesGetter.java | 14 ++--------- .../OpenTelemetryHttpRequestFilterTest.java | 4 ++-- .../OpenTelemetryHttpServerFilterTest.java | 4 ++-- 13 files changed, 64 insertions(+), 65 deletions(-) rename servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/{ServicetalkHttpClientCommonAttributesGetter.java => ServicetalkHttpClientAttributesGetter.java} (92%) rename servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/{ServicetalkHttpServerCommonAttributesGetter.java => ServicetalkHttpServerAttributesGetter.java} (90%) diff --git a/gradle.properties b/gradle.properties index 31bc451773..6ded5ab9fa 100644 --- a/gradle.properties +++ b/gradle.properties @@ -54,7 +54,7 @@ jacksonVersion=2.14.3 openTracingVersion=0.33.0 zipkinReporterVersion=2.16.4 opentelemetryVersion=1.28.0 -opentelemetryApiVersion=1.28.0-alpha +opentelemetryInstrumentationVersion=1.28.0-alpha # gRPC protobufGradlePluginVersion=0.9.4 diff --git a/servicetalk-http-api/src/main/java/io/servicetalk/http/api/HttpProtocolVersion.java b/servicetalk-http-api/src/main/java/io/servicetalk/http/api/HttpProtocolVersion.java index 31b01e9831..7d381d71df 100644 --- a/servicetalk-http-api/src/main/java/io/servicetalk/http/api/HttpProtocolVersion.java +++ b/servicetalk-http-api/src/main/java/io/servicetalk/http/api/HttpProtocolVersion.java @@ -149,6 +149,24 @@ public String name() { return httpVersion; } + /** + * Resolves and return the http version number as a String. e.g. : 1.1 or 2.0. + * @return the http version number string. + */ + public String fullVersion() { + if (major == 1) { + if (minor == 1) { + return "1.1"; + } + if (minor == 0) { + return "1.0"; + } + } else if (major == 2 && minor == 0) { + return "2.0"; + } + return major + "." + minor; + } + /** * Determine if the protocol version is {@link #major()} is {@code 1} and trailers are supported. * @param version The version to check. diff --git a/servicetalk-http-api/src/test/java/io/servicetalk/http/api/HttpProtocolVersionTest.java b/servicetalk-http-api/src/test/java/io/servicetalk/http/api/HttpProtocolVersionTest.java index 2eb0608682..963870129e 100644 --- a/servicetalk-http-api/src/test/java/io/servicetalk/http/api/HttpProtocolVersionTest.java +++ b/servicetalk-http-api/src/test/java/io/servicetalk/http/api/HttpProtocolVersionTest.java @@ -39,6 +39,7 @@ class HttpProtocolVersionTest { void testHttp11Constant() { assertEquals(1, HTTP_1_1.major()); assertEquals(1, HTTP_1_1.minor()); + assertEquals("1.1", HTTP_1_1.fullVersion()); assertEquals("HTTP/1.1", HTTP_1_1.toString()); assertWriteToBuffer("HTTP/1.1", HTTP_1_1); } @@ -47,6 +48,7 @@ void testHttp11Constant() { void testHttp10Constant() { assertEquals(1, HTTP_1_0.major()); assertEquals(0, HTTP_1_0.minor()); + assertEquals("1.0", HTTP_1_0.fullVersion()); assertEquals("HTTP/1.0", HTTP_1_0.toString()); assertWriteToBuffer("HTTP/1.0", HTTP_1_0); } @@ -63,6 +65,7 @@ void testCreateNewProtocolVersionFromMajorAndMinor() { assertEquals(9, version98.major()); assertEquals(8, version98.minor()); assertEquals("HTTP/9.8", version98.toString()); + assertEquals("9.8", version98.fullVersion()); assertWriteToBuffer("HTTP/9.8", version98); } diff --git a/servicetalk-opentelemetry-http/build.gradle b/servicetalk-opentelemetry-http/build.gradle index cc77bb4a7d..276454df21 100755 --- a/servicetalk-opentelemetry-http/build.gradle +++ b/servicetalk-opentelemetry-http/build.gradle @@ -24,7 +24,8 @@ dependencies { api project(":servicetalk-http-api") api "io.opentelemetry:opentelemetry-api:$opentelemetryVersion" - implementation("io.opentelemetry.instrumentation:opentelemetry-instrumentation-api-semconv:$opentelemetryApiVersion") + implementation("io.opentelemetry.instrumentation:opentelemetry-instrumentation-api-semconv:" + + "$opentelemetryInstrumentationVersion") implementation project(":servicetalk-annotations") implementation project(":servicetalk-http-utils") @@ -38,7 +39,7 @@ dependencies { testImplementation project(":servicetalk-test-resources") testImplementation "io.opentelemetry:opentelemetry-sdk-testing:$opentelemetryVersion" testRuntimeOnly("io.opentelemetry.instrumentation:opentelemetry-log4j-context-data-2.17-autoconfigure:" + - "$opentelemetryApiVersion") + "$opentelemetryInstrumentationVersion") testImplementation "org.junit.jupiter:junit-jupiter-api" testImplementation "org.assertj:assertj-core:$assertJCoreVersion" testImplementation "org.mockito:mockito-core:$mockitoCoreVersion" diff --git a/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/OpenTelemetryHttpRequestFilter.java b/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/OpenTelemetryHttpRequestFilter.java index 99029467d1..93396cae0d 100755 --- a/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/OpenTelemetryHttpRequestFilter.java +++ b/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/OpenTelemetryHttpRequestFilter.java @@ -76,21 +76,21 @@ public OpenTelemetryHttpRequestFilter(final OpenTelemetry openTelemetry, String OpentelemetryOptions opentelemetryOptions) { super(openTelemetry); SpanNameExtractor serverSpanNameExtractor = - HttpSpanNameExtractor.create(ServicetalkHttpClientCommonAttributesGetter.INSTANCE); + HttpSpanNameExtractor.create(ServicetalkHttpClientAttributesGetter.INSTANCE); InstrumenterBuilder clientInstrumenterBuilder = Instrumenter.builder(openTelemetry, INSTRUMENTATION_SCOPE_NAME, serverSpanNameExtractor); clientInstrumenterBuilder.setSpanStatusExtractor(ServicetalkSpanStatusExtractor.INSTANCE); clientInstrumenterBuilder .addAttributesExtractor(HttpClientAttributesExtractor - .builder(ServicetalkHttpClientCommonAttributesGetter.INSTANCE, + .builder(ServicetalkHttpClientAttributesGetter.INSTANCE, ServicetalkNetClientAttributesGetter.INSTANCE) - .setCapturedRequestHeaders(opentelemetryOptions.getCaptureRequestHeaders()) - .setCapturedResponseHeaders(opentelemetryOptions.getCaptureResponseHeaders()) + .setCapturedRequestHeaders(opentelemetryOptions.captureRequestHeaders()) + .setCapturedResponseHeaders(opentelemetryOptions.captureResponseHeaders()) .build()) .addAttributesExtractor( NetClientAttributesExtractor.create(ServicetalkNetClientAttributesGetter.INSTANCE)); - if (opentelemetryOptions.isEnableMetrics()) { + if (opentelemetryOptions.enableMetrics()) { clientInstrumenterBuilder.addOperationMetrics(HttpClientMetrics.get()); } if (!componentName.trim().isEmpty()) { diff --git a/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/OpenTelemetryHttpServerFilter.java b/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/OpenTelemetryHttpServerFilter.java index b97ccda49d..2268759899 100755 --- a/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/OpenTelemetryHttpServerFilter.java +++ b/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/OpenTelemetryHttpServerFilter.java @@ -70,21 +70,21 @@ public final class OpenTelemetryHttpServerFilter extends AbstractOpenTelemetryFi public OpenTelemetryHttpServerFilter(final OpenTelemetry openTelemetry, OpentelemetryOptions opentelemetryOptions) { super(openTelemetry); SpanNameExtractor serverSpanNameExtractor = - HttpSpanNameExtractor.create(ServicetalkHttpServerCommonAttributesGetter.INSTANCE); + HttpSpanNameExtractor.create(ServicetalkHttpServerAttributesGetter.INSTANCE); InstrumenterBuilder serverInstrumenterBuilder = Instrumenter.builder(openTelemetry, INSTRUMENTATION_SCOPE_NAME, serverSpanNameExtractor); serverInstrumenterBuilder.setSpanStatusExtractor(ServicetalkSpanStatusExtractor.INSTANCE); serverInstrumenterBuilder .addAttributesExtractor(HttpServerAttributesExtractor - .builder(ServicetalkHttpServerCommonAttributesGetter.INSTANCE, + .builder(ServicetalkHttpServerAttributesGetter.INSTANCE, ServicetalkNetServerAttributesGetter.INSTANCE) - .setCapturedRequestHeaders(opentelemetryOptions.getCaptureRequestHeaders()) - .setCapturedResponseHeaders(opentelemetryOptions.getCaptureResponseHeaders()) + .setCapturedRequestHeaders(opentelemetryOptions.captureRequestHeaders()) + .setCapturedResponseHeaders(opentelemetryOptions.captureResponseHeaders()) .build()) .addAttributesExtractor( NetServerAttributesExtractor.create(ServicetalkNetServerAttributesGetter.INSTANCE)); - if (opentelemetryOptions.isEnableMetrics()) { + if (opentelemetryOptions.enableMetrics()) { serverInstrumenterBuilder.addOperationMetrics(HttpServerMetrics.get()); } diff --git a/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/OpentelemetryOptions.java b/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/OpentelemetryOptions.java index fa57ed5520..7272d9f15b 100644 --- a/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/OpentelemetryOptions.java +++ b/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/OpentelemetryOptions.java @@ -41,42 +41,45 @@ public static Builder newBuilder() { return new Builder(); } - public List getCaptureRequestHeaders() { + public List captureRequestHeaders() { return Collections.unmodifiableList(captureRequestHeaders); } - public List getCaptureResponseHeaders() { + public List captureResponseHeaders() { return Collections.unmodifiableList(captureResponseHeaders); } - public boolean isEnableMetrics() { + public boolean enableMetrics() { return enableMetrics; } /** * a Builder of {@link OpentelemetryOptions}. */ - public static class Builder { - private List captureRequestHeaders = new ArrayList<>(); - private List captureResponseHeaders = new ArrayList<>(); + public static final class Builder { + private final List captureRequestHeaders = new ArrayList<>(); + private final List captureResponseHeaders = new ArrayList<>(); private boolean enableMetrics; + private Builder() { + } + /** - * set the headers to be captured as extra tags. + * add the headers to be captured as extra tags. * @param captureRequestHeaders extra headers to be captured in client/server requests and added as tags. * @return an instance of itself */ - public OpentelemetryOptions.Builder captureRequestHeaders(List captureRequestHeaders) { + public OpentelemetryOptions.Builder addCaptureRequestHeaders(List captureRequestHeaders) { this.captureRequestHeaders.addAll(Objects.requireNonNull(captureRequestHeaders)); return this; } /** - * set the headers to be captured as extra tags. + * add the headers to be captured as extra tags. * @param captureResponseHeaders extra headers to be captured in client/server response and added as tags. * @return an instance of itself */ - public OpentelemetryOptions.Builder captureResponseHeaders(List captureResponseHeaders) { + public OpentelemetryOptions.Builder addCaptureResponseHeaders(List captureResponseHeaders) { this.captureResponseHeaders.addAll(Objects.requireNonNull(captureResponseHeaders)); return this; } diff --git a/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ServicetalkHttpClientCommonAttributesGetter.java b/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ServicetalkHttpClientAttributesGetter.java similarity index 92% rename from servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ServicetalkHttpClientCommonAttributesGetter.java rename to servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ServicetalkHttpClientAttributesGetter.java index b9575dd3e5..95746d305c 100644 --- a/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ServicetalkHttpClientCommonAttributesGetter.java +++ b/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ServicetalkHttpClientAttributesGetter.java @@ -26,13 +26,13 @@ import java.util.List; import javax.annotation.Nullable; -final class ServicetalkHttpClientCommonAttributesGetter +final class ServicetalkHttpClientAttributesGetter implements HttpClientAttributesGetter { - static final ServicetalkHttpClientCommonAttributesGetter INSTANCE = - new ServicetalkHttpClientCommonAttributesGetter(); + static final ServicetalkHttpClientAttributesGetter INSTANCE = + new ServicetalkHttpClientAttributesGetter(); - private ServicetalkHttpClientCommonAttributesGetter() { + private ServicetalkHttpClientAttributesGetter() { } @Override diff --git a/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ServicetalkHttpServerCommonAttributesGetter.java b/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ServicetalkHttpServerAttributesGetter.java similarity index 90% rename from servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ServicetalkHttpServerCommonAttributesGetter.java rename to servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ServicetalkHttpServerAttributesGetter.java index 66c915f197..eec44e361a 100644 --- a/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ServicetalkHttpServerCommonAttributesGetter.java +++ b/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ServicetalkHttpServerAttributesGetter.java @@ -25,13 +25,13 @@ import java.util.List; import javax.annotation.Nullable; -final class ServicetalkHttpServerCommonAttributesGetter +final class ServicetalkHttpServerAttributesGetter implements HttpServerAttributesGetter { - static final ServicetalkHttpServerCommonAttributesGetter INSTANCE = - new ServicetalkHttpServerCommonAttributesGetter(); + static final ServicetalkHttpServerAttributesGetter INSTANCE = + new ServicetalkHttpServerAttributesGetter(); - private ServicetalkHttpServerCommonAttributesGetter() { + private ServicetalkHttpServerAttributesGetter() { } @Nullable @@ -40,19 +40,16 @@ public String getUrlScheme(HttpRequestMetaData httpRequestMetaData) { return httpRequestMetaData.scheme(); } - @Nullable @Override public String getUrlPath(HttpRequestMetaData httpRequestMetaData) { return httpRequestMetaData.path(); } - @Nullable @Override public String getUrlQuery(HttpRequestMetaData httpRequestMetaData) { return httpRequestMetaData.query(); } - @Nullable @Override public String getHttpRequestMethod(HttpRequestMetaData httpRequestMetaData) { return httpRequestMetaData.method().name(); @@ -67,7 +64,6 @@ public List getHttpRequestHeader(HttpRequestMetaData httpRequestMetaData return Collections.emptyList(); } - @Nullable @Override public Integer getHttpResponseStatusCode(HttpRequestMetaData httpRequestMetaData, HttpResponseMetaData httpResponseMetaData, @@ -86,7 +82,6 @@ public List getHttpResponseHeader(HttpRequestMetaData httpRequestMetaDat return Collections.emptyList(); } - @Nullable @Override public String getHttpRoute(HttpRequestMetaData httpRequestMetaData) { return httpRequestMetaData.path(); diff --git a/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ServicetalkNetClientAttributesGetter.java b/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ServicetalkNetClientAttributesGetter.java index a26b1efd39..3016cf5f9a 100644 --- a/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ServicetalkNetClientAttributesGetter.java +++ b/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ServicetalkNetClientAttributesGetter.java @@ -16,7 +16,6 @@ package io.servicetalk.opentelemetry.http; -import io.servicetalk.http.api.HttpProtocolVersion; import io.servicetalk.http.api.HttpRequestMetaData; import io.servicetalk.http.api.HttpResponseMetaData; import io.servicetalk.transport.api.HostAndPort; @@ -52,18 +51,7 @@ public String getNetworkProtocolVersion(HttpRequestMetaData request, if (response == null) { return null; } - HttpProtocolVersion version = response.version(); - if (version.major() == 1) { - if (version.minor() == 1) { - return "1.1"; - } - if (version.minor() == 0) { - return "1.0"; - } - } else if (version.major() == 2 && version.minor() == 0) { - return "2.0"; - } - return version.major() + "." + version.minor(); + return response.version().fullVersion(); } @Override @@ -73,6 +61,7 @@ public String getServerAddress(HttpRequestMetaData request) { return effectiveHostAndPort != null ? effectiveHostAndPort.hostName() : null; } + @Nullable @Override public Integer getServerPort(HttpRequestMetaData request) { HostAndPort effectiveHostAndPort = request.effectiveHostAndPort(); diff --git a/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ServicetalkNetServerAttributesGetter.java b/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ServicetalkNetServerAttributesGetter.java index bbcb1fc58a..e903c3ff62 100644 --- a/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ServicetalkNetServerAttributesGetter.java +++ b/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ServicetalkNetServerAttributesGetter.java @@ -47,22 +47,11 @@ public String getNetworkProtocolName(HttpRequestMetaData request, @Nullable Http return "http"; } - @Nullable @Override public String getNetworkProtocolVersion(HttpRequestMetaData request, @Nullable HttpResponseMetaData response) { HttpProtocolVersion version = request.version(); - if (version.major() == 1) { - if (version.minor() == 1) { - return "1.1"; - } - if (version.minor() == 0) { - return "1.0"; - } - } else if (version.major() == 2 && version.minor() == 0) { - return "2.0"; - } - return version.major() + "." + version.minor(); + return version.fullVersion(); } @Override @@ -73,6 +62,7 @@ public String getServerAddress(HttpRequestMetaData request) { } @Override + @Nullable public Integer getServerPort(HttpRequestMetaData request) { HostAndPort effectiveHostAndPort = request.effectiveHostAndPort(); return effectiveHostAndPort != null ? effectiveHostAndPort.port() : null; diff --git a/servicetalk-opentelemetry-http/src/test/java/io/servicetalk/opentelemetry/http/OpenTelemetryHttpRequestFilterTest.java b/servicetalk-opentelemetry-http/src/test/java/io/servicetalk/opentelemetry/http/OpenTelemetryHttpRequestFilterTest.java index 40b09e5236..c5e47799af 100755 --- a/servicetalk-opentelemetry-http/src/test/java/io/servicetalk/opentelemetry/http/OpenTelemetryHttpRequestFilterTest.java +++ b/servicetalk-opentelemetry-http/src/test/java/io/servicetalk/opentelemetry/http/OpenTelemetryHttpRequestFilterTest.java @@ -213,8 +213,8 @@ void testCaptureHeader() throws Exception { try (HttpClient client = forSingleAddress(serverHostAndPort(context)) .appendClientFilter(new OpenTelemetryHttpRequestFilter(openTelemetry, "testClient", OpentelemetryOptions.newBuilder() - .captureResponseHeaders(Collections.singletonList("my-header")) - .captureRequestHeaders(Collections.singletonList("some-request-header")) + .addCaptureResponseHeaders(Collections.singletonList("my-header")) + .addCaptureRequestHeaders(Collections.singletonList("some-request-header")) .build())) .appendClientFilter(new TestTracingClientLoggerFilter(TRACING_TEST_LOG_LINE_PREFIX)).build()) { HttpResponse response = client.request(client.get(requestUrl) diff --git a/servicetalk-opentelemetry-http/src/test/java/io/servicetalk/opentelemetry/http/OpenTelemetryHttpServerFilterTest.java b/servicetalk-opentelemetry-http/src/test/java/io/servicetalk/opentelemetry/http/OpenTelemetryHttpServerFilterTest.java index 4c92279ad5..74147e5295 100644 --- a/servicetalk-opentelemetry-http/src/test/java/io/servicetalk/opentelemetry/http/OpenTelemetryHttpServerFilterTest.java +++ b/servicetalk-opentelemetry-http/src/test/java/io/servicetalk/opentelemetry/http/OpenTelemetryHttpServerFilterTest.java @@ -200,8 +200,8 @@ void testCaptureHeaders() throws Exception { final String requestUrl = "/path"; try (ServerContext context = buildServer(otelTesting.getOpenTelemetry(), OpentelemetryOptions.newBuilder() - .captureResponseHeaders(Collections.singletonList("my-header")) - .captureRequestHeaders(Collections.singletonList("some-request-header")) + .addCaptureResponseHeaders(Collections.singletonList("my-header")) + .addCaptureRequestHeaders(Collections.singletonList("some-request-header")) .build())) { try (HttpClient client = forSingleAddress(serverHostAndPort(context)).build()) { HttpResponse response = client.request(client.get(requestUrl) From f6705a360fe2fa5a59f28bd29d4cb214c63516d0 Mon Sep 17 00:00:00 2001 From: Alex Campelo Date: Mon, 14 Aug 2023 14:02:24 -0700 Subject: [PATCH 3/7] store variable for full version --- .../http/api/HttpProtocolVersion.java | 17 ++++------------- .../ServicetalkNetClientAttributesGetter.java | 10 +--------- 2 files changed, 5 insertions(+), 22 deletions(-) diff --git a/servicetalk-http-api/src/main/java/io/servicetalk/http/api/HttpProtocolVersion.java b/servicetalk-http-api/src/main/java/io/servicetalk/http/api/HttpProtocolVersion.java index 7d381d71df..893051c4b4 100644 --- a/servicetalk-http-api/src/main/java/io/servicetalk/http/api/HttpProtocolVersion.java +++ b/servicetalk-http-api/src/main/java/io/servicetalk/http/api/HttpProtocolVersion.java @@ -45,6 +45,7 @@ public final class HttpProtocolVersion implements Protocol, Comparable 9) { @@ -56,8 +57,8 @@ private HttpProtocolVersion(final int major, final int minor) { throw new IllegalArgumentException("Illegal minor version: " + minor + ", expected [0-9]"); } this.minor = minor; - - httpVersion = "HTTP/" + major + '.' + minor; + this.fullVersion = major + "." + minor; + httpVersion = "HTTP/" + fullVersion; encodedAsBuffer = PREFER_HEAP_RO_ALLOCATOR.fromAscii(httpVersion); } @@ -154,17 +155,7 @@ public String name() { * @return the http version number string. */ public String fullVersion() { - if (major == 1) { - if (minor == 1) { - return "1.1"; - } - if (minor == 0) { - return "1.0"; - } - } else if (major == 2 && minor == 0) { - return "2.0"; - } - return major + "." + minor; + return fullVersion; } /** diff --git a/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ServicetalkNetClientAttributesGetter.java b/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ServicetalkNetClientAttributesGetter.java index 3016cf5f9a..6bb47a0252 100644 --- a/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ServicetalkNetClientAttributesGetter.java +++ b/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ServicetalkNetClientAttributesGetter.java @@ -35,23 +35,15 @@ final class ServicetalkNetClientAttributesGetter private ServicetalkNetClientAttributesGetter() { } - @Nullable @Override public String getNetworkProtocolName(HttpRequestMetaData request, @Nullable HttpResponseMetaData response) { - if (response == null) { - return null; - } return "http"; } - @Nullable @Override public String getNetworkProtocolVersion(HttpRequestMetaData request, @Nullable HttpResponseMetaData response) { - if (response == null) { - return null; - } - return response.version().fullVersion(); + return request.version().fullVersion(); } @Override From 3d7d9acc55b87d57ef9aa910b231eeb2e7f63e56 Mon Sep 17 00:00:00 2001 From: Alex Campelo Date: Wed, 16 Aug 2023 12:10:10 -0700 Subject: [PATCH 4/7] update path as nonexistent according to specification --- .../gradle/spotbugs/test-exclusions.xml | 23 ++++++ .../http/CancelledRequestException.java | 25 ++++++ .../http/HeadersPropagatorGetter.java | 10 +++ .../http/OpenTelemetryHttpRequestFilter.java | 15 +++- .../http/OpenTelemetryHttpServerFilter.java | 14 +++- .../http/RequestHeadersPropagatorGetter.java | 2 +- .../http/RequestHeadersPropagatorSetter.java | 2 +- .../opentelemetry/http/ScopeTracker.java | 14 ++-- ...ServicetalkHttpClientAttributesGetter.java | 26 ++---- ...ServicetalkHttpServerAttributesGetter.java | 20 +---- .../ServicetalkNetClientAttributesGetter.java | 5 +- .../ServicetalkNetServerAttributesGetter.java | 12 +-- .../http/ServicetalkSpanStatusExtractor.java | 10 +-- .../OpenTelemetryHttpRequestFilterTest.java | 10 +-- .../OpenTelemetryHttpServerFilterTest.java | 6 +- .../ServicetalkSpanStatusExtractorTest.java | 81 +++++++++++++++++++ 16 files changed, 201 insertions(+), 74 deletions(-) create mode 100644 servicetalk-opentelemetry-http/gradle/spotbugs/test-exclusions.xml create mode 100644 servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/CancelledRequestException.java create mode 100644 servicetalk-opentelemetry-http/src/test/java/io/servicetalk/opentelemetry/http/ServicetalkSpanStatusExtractorTest.java diff --git a/servicetalk-opentelemetry-http/gradle/spotbugs/test-exclusions.xml b/servicetalk-opentelemetry-http/gradle/spotbugs/test-exclusions.xml new file mode 100644 index 0000000000..a10553e0a3 --- /dev/null +++ b/servicetalk-opentelemetry-http/gradle/spotbugs/test-exclusions.xml @@ -0,0 +1,23 @@ + + + + + + + + + diff --git a/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/CancelledRequestException.java b/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/CancelledRequestException.java new file mode 100644 index 0000000000..b28427d99d --- /dev/null +++ b/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/CancelledRequestException.java @@ -0,0 +1,25 @@ +/* + * Copyright © 2023 Apple Inc. and the ServiceTalk project authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.servicetalk.opentelemetry.http; + +class CancelledRequestException extends Exception { + static final CancelledRequestException INSTANCE = new CancelledRequestException(); + + CancelledRequestException() { + super("canceled", null, false, false); + } +} diff --git a/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/HeadersPropagatorGetter.java b/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/HeadersPropagatorGetter.java index b3f27d4940..36e13eb0bb 100755 --- a/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/HeadersPropagatorGetter.java +++ b/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/HeadersPropagatorGetter.java @@ -21,7 +21,10 @@ import io.opentelemetry.context.propagation.TextMapGetter; import java.util.Iterator; +import java.util.List; import java.util.Map; +import java.util.stream.Collectors; +import java.util.stream.StreamSupport; import javax.annotation.Nullable; final class HeadersPropagatorGetter implements TextMapGetter { @@ -67,4 +70,11 @@ public String get(@Nullable HttpHeaders carrier, final String key) { final CharSequence value = carrier.get(key); return value == null ? null : value.toString(); } + + static List getHeadersValue(String name, HttpHeaders headers) { + Iterable value = headers.values(name); + return StreamSupport.stream(value.spliterator(), false) + .map(CharSequence::toString) + .collect(Collectors.toList()); + } } diff --git a/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/OpenTelemetryHttpRequestFilter.java b/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/OpenTelemetryHttpRequestFilter.java index 93396cae0d..77d5c058cf 100755 --- a/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/OpenTelemetryHttpRequestFilter.java +++ b/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/OpenTelemetryHttpRequestFilter.java @@ -63,6 +63,7 @@ public final class OpenTelemetryHttpRequestFilter extends AbstractOpenTelemetryFilter implements StreamingHttpClientFilterFactory, StreamingHttpConnectionFilterFactory { + public static final OpentelemetryOptions DEFAULT_OPTIONS = OpentelemetryOptions.newBuilder().build(); private final Instrumenter instrumenter; /** @@ -72,7 +73,7 @@ public final class OpenTelemetryHttpRequestFilter extends AbstractOpenTelemetryF * @param componentName The component name used during building new spans. * @param opentelemetryOptions extra options to create the opentelemetry filter. */ - public OpenTelemetryHttpRequestFilter(final OpenTelemetry openTelemetry, String componentName, + OpenTelemetryHttpRequestFilter(final OpenTelemetry openTelemetry, String componentName, OpentelemetryOptions opentelemetryOptions) { super(openTelemetry); SpanNameExtractor serverSpanNameExtractor = @@ -107,7 +108,7 @@ public OpenTelemetryHttpRequestFilter(final OpenTelemetry openTelemetry, String * @param componentName The component name used during building new spans. */ public OpenTelemetryHttpRequestFilter(String componentName) { - this(GlobalOpenTelemetry.get(), componentName, OpentelemetryOptions.newBuilder().build()); + this(componentName, DEFAULT_OPTIONS); } /** @@ -115,9 +116,15 @@ public OpenTelemetryHttpRequestFilter(String componentName) { * * @param openTelemetry the {@link OpenTelemetry}. * @param componentName The component name used during building new spans. + * @deprecated this method is internal, no client should be setting the Opentelemetry as it is obtained by using + * * {@link GlobalOpenTelemetry#get()} and there should be no other implementations but the one available in + * * the classpath, this constructor will be removed. + * Use {@link #OpenTelemetryHttpRequestFilter(String)} instead. */ + @Deprecated + @SuppressWarnings("DeprecatedIsStillUsed") public OpenTelemetryHttpRequestFilter(final OpenTelemetry openTelemetry, String componentName) { - this(openTelemetry, componentName, OpentelemetryOptions.newBuilder().build()); + this(openTelemetry, componentName, DEFAULT_OPTIONS); } /** @@ -135,7 +142,7 @@ public OpenTelemetryHttpRequestFilter(String componentName, OpentelemetryOptions * using the hostname as the component name. */ public OpenTelemetryHttpRequestFilter() { - this(GlobalOpenTelemetry.get(), "", OpentelemetryOptions.newBuilder().build()); + this(""); } @Override diff --git a/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/OpenTelemetryHttpServerFilter.java b/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/OpenTelemetryHttpServerFilter.java index 2268759899..8af9cb81fb 100755 --- a/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/OpenTelemetryHttpServerFilter.java +++ b/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/OpenTelemetryHttpServerFilter.java @@ -59,6 +59,7 @@ */ public final class OpenTelemetryHttpServerFilter extends AbstractOpenTelemetryFilter implements StreamingHttpServiceFilterFactory { + public static final OpentelemetryOptions DEFAULT_OPTIONS = OpentelemetryOptions.newBuilder().build(); private final Instrumenter instrumenter; /** @@ -67,7 +68,7 @@ public final class OpenTelemetryHttpServerFilter extends AbstractOpenTelemetryFi * @param openTelemetry the {@link OpenTelemetry}. * @param opentelemetryOptions extra options to create the opentelemetry filter. */ - public OpenTelemetryHttpServerFilter(final OpenTelemetry openTelemetry, OpentelemetryOptions opentelemetryOptions) { + OpenTelemetryHttpServerFilter(final OpenTelemetry openTelemetry, OpentelemetryOptions opentelemetryOptions) { super(openTelemetry); SpanNameExtractor serverSpanNameExtractor = HttpSpanNameExtractor.create(ServicetalkHttpServerAttributesGetter.INSTANCE); @@ -96,7 +97,7 @@ public OpenTelemetryHttpServerFilter(final OpenTelemetry openTelemetry, Opentele * Create a new Instance, searching for any instance of an opentelemetry available. */ public OpenTelemetryHttpServerFilter() { - this(GlobalOpenTelemetry.get(), OpentelemetryOptions.newBuilder().build()); + this(DEFAULT_OPTIONS); } /** @@ -112,9 +113,16 @@ public OpenTelemetryHttpServerFilter(OpentelemetryOptions opentelemetryOptions) * Create a new instance. * * @param openTelemetry the {@link OpenTelemetry}. + * @deprecated this method is internal, no client should be setting the Opentelemetry as it is obtained by using + * {@link GlobalOpenTelemetry#get()} and there should be no other implementations but the one available in + * the classpath, this constructor will be removed. + * Use {@link #OpenTelemetryHttpServerFilter(OpentelemetryOptions)} or {@link #OpenTelemetryHttpServerFilter()} + * instead. */ + @Deprecated + @SuppressWarnings("DeprecatedIsStillUsed") public OpenTelemetryHttpServerFilter(final OpenTelemetry openTelemetry) { - this(openTelemetry, OpentelemetryOptions.newBuilder().build()); + this(openTelemetry, DEFAULT_OPTIONS); } @Override diff --git a/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/RequestHeadersPropagatorGetter.java b/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/RequestHeadersPropagatorGetter.java index 1ee1ed7195..4448b3ba75 100755 --- a/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/RequestHeadersPropagatorGetter.java +++ b/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/RequestHeadersPropagatorGetter.java @@ -1,5 +1,5 @@ /* - * Copyright © 2022 Apple Inc. and the ServiceTalk project authors + * Copyright © 2023 Apple Inc. and the ServiceTalk project authors * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. diff --git a/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/RequestHeadersPropagatorSetter.java b/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/RequestHeadersPropagatorSetter.java index 165de1a602..3f0a4ab9ef 100755 --- a/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/RequestHeadersPropagatorSetter.java +++ b/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/RequestHeadersPropagatorSetter.java @@ -1,5 +1,5 @@ /* - * Copyright © 2022 Apple Inc. and the ServiceTalk project authors + * Copyright © 2023 Apple Inc. and the ServiceTalk project authors * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. diff --git a/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ScopeTracker.java b/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ScopeTracker.java index 67a61c391b..447dff1822 100755 --- a/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ScopeTracker.java +++ b/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ScopeTracker.java @@ -36,17 +36,17 @@ class ScopeTracker implements TerminalSignalConsumer { private final Scope currentScope; private final Context context; - private final StreamingHttpRequest request; + private final StreamingHttpRequest requestMetaData; private final Instrumenter instrumenter; @Nullable - protected HttpResponseMetaData metaData; + private HttpResponseMetaData metaData; - ScopeTracker(Scope currentScope, Context context, StreamingHttpRequest request, + ScopeTracker(Scope currentScope, Context context, StreamingHttpRequest requestMetaData, Instrumenter instrumenter) { this.currentScope = requireNonNull(currentScope); this.context = requireNonNull(context); - this.request = requireNonNull(request); + this.requestMetaData = requireNonNull(requestMetaData); this.instrumenter = requireNonNull(instrumenter); } @@ -58,7 +58,7 @@ void onResponseMeta(final HttpResponseMetaData metaData) { public void onComplete() { assert metaData != null : "can't have succeeded without capturing metadata first"; try { - instrumenter.end(context, request, metaData, null); + instrumenter.end(context, requestMetaData, metaData, null); } finally { closeAll(); } @@ -67,7 +67,7 @@ public void onComplete() { @Override public void onError(final Throwable throwable) { try { - instrumenter.end(context, request, metaData, throwable); + instrumenter.end(context, requestMetaData, metaData, throwable); } finally { closeAll(); } @@ -76,7 +76,7 @@ public void onError(final Throwable throwable) { @Override public void cancel() { try { - instrumenter.end(context, request, metaData, null); + instrumenter.end(context, requestMetaData, metaData, CancelledRequestException.INSTANCE); } finally { closeAll(); } diff --git a/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ServicetalkHttpClientAttributesGetter.java b/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ServicetalkHttpClientAttributesGetter.java index 95746d305c..f365752dd2 100644 --- a/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ServicetalkHttpClientAttributesGetter.java +++ b/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ServicetalkHttpClientAttributesGetter.java @@ -22,7 +22,6 @@ import io.opentelemetry.instrumentation.api.instrumenter.http.HttpClientAttributesGetter; -import java.util.Collections; import java.util.List; import javax.annotation.Nullable; @@ -42,11 +41,7 @@ public String getHttpRequestMethod(HttpRequestMetaData httpRequestMetaData) { @Override public List getHttpRequestHeader(HttpRequestMetaData httpRequestMetaData, String name) { - CharSequence value = httpRequestMetaData.headers().get(name); - if (value != null) { - return Collections.singletonList(value.toString()); - } - return Collections.emptyList(); + return HeadersPropagatorGetter.getHeadersValue(name, httpRequestMetaData.headers()); } @Override @@ -60,23 +55,18 @@ public Integer getHttpResponseStatusCode(HttpRequestMetaData httpRequestMetaData public List getHttpResponseHeader(HttpRequestMetaData httpRequestMetaData, HttpResponseMetaData httpResponseMetaData, String name) { - CharSequence value = httpResponseMetaData.headers().get(name); - if (value != null) { - return Collections.singletonList(value.toString()); - } - return Collections.emptyList(); + return HeadersPropagatorGetter.getHeadersValue(name, httpResponseMetaData.headers()); } @Nullable @Override public String getUrlFull(HttpRequestMetaData request) { HostAndPort effectiveHostAndPort = request.effectiveHostAndPort(); - String requestScheme = request.scheme() != null ? request.scheme() - : "http"; - CharSequence hostAndPort = request.headers().contains("host") ? request.headers().get("host") - : effectiveHostAndPort != null ? - String.format("%s:%d", effectiveHostAndPort.hostName(), effectiveHostAndPort.port()) : null; - return hostAndPort != null ? String.format("%s://%s%s", requestScheme, hostAndPort, request.path()) : - request.path(); + if (effectiveHostAndPort == null) { + return null; + } + String requestScheme = request.scheme() == null ? "http" : request.scheme(); + String hostAndPort = effectiveHostAndPort.hostName() + ":" + effectiveHostAndPort.port(); + return requestScheme + "://" + hostAndPort + "/" + request.requestTarget(); } } diff --git a/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ServicetalkHttpServerAttributesGetter.java b/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ServicetalkHttpServerAttributesGetter.java index eec44e361a..94dd286833 100644 --- a/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ServicetalkHttpServerAttributesGetter.java +++ b/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ServicetalkHttpServerAttributesGetter.java @@ -21,7 +21,6 @@ import io.opentelemetry.instrumentation.api.instrumenter.http.HttpServerAttributesGetter; -import java.util.Collections; import java.util.List; import javax.annotation.Nullable; @@ -37,7 +36,7 @@ private ServicetalkHttpServerAttributesGetter() { @Nullable @Override public String getUrlScheme(HttpRequestMetaData httpRequestMetaData) { - return httpRequestMetaData.scheme(); + return httpRequestMetaData.scheme() == null ? "http" : httpRequestMetaData.scheme(); } @Override @@ -57,11 +56,7 @@ public String getHttpRequestMethod(HttpRequestMetaData httpRequestMetaData) { @Override public List getHttpRequestHeader(HttpRequestMetaData httpRequestMetaData, String name) { - CharSequence value = httpRequestMetaData.headers().get(name); - if (value != null) { - return Collections.singletonList(value.toString()); - } - return Collections.emptyList(); + return HeadersPropagatorGetter.getHeadersValue(name, httpRequestMetaData.headers()); } @Override @@ -75,15 +70,6 @@ public Integer getHttpResponseStatusCode(HttpRequestMetaData httpRequestMetaData public List getHttpResponseHeader(HttpRequestMetaData httpRequestMetaData, HttpResponseMetaData httpResponseMetaData, String name) { - CharSequence value = httpResponseMetaData.headers().get(name); - if (value != null) { - return Collections.singletonList(value.toString()); - } - return Collections.emptyList(); - } - - @Override - public String getHttpRoute(HttpRequestMetaData httpRequestMetaData) { - return httpRequestMetaData.path(); + return HeadersPropagatorGetter.getHeadersValue(name, httpResponseMetaData.headers()); } } diff --git a/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ServicetalkNetClientAttributesGetter.java b/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ServicetalkNetClientAttributesGetter.java index 6bb47a0252..1d274d667b 100644 --- a/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ServicetalkNetClientAttributesGetter.java +++ b/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ServicetalkNetClientAttributesGetter.java @@ -43,7 +43,10 @@ public String getNetworkProtocolName(HttpRequestMetaData request, @Nullable Http @Override public String getNetworkProtocolVersion(HttpRequestMetaData request, @Nullable HttpResponseMetaData response) { - return request.version().fullVersion(); + if (response == null) { + return request.version().fullVersion(); + } + return response.version().fullVersion(); } @Override diff --git a/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ServicetalkNetServerAttributesGetter.java b/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ServicetalkNetServerAttributesGetter.java index e903c3ff62..f80db6a4cc 100644 --- a/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ServicetalkNetServerAttributesGetter.java +++ b/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ServicetalkNetServerAttributesGetter.java @@ -16,7 +16,6 @@ package io.servicetalk.opentelemetry.http; -import io.servicetalk.http.api.HttpProtocolVersion; import io.servicetalk.http.api.HttpRequestMetaData; import io.servicetalk.http.api.HttpResponseMetaData; import io.servicetalk.transport.api.HostAndPort; @@ -37,21 +36,18 @@ final class ServicetalkNetServerAttributesGetter private ServicetalkNetServerAttributesGetter() { } - @Nullable @Override public String getNetworkProtocolName(HttpRequestMetaData request, @Nullable HttpResponseMetaData response) { - - if (response == null) { - return null; - } return "http"; } @Override public String getNetworkProtocolVersion(HttpRequestMetaData request, @Nullable HttpResponseMetaData response) { - HttpProtocolVersion version = request.version(); - return version.fullVersion(); + if (response != null) { + return response.version().fullVersion(); + } + return request.version().fullVersion(); } @Override diff --git a/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ServicetalkSpanStatusExtractor.java b/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ServicetalkSpanStatusExtractor.java index 3d1b9b2706..e63b499d0f 100644 --- a/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ServicetalkSpanStatusExtractor.java +++ b/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ServicetalkSpanStatusExtractor.java @@ -42,13 +42,13 @@ public void extract( spanStatusBuilder.setStatus(StatusCode.ERROR); } else if (status != null) { switch (status.status().statusClass()) { - case INFORMATIONAL_1XX: - case SUCCESSFUL_2XX: - case REDIRECTION_3XX: - spanStatusBuilder.setStatus(StatusCode.OK); + case CLIENT_ERROR_4XX: + case SERVER_ERROR_5XX: + spanStatusBuilder.setStatus(StatusCode.ERROR); break; default: - spanStatusBuilder.setStatus(StatusCode.ERROR); + spanStatusBuilder.setStatus(StatusCode.OK); + break; } } else { SpanStatusExtractor.getDefault().extract(spanStatusBuilder, request, null, null); diff --git a/servicetalk-opentelemetry-http/src/test/java/io/servicetalk/opentelemetry/http/OpenTelemetryHttpRequestFilterTest.java b/servicetalk-opentelemetry-http/src/test/java/io/servicetalk/opentelemetry/http/OpenTelemetryHttpRequestFilterTest.java index c5e47799af..02ff1a866e 100755 --- a/servicetalk-opentelemetry-http/src/test/java/io/servicetalk/opentelemetry/http/OpenTelemetryHttpRequestFilterTest.java +++ b/servicetalk-opentelemetry-http/src/test/java/io/servicetalk/opentelemetry/http/OpenTelemetryHttpRequestFilterTest.java @@ -99,8 +99,8 @@ void testInjectWithNoParent() throws Exception { otelTesting.assertTraces() .hasTracesSatisfyingExactly(ta -> - assertThat(ta.getSpan(0).getAttributes().get(SemanticAttributes.HTTP_URL)) - .isEqualTo("/")); + assertThat(ta.getSpan(0).getAttributes().get(SemanticAttributes.NET_PROTOCOL_NAME)) + .isEqualTo("http")); } } } @@ -131,8 +131,6 @@ void testInjectWithAParent() throws Exception { otelTesting.assertTraces() .hasTracesSatisfyingExactly(ta -> { SpanData span = ta.getSpan(0); - assertThat(span.getAttributes().get(SemanticAttributes.HTTP_URL)) - .isEqualTo("/path"); assertThat(span.getAttributes().get(SemanticAttributes.HTTP_METHOD)) .isEqualTo("GET"); assertThat(span.getAttributes().get(SemanticAttributes.HTTP_RESPONSE_CONTENT_LENGTH)) @@ -190,8 +188,8 @@ void testInjectWithAParentCreated() throws Exception { otelTesting.assertTraces() .hasTracesSatisfyingExactly(ta -> - assertThat(ta.getSpan(1).getAttributes().get(SemanticAttributes.HTTP_URL)) - .isEqualTo("/path/to/resource")); + assertThat(ta.getSpan(1).getAttributes().get(SemanticAttributes.NET_PROTOCOL_NAME)) + .isEqualTo("http")); otelTesting.assertTraces() .hasTracesSatisfyingExactly(ta -> assertThat(ta.getSpan(0).getAttributes().get(AttributeKey.stringKey("component"))) diff --git a/servicetalk-opentelemetry-http/src/test/java/io/servicetalk/opentelemetry/http/OpenTelemetryHttpServerFilterTest.java b/servicetalk-opentelemetry-http/src/test/java/io/servicetalk/opentelemetry/http/OpenTelemetryHttpServerFilterTest.java index 74147e5295..0c017cc063 100644 --- a/servicetalk-opentelemetry-http/src/test/java/io/servicetalk/opentelemetry/http/OpenTelemetryHttpServerFilterTest.java +++ b/servicetalk-opentelemetry-http/src/test/java/io/servicetalk/opentelemetry/http/OpenTelemetryHttpServerFilterTest.java @@ -102,7 +102,7 @@ void testInjectWithNoParent() throws Exception { .isEqualTo("1.1"); assertThat(span.getAttributes().get(SemanticAttributes.HTTP_METHOD)) .isEqualTo("GET"); - assertThat(span.getName()).isEqualTo("GET /path"); + assertThat(span.getName()).isEqualTo("GET"); assertThat(span.getAttributes() .get(AttributeKey.stringArrayKey("http.response.header.my_header"))) .isNull(); @@ -138,8 +138,8 @@ void testInjectWithAParent() throws Exception { otelTesting.assertTraces() .hasTracesSatisfyingExactly(ta -> { - assertThat(ta.getSpan(0).getAttributes().get(SemanticAttributes.HTTP_URL)) - .isEqualTo("/path"); + assertThat(ta.getSpan(0).getAttributes().get(SemanticAttributes.NET_PROTOCOL_NAME)) + .isEqualTo("http"); assertThat(ta.getSpan(0).getAttributes().get(SemanticAttributes.NET_PROTOCOL_VERSION)) .isEqualTo("1.1"); }); diff --git a/servicetalk-opentelemetry-http/src/test/java/io/servicetalk/opentelemetry/http/ServicetalkSpanStatusExtractorTest.java b/servicetalk-opentelemetry-http/src/test/java/io/servicetalk/opentelemetry/http/ServicetalkSpanStatusExtractorTest.java new file mode 100644 index 0000000000..654556b144 --- /dev/null +++ b/servicetalk-opentelemetry-http/src/test/java/io/servicetalk/opentelemetry/http/ServicetalkSpanStatusExtractorTest.java @@ -0,0 +1,81 @@ +/* + * Copyright © 2023 Apple Inc. and the ServiceTalk project authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.servicetalk.opentelemetry.http; + +import io.servicetalk.http.api.HttpRequestMetaData; +import io.servicetalk.http.api.HttpResponseMetaData; +import io.servicetalk.http.api.HttpResponseStatus; + +import io.opentelemetry.api.trace.StatusCode; +import io.opentelemetry.instrumentation.api.instrumenter.SpanStatusBuilder; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; + +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; +import static org.mockito.internal.verification.VerificationModeFactory.times; + +@ExtendWith(MockitoExtension.class) +class ServicetalkSpanStatusExtractorTest { + + @Mock + private SpanStatusBuilder spanStatusBuilder; + + @Mock + HttpRequestMetaData requestMetaData; + + @Mock + HttpResponseMetaData responseMetaData; + + @Test + void testStatus200To399() { + int executions = 0; + for (int code = 100; code < 400; code++) { + executions++; + when(responseMetaData.status()).thenReturn(HttpResponseStatus.of(code, "any")); + ServicetalkSpanStatusExtractor.INSTANCE.extract(spanStatusBuilder, requestMetaData, responseMetaData, null); + } + verify(spanStatusBuilder, times(executions)).setStatus(StatusCode.OK); + } + + @Test + void testStatus400to599() { + int executions = 0; + for (int code = 400; code < 600; code++) { + executions++; + when(responseMetaData.status()).thenReturn(HttpResponseStatus.of(code, "any")); + ServicetalkSpanStatusExtractor.INSTANCE.extract(spanStatusBuilder, requestMetaData, responseMetaData, null); + } + verify(spanStatusBuilder, times(executions)).setStatus(StatusCode.ERROR); + } + + @Test + void testStatusUnknown() { + when(responseMetaData.status()).thenReturn(HttpResponseStatus.of(600, "any")); + ServicetalkSpanStatusExtractor.INSTANCE.extract(spanStatusBuilder, requestMetaData, responseMetaData, null); + verify(spanStatusBuilder).setStatus(StatusCode.OK); + } + + @Test + void testExceptionError() { + ServicetalkSpanStatusExtractor.INSTANCE.extract(spanStatusBuilder, requestMetaData, responseMetaData, + new RuntimeException()); + verify(spanStatusBuilder).setStatus(StatusCode.ERROR); + } +} From 7bd940097ebe61576cbdefbff317c6f044c38953 Mon Sep 17 00:00:00 2001 From: Idel Pivnitskiy Date: Thu, 17 Aug 2023 13:57:37 -0700 Subject: [PATCH 5/7] Patch for the final review feedback --- .../http/AbstractOpenTelemetryFilter.java | 3 +- .../http/CancelledRequestException.java | 25 --- .../http/HeadersPropagatorGetter.java | 32 ++- .../http/OpenTelemetryHttpRequestFilter.java | 103 +++++----- .../http/OpenTelemetryHttpServerFilter.java | 79 ++++---- .../http/OpenTelemetryOptions.java | 182 ++++++++++++++++++ .../http/OpentelemetryOptions.java | 101 ---------- .../http/RequestHeadersPropagatorSetter.java | 6 +- .../opentelemetry/http/ScopeTracker.java | 25 ++- ...ServicetalkHttpClientAttributesGetter.java | 10 +- ...ServicetalkHttpServerAttributesGetter.java | 6 +- .../OpenTelemetryHttpRequestFilterTest.java | 16 +- .../OpenTelemetryHttpServerFilterTest.java | 22 +-- 13 files changed, 344 insertions(+), 266 deletions(-) delete mode 100644 servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/CancelledRequestException.java create mode 100644 servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/OpenTelemetryOptions.java delete mode 100644 servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/OpentelemetryOptions.java diff --git a/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/AbstractOpenTelemetryFilter.java b/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/AbstractOpenTelemetryFilter.java index 323f9013ed..11f58cc700 100644 --- a/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/AbstractOpenTelemetryFilter.java +++ b/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/AbstractOpenTelemetryFilter.java @@ -1,5 +1,5 @@ /* - * Copyright © 2022 Apple Inc. and the ServiceTalk project authors + * Copyright © 2022-2023 Apple Inc. and the ServiceTalk project authors * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -25,6 +25,7 @@ import io.opentelemetry.context.propagation.ContextPropagators; abstract class AbstractOpenTelemetryFilter implements HttpExecutionStrategyInfluencer { + static final OpenTelemetryOptions DEFAULT_OPTIONS = new OpenTelemetryOptions.Builder().build(); static final String INSTRUMENTATION_SCOPE_NAME = "io.servicetalk"; final Tracer tracer; final ContextPropagators propagators; diff --git a/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/CancelledRequestException.java b/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/CancelledRequestException.java deleted file mode 100644 index b28427d99d..0000000000 --- a/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/CancelledRequestException.java +++ /dev/null @@ -1,25 +0,0 @@ -/* - * Copyright © 2023 Apple Inc. and the ServiceTalk project authors - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package io.servicetalk.opentelemetry.http; - -class CancelledRequestException extends Exception { - static final CancelledRequestException INSTANCE = new CancelledRequestException(); - - CancelledRequestException() { - super("canceled", null, false, false); - } -} diff --git a/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/HeadersPropagatorGetter.java b/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/HeadersPropagatorGetter.java index 36e13eb0bb..c14109efcc 100755 --- a/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/HeadersPropagatorGetter.java +++ b/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/HeadersPropagatorGetter.java @@ -1,5 +1,5 @@ /* - * Copyright © 2022 Apple Inc. and the ServiceTalk project authors + * Copyright © 2022-2023 Apple Inc. and the ServiceTalk project authors * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -20,13 +20,16 @@ import io.opentelemetry.context.propagation.TextMapGetter; +import java.util.ArrayList; import java.util.Iterator; import java.util.List; import java.util.Map; -import java.util.stream.Collectors; -import java.util.stream.StreamSupport; import javax.annotation.Nullable; +import static java.util.Collections.emptyList; +import static java.util.Collections.singletonList; +import static java.util.Collections.unmodifiableList; + final class HeadersPropagatorGetter implements TextMapGetter { static final TextMapGetter INSTANCE = new HeadersPropagatorGetter(); @@ -63,7 +66,7 @@ public void remove() { @Override @Nullable - public String get(@Nullable HttpHeaders carrier, final String key) { + public String get(@Nullable final HttpHeaders carrier, final String key) { if (carrier == null) { return null; } @@ -71,10 +74,21 @@ public String get(@Nullable HttpHeaders carrier, final String key) { return value == null ? null : value.toString(); } - static List getHeadersValue(String name, HttpHeaders headers) { - Iterable value = headers.values(name); - return StreamSupport.stream(value.spliterator(), false) - .map(CharSequence::toString) - .collect(Collectors.toList()); + static List getHeaderValues(final HttpHeaders headers, final String name) { + final Iterator iterator = headers.valuesIterator(name); + if (!iterator.hasNext()) { + return emptyList(); + } + final CharSequence firstValue = iterator.next(); + if (!iterator.hasNext()) { + return singletonList(firstValue.toString()); + } + final List result = new ArrayList<>(2); + result.add(firstValue.toString()); + result.add(iterator.next().toString()); + while (iterator.hasNext()) { + result.add(iterator.next().toString()); + } + return unmodifiableList(result); } } diff --git a/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/OpenTelemetryHttpRequestFilter.java b/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/OpenTelemetryHttpRequestFilter.java index 77d5c058cf..aae7919241 100755 --- a/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/OpenTelemetryHttpRequestFilter.java +++ b/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/OpenTelemetryHttpRequestFilter.java @@ -1,5 +1,5 @@ /* - * Copyright © 2022 Apple Inc. and the ServiceTalk project authors + * Copyright © 2022-2023 Apple Inc. and the ServiceTalk project authors * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -63,68 +63,32 @@ public final class OpenTelemetryHttpRequestFilter extends AbstractOpenTelemetryFilter implements StreamingHttpClientFilterFactory, StreamingHttpConnectionFilterFactory { - public static final OpentelemetryOptions DEFAULT_OPTIONS = OpentelemetryOptions.newBuilder().build(); private final Instrumenter instrumenter; /** * Create a new instance. * - * @param openTelemetry the {@link OpenTelemetry}. - * @param componentName The component name used during building new spans. - * @param opentelemetryOptions extra options to create the opentelemetry filter. - */ - OpenTelemetryHttpRequestFilter(final OpenTelemetry openTelemetry, String componentName, - OpentelemetryOptions opentelemetryOptions) { - super(openTelemetry); - SpanNameExtractor serverSpanNameExtractor = - HttpSpanNameExtractor.create(ServicetalkHttpClientAttributesGetter.INSTANCE); - InstrumenterBuilder clientInstrumenterBuilder = - Instrumenter.builder(openTelemetry, INSTRUMENTATION_SCOPE_NAME, serverSpanNameExtractor); - clientInstrumenterBuilder.setSpanStatusExtractor(ServicetalkSpanStatusExtractor.INSTANCE); - - clientInstrumenterBuilder - .addAttributesExtractor(HttpClientAttributesExtractor - .builder(ServicetalkHttpClientAttributesGetter.INSTANCE, - ServicetalkNetClientAttributesGetter.INSTANCE) - .setCapturedRequestHeaders(opentelemetryOptions.captureRequestHeaders()) - .setCapturedResponseHeaders(opentelemetryOptions.captureResponseHeaders()) - .build()) - .addAttributesExtractor( - NetClientAttributesExtractor.create(ServicetalkNetClientAttributesGetter.INSTANCE)); - if (opentelemetryOptions.enableMetrics()) { - clientInstrumenterBuilder.addOperationMetrics(HttpClientMetrics.get()); - } - if (!componentName.trim().isEmpty()) { - clientInstrumenterBuilder.addAttributesExtractor( - AttributesExtractor.constant(SemanticAttributes.PEER_SERVICE, componentName)); - } - instrumenter = - clientInstrumenterBuilder.buildClientInstrumenter(RequestHeadersPropagatorSetter.INSTANCE); - } - - /** - * Create a new instance, searching for any instance of an opentelemetry available. - * + * @param openTelemetry the {@link OpenTelemetry}. * @param componentName The component name used during building new spans. + * @deprecated this method is internal, no user should be setting the {@link OpenTelemetry} as it is obtained by + * using {@link GlobalOpenTelemetry#get()} and there should be no other implementations but the one available in + * the classpath, this constructor will be removed in the future releases. + * Use {@link #OpenTelemetryHttpRequestFilter(String, OpenTelemetryOptions)} or + * {@link #OpenTelemetryHttpRequestFilter()} instead. */ - public OpenTelemetryHttpRequestFilter(String componentName) { - this(componentName, DEFAULT_OPTIONS); + @Deprecated // FIXME: 0.43 - remove deprecated ctor + @SuppressWarnings("DeprecatedIsStillUsed") + public OpenTelemetryHttpRequestFilter(final OpenTelemetry openTelemetry, String componentName) { + this(openTelemetry, componentName, DEFAULT_OPTIONS); } /** * Create a new instance, searching for any instance of an opentelemetry available. * - * @param openTelemetry the {@link OpenTelemetry}. * @param componentName The component name used during building new spans. - * @deprecated this method is internal, no client should be setting the Opentelemetry as it is obtained by using - * * {@link GlobalOpenTelemetry#get()} and there should be no other implementations but the one available in - * * the classpath, this constructor will be removed. - * Use {@link #OpenTelemetryHttpRequestFilter(String)} instead. */ - @Deprecated - @SuppressWarnings("DeprecatedIsStillUsed") - public OpenTelemetryHttpRequestFilter(final OpenTelemetry openTelemetry, String componentName) { - this(openTelemetry, componentName, DEFAULT_OPTIONS); + public OpenTelemetryHttpRequestFilter(final String componentName) { + this(componentName, DEFAULT_OPTIONS); } /** @@ -133,7 +97,7 @@ public OpenTelemetryHttpRequestFilter(final OpenTelemetry openTelemetry, String * @param componentName The component name used during building new spans. * @param opentelemetryOptions extra options to create the opentelemetry filter */ - public OpenTelemetryHttpRequestFilter(String componentName, OpentelemetryOptions opentelemetryOptions) { + public OpenTelemetryHttpRequestFilter(final String componentName, final OpenTelemetryOptions opentelemetryOptions) { this(GlobalOpenTelemetry.get(), componentName, opentelemetryOptions); } @@ -145,6 +109,43 @@ public OpenTelemetryHttpRequestFilter() { this(""); } + /** + * Create a new instance. + * + * @param openTelemetry the {@link OpenTelemetry}. + * @param componentName The component name used during building new spans. + * @param opentelemetryOptions extra options to create the opentelemetry filter. + */ + OpenTelemetryHttpRequestFilter(final OpenTelemetry openTelemetry, String componentName, + final OpenTelemetryOptions opentelemetryOptions) { + super(openTelemetry); + SpanNameExtractor serverSpanNameExtractor = + HttpSpanNameExtractor.create(ServicetalkHttpClientAttributesGetter.INSTANCE); + InstrumenterBuilder clientInstrumenterBuilder = + Instrumenter.builder(openTelemetry, INSTRUMENTATION_SCOPE_NAME, serverSpanNameExtractor); + clientInstrumenterBuilder.setSpanStatusExtractor(ServicetalkSpanStatusExtractor.INSTANCE); + + clientInstrumenterBuilder + .addAttributesExtractor(HttpClientAttributesExtractor + .builder(ServicetalkHttpClientAttributesGetter.INSTANCE, + ServicetalkNetClientAttributesGetter.INSTANCE) + .setCapturedRequestHeaders(opentelemetryOptions.capturedRequestHeaders()) + .setCapturedResponseHeaders(opentelemetryOptions.capturedResponseHeaders()) + .build()) + .addAttributesExtractor( + NetClientAttributesExtractor.create(ServicetalkNetClientAttributesGetter.INSTANCE)); + if (opentelemetryOptions.enableMetrics()) { + clientInstrumenterBuilder.addOperationMetrics(HttpClientMetrics.get()); + } + componentName = componentName.trim(); + if (!componentName.isEmpty()) { + clientInstrumenterBuilder.addAttributesExtractor( + AttributesExtractor.constant(SemanticAttributes.PEER_SERVICE, componentName)); + } + instrumenter = + clientInstrumenterBuilder.buildClientInstrumenter(RequestHeadersPropagatorSetter.INSTANCE); + } + @Override public StreamingHttpClientFilter create(FilterableStreamingHttpClient client) { return new StreamingHttpClientFilter(client) { @@ -170,7 +171,7 @@ public Single request(final StreamingHttpRequest request) private Single trackRequest(final StreamingHttpRequester delegate, final StreamingHttpRequest request) { final Context parentContext = Context.current(); - Context context = instrumenter.start(parentContext, request); + final Context context = instrumenter.start(parentContext, request); final Scope scope = context.makeCurrent(); final ScopeTracker tracker = new ScopeTracker(scope, context, request, instrumenter); diff --git a/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/OpenTelemetryHttpServerFilter.java b/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/OpenTelemetryHttpServerFilter.java index 8af9cb81fb..cfcb15d124 100755 --- a/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/OpenTelemetryHttpServerFilter.java +++ b/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/OpenTelemetryHttpServerFilter.java @@ -1,5 +1,5 @@ /* - * Copyright © 2022 Apple Inc. and the ServiceTalk project authors + * Copyright © 2022-2023 Apple Inc. and the ServiceTalk project authors * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -59,38 +59,22 @@ */ public final class OpenTelemetryHttpServerFilter extends AbstractOpenTelemetryFilter implements StreamingHttpServiceFilterFactory { - public static final OpentelemetryOptions DEFAULT_OPTIONS = OpentelemetryOptions.newBuilder().build(); private final Instrumenter instrumenter; /** * Create a new instance. * - * @param openTelemetry the {@link OpenTelemetry}. - * @param opentelemetryOptions extra options to create the opentelemetry filter. + * @param openTelemetry the {@link OpenTelemetry}. + * @deprecated this method is internal, no user should be setting the {@link OpenTelemetry} as it is obtained by + * using {@link GlobalOpenTelemetry#get()} and there should be no other implementations but the one available in + * the classpath, this constructor will be removed in the future releases. + * Use {@link #OpenTelemetryHttpServerFilter(OpenTelemetryOptions)} or {@link #OpenTelemetryHttpServerFilter()} + * instead. */ - OpenTelemetryHttpServerFilter(final OpenTelemetry openTelemetry, OpentelemetryOptions opentelemetryOptions) { - super(openTelemetry); - SpanNameExtractor serverSpanNameExtractor = - HttpSpanNameExtractor.create(ServicetalkHttpServerAttributesGetter.INSTANCE); - InstrumenterBuilder serverInstrumenterBuilder = - Instrumenter.builder(openTelemetry, INSTRUMENTATION_SCOPE_NAME, serverSpanNameExtractor); - serverInstrumenterBuilder.setSpanStatusExtractor(ServicetalkSpanStatusExtractor.INSTANCE); - - serverInstrumenterBuilder - .addAttributesExtractor(HttpServerAttributesExtractor - .builder(ServicetalkHttpServerAttributesGetter.INSTANCE, - ServicetalkNetServerAttributesGetter.INSTANCE) - .setCapturedRequestHeaders(opentelemetryOptions.captureRequestHeaders()) - .setCapturedResponseHeaders(opentelemetryOptions.captureResponseHeaders()) - .build()) - .addAttributesExtractor( - NetServerAttributesExtractor.create(ServicetalkNetServerAttributesGetter.INSTANCE)); - if (opentelemetryOptions.enableMetrics()) { - serverInstrumenterBuilder.addOperationMetrics(HttpServerMetrics.get()); - } - - instrumenter = - serverInstrumenterBuilder.buildServerInstrumenter(RequestHeadersPropagatorGetter.INSTANCE); + @Deprecated // FIXME: 0.43 - remove deprecated ctor + @SuppressWarnings("DeprecatedIsStillUsed") + public OpenTelemetryHttpServerFilter(final OpenTelemetry openTelemetry) { + this(openTelemetry, DEFAULT_OPTIONS); } /** @@ -105,24 +89,33 @@ public OpenTelemetryHttpServerFilter() { * * @param opentelemetryOptions extra options to create the opentelemetry filter */ - public OpenTelemetryHttpServerFilter(OpentelemetryOptions opentelemetryOptions) { + public OpenTelemetryHttpServerFilter(final OpenTelemetryOptions opentelemetryOptions) { this(GlobalOpenTelemetry.get(), opentelemetryOptions); } - /** - * Create a new instance. - * - * @param openTelemetry the {@link OpenTelemetry}. - * @deprecated this method is internal, no client should be setting the Opentelemetry as it is obtained by using - * {@link GlobalOpenTelemetry#get()} and there should be no other implementations but the one available in - * the classpath, this constructor will be removed. - * Use {@link #OpenTelemetryHttpServerFilter(OpentelemetryOptions)} or {@link #OpenTelemetryHttpServerFilter()} - * instead. - */ - @Deprecated - @SuppressWarnings("DeprecatedIsStillUsed") - public OpenTelemetryHttpServerFilter(final OpenTelemetry openTelemetry) { - this(openTelemetry, DEFAULT_OPTIONS); + OpenTelemetryHttpServerFilter(final OpenTelemetry openTelemetry, final OpenTelemetryOptions opentelemetryOptions) { + super(openTelemetry); + SpanNameExtractor serverSpanNameExtractor = + HttpSpanNameExtractor.create(ServicetalkHttpServerAttributesGetter.INSTANCE); + InstrumenterBuilder serverInstrumenterBuilder = + Instrumenter.builder(openTelemetry, INSTRUMENTATION_SCOPE_NAME, serverSpanNameExtractor); + serverInstrumenterBuilder.setSpanStatusExtractor(ServicetalkSpanStatusExtractor.INSTANCE); + + serverInstrumenterBuilder + .addAttributesExtractor(HttpServerAttributesExtractor + .builder(ServicetalkHttpServerAttributesGetter.INSTANCE, + ServicetalkNetServerAttributesGetter.INSTANCE) + .setCapturedRequestHeaders(opentelemetryOptions.capturedRequestHeaders()) + .setCapturedResponseHeaders(opentelemetryOptions.capturedResponseHeaders()) + .build()) + .addAttributesExtractor( + NetServerAttributesExtractor.create(ServicetalkNetServerAttributesGetter.INSTANCE)); + if (opentelemetryOptions.enableMetrics()) { + serverInstrumenterBuilder.addOperationMetrics(HttpServerMetrics.get()); + } + + instrumenter = + serverInstrumenterBuilder.buildServerInstrumenter(RequestHeadersPropagatorGetter.INSTANCE); } @Override @@ -146,7 +139,7 @@ private Single trackRequest(final StreamingHttpService de if (!instrumenter.shouldStart(parentContext, request)) { return delegate.handle(ctx, request, responseFactory); } - Context context = instrumenter.start(parentContext, request); + final Context context = instrumenter.start(parentContext, request); final Scope scope = context.makeCurrent(); final ScopeTracker tracker = new ScopeTracker(scope, context, request, instrumenter); diff --git a/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/OpenTelemetryOptions.java b/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/OpenTelemetryOptions.java new file mode 100644 index 0000000000..d3170d2722 --- /dev/null +++ b/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/OpenTelemetryOptions.java @@ -0,0 +1,182 @@ +/* + * Copyright © 2023 Apple Inc. and the ServiceTalk project authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.servicetalk.opentelemetry.http; + +import io.opentelemetry.instrumentation.api.instrumenter.InstrumenterBuilder; +import io.opentelemetry.instrumentation.api.instrumenter.OperationMetrics; +import io.opentelemetry.instrumentation.api.instrumenter.http.HttpClientAttributesExtractorBuilder; +import io.opentelemetry.instrumentation.api.instrumenter.http.HttpClientMetrics; +import io.opentelemetry.instrumentation.api.instrumenter.http.HttpServerAttributesExtractorBuilder; +import io.opentelemetry.instrumentation.api.instrumenter.http.HttpServerMetrics; + +import java.util.ArrayList; +import java.util.List; + +import static java.util.Collections.emptyList; +import static java.util.Collections.unmodifiableList; +import static java.util.Objects.requireNonNull; + +/** + * A set of options for configuring OpenTelemetry filters. + */ +public final class OpenTelemetryOptions { + + private final List capturedRequestHeaders; + private final List capturedResponseHeaders; + private final boolean enableMetrics; + + OpenTelemetryOptions(final List capturedRequestHeaders, + final List capturedResponseHeaders, + final boolean enableMetrics) { + this.capturedRequestHeaders = unmodifiableList(new ArrayList<>(capturedRequestHeaders)); + this.capturedResponseHeaders = unmodifiableList(new ArrayList<>(capturedResponseHeaders)); + this.enableMetrics = enableMetrics; + } + + /** + * List of request headers to be captured as extra span attributes. + * + * @return List of request headers to be captured as extra span attributes + * @see HttpClientAttributesExtractorBuilder#setCapturedRequestHeaders(List) + * @see HttpServerAttributesExtractorBuilder#setCapturedRequestHeaders(List) + */ + public List capturedRequestHeaders() { + return capturedRequestHeaders; + } + + /** + * List of response headers to be captured as extra span attributes. + * + * @return List of response headers to be captured as extra span attributes. + * @see HttpClientAttributesExtractorBuilder#setCapturedResponseHeaders(List) + * @see HttpServerAttributesExtractorBuilder#setCapturedResponseHeaders(List) + */ + public List capturedResponseHeaders() { + return capturedResponseHeaders; + } + + /** + * Whether to enable operation metrics or not. + * + * @return {@code true} when operation metrics should be enabled, {@code false} otherwise + * @see InstrumenterBuilder#addOperationMetrics(OperationMetrics) + * @see HttpClientMetrics + * @see HttpServerMetrics + */ + public boolean enableMetrics() { + return enableMetrics; + } + + @Override + public boolean equals(final Object o) { + if (this == o) { + return true; + } + if (!(o instanceof OpenTelemetryOptions)) { + return false; + } + + final OpenTelemetryOptions that = (OpenTelemetryOptions) o; + if (enableMetrics != that.enableMetrics) { + return false; + } + if (!capturedRequestHeaders.equals(that.capturedRequestHeaders)) { + return false; + } + return capturedResponseHeaders.equals(that.capturedResponseHeaders); + } + + @Override + public int hashCode() { + int result = capturedRequestHeaders.hashCode(); + result = 31 * result + capturedResponseHeaders.hashCode(); + result = 31 * result + (enableMetrics ? 1 : 0); + return result; + } + + @Override + public String toString() { + return getClass().getSimpleName() + + "{capturedRequestHeaders=" + capturedRequestHeaders + + ", capturedResponseHeaders=" + capturedResponseHeaders + + ", enableMetrics=" + enableMetrics + + '}'; + } + + /** + * A builder for {@link OpenTelemetryOptions}. + */ + public static final class Builder { + private List capturedRequestHeaders = emptyList(); + private List capturedResponseHeaders = emptyList(); + private boolean enableMetrics; + + /** + * Add the headers to be captured as extra span attributes. + * + * @param capturedRequestHeaders extra headers to be captured in client/server requests and added as extra span + * attributes + * @return an instance of itself + * @see #capturedRequestHeaders() + * @see HttpClientAttributesExtractorBuilder#setCapturedRequestHeaders(List) + * @see HttpServerAttributesExtractorBuilder#setCapturedRequestHeaders(List) + */ + public Builder capturedRequestHeaders(final List capturedRequestHeaders) { + this.capturedRequestHeaders = requireNonNull(capturedRequestHeaders); + return this; + } + + /** + * Add the headers to be captured as extra span attributes. + * + * @param capturedResponseHeaders extra headers to be captured in client/server response and added as extra span + * attributes + * @return an instance of itself + * @see #capturedResponseHeaders() + * @see HttpClientAttributesExtractorBuilder#setCapturedResponseHeaders(List) + * @see HttpServerAttributesExtractorBuilder#setCapturedResponseHeaders(List) + */ + public Builder capturedResponseHeaders(final List capturedResponseHeaders) { + this.capturedResponseHeaders = requireNonNull(capturedResponseHeaders); + return this; + } + + /** + * Whether to enable operation metrics or not. + * + * @param enableMetrics whether to enable operation metrics or not + * @return an instance of itself + * @see #enableMetrics() + * @see InstrumenterBuilder#addOperationMetrics(OperationMetrics) + * @see HttpClientMetrics + * @see HttpServerMetrics + */ + public Builder enableMetrics(final boolean enableMetrics) { + this.enableMetrics = enableMetrics; + return this; + } + + /** + * Builds a new {@link OpenTelemetryOptions}. + * + * @return a new {@link OpenTelemetryOptions} + */ + public OpenTelemetryOptions build() { + return new OpenTelemetryOptions(capturedRequestHeaders, capturedResponseHeaders, enableMetrics); + } + } +} diff --git a/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/OpentelemetryOptions.java b/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/OpentelemetryOptions.java deleted file mode 100644 index 7272d9f15b..0000000000 --- a/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/OpentelemetryOptions.java +++ /dev/null @@ -1,101 +0,0 @@ -/* - * Copyright © 2023 Apple Inc. and the ServiceTalk project authors - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package io.servicetalk.opentelemetry.http; - -import java.util.ArrayList; -import java.util.Collections; -import java.util.List; -import java.util.Objects; - -/** - * A set of options for creating the opentelemetry filter. - */ -public final class OpentelemetryOptions { - - private final List captureRequestHeaders; - private final List captureResponseHeaders; - private final boolean enableMetrics; - - OpentelemetryOptions(List captureRequestHeaders, List captureResponseHeaders, - boolean enableMetrics) { - this.captureRequestHeaders = captureRequestHeaders; - this.captureResponseHeaders = captureResponseHeaders; - this.enableMetrics = enableMetrics; - } - - public static Builder newBuilder() { - return new Builder(); - } - - public List captureRequestHeaders() { - return Collections.unmodifiableList(captureRequestHeaders); - } - - public List captureResponseHeaders() { - return Collections.unmodifiableList(captureResponseHeaders); - } - - public boolean enableMetrics() { - return enableMetrics; - } - - /** - * a Builder of {@link OpentelemetryOptions}. - */ - public static final class Builder { - private final List captureRequestHeaders = new ArrayList<>(); - private final List captureResponseHeaders = new ArrayList<>(); - private boolean enableMetrics; - - private Builder() { - } - - /** - * add the headers to be captured as extra tags. - * @param captureRequestHeaders extra headers to be captured in client/server requests and added as tags. - * @return an instance of itself - */ - public OpentelemetryOptions.Builder addCaptureRequestHeaders(List captureRequestHeaders) { - this.captureRequestHeaders.addAll(Objects.requireNonNull(captureRequestHeaders)); - return this; - } - - /** - * add the headers to be captured as extra tags. - * @param captureResponseHeaders extra headers to be captured in client/server response and added as tags. - * @return an instance of itself - */ - public OpentelemetryOptions.Builder addCaptureResponseHeaders(List captureResponseHeaders) { - this.captureResponseHeaders.addAll(Objects.requireNonNull(captureResponseHeaders)); - return this; - } - - /** - * whether to enable span metrics. - * @param enableMetrics whether to enable opentelemetry metrics. - * @return an instance of itself - */ - public OpentelemetryOptions.Builder enableMetrics(boolean enableMetrics) { - this.enableMetrics = enableMetrics; - return this; - } - - public OpentelemetryOptions build() { - return new OpentelemetryOptions(captureRequestHeaders, captureResponseHeaders, enableMetrics); - } - } -} diff --git a/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/RequestHeadersPropagatorSetter.java b/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/RequestHeadersPropagatorSetter.java index 3f0a4ab9ef..3115bf56ca 100755 --- a/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/RequestHeadersPropagatorSetter.java +++ b/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/RequestHeadersPropagatorSetter.java @@ -30,9 +30,9 @@ private RequestHeadersPropagatorSetter() { } @Override - public void set(@Nullable final HttpRequestMetaData headers, final String key, final String value) { - if (headers != null) { - HeadersPropagatorSetter.INSTANCE.set(headers.headers(), key, value); + public void set(@Nullable final HttpRequestMetaData carrier, final String key, final String value) { + if (carrier != null) { + HeadersPropagatorSetter.INSTANCE.set(carrier.headers(), key, value); } } } diff --git a/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ScopeTracker.java b/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ScopeTracker.java index 447dff1822..eb194cb18b 100755 --- a/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ScopeTracker.java +++ b/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ScopeTracker.java @@ -1,5 +1,5 @@ /* - * Copyright © 2022 Apple Inc. and the ServiceTalk project authors + * Copyright © 2022-2023 Apple Inc. and the ServiceTalk project authors * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -36,11 +36,11 @@ class ScopeTracker implements TerminalSignalConsumer { private final Scope currentScope; private final Context context; - private final StreamingHttpRequest requestMetaData; + private final HttpRequestMetaData requestMetaData; private final Instrumenter instrumenter; @Nullable - private HttpResponseMetaData metaData; + private HttpResponseMetaData responseMetaData; ScopeTracker(Scope currentScope, Context context, StreamingHttpRequest requestMetaData, Instrumenter instrumenter) { @@ -51,14 +51,14 @@ class ScopeTracker implements TerminalSignalConsumer { } void onResponseMeta(final HttpResponseMetaData metaData) { - this.metaData = metaData; + this.responseMetaData = metaData; } @Override public void onComplete() { - assert metaData != null : "can't have succeeded without capturing metadata first"; + assert responseMetaData != null : "can't have succeeded without capturing metadata first"; try { - instrumenter.end(context, requestMetaData, metaData, null); + instrumenter.end(context, requestMetaData, responseMetaData, null); } finally { closeAll(); } @@ -67,7 +67,7 @@ public void onComplete() { @Override public void onError(final Throwable throwable) { try { - instrumenter.end(context, requestMetaData, metaData, throwable); + instrumenter.end(context, requestMetaData, responseMetaData, throwable); } finally { closeAll(); } @@ -76,7 +76,7 @@ public void onError(final Throwable throwable) { @Override public void cancel() { try { - instrumenter.end(context, requestMetaData, metaData, CancelledRequestException.INSTANCE); + instrumenter.end(context, requestMetaData, responseMetaData, CancelledRequestException.INSTANCE); } finally { closeAll(); } @@ -94,4 +94,13 @@ Single track(Single responseSingle private void closeAll() { currentScope.close(); } + + private static final class CancelledRequestException extends Exception { + private static final long serialVersionUID = 6357694797622093267L; + static final CancelledRequestException INSTANCE = new CancelledRequestException(); + + CancelledRequestException() { + super("canceled", null, false, false); + } + } } diff --git a/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ServicetalkHttpClientAttributesGetter.java b/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ServicetalkHttpClientAttributesGetter.java index f365752dd2..e369c4c273 100644 --- a/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ServicetalkHttpClientAttributesGetter.java +++ b/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ServicetalkHttpClientAttributesGetter.java @@ -25,6 +25,8 @@ import java.util.List; import javax.annotation.Nullable; +import static io.servicetalk.opentelemetry.http.HeadersPropagatorGetter.getHeaderValues; + final class ServicetalkHttpClientAttributesGetter implements HttpClientAttributesGetter { @@ -41,7 +43,7 @@ public String getHttpRequestMethod(HttpRequestMetaData httpRequestMetaData) { @Override public List getHttpRequestHeader(HttpRequestMetaData httpRequestMetaData, String name) { - return HeadersPropagatorGetter.getHeadersValue(name, httpRequestMetaData.headers()); + return getHeaderValues(httpRequestMetaData.headers(), name); } @Override @@ -55,7 +57,7 @@ public Integer getHttpResponseStatusCode(HttpRequestMetaData httpRequestMetaData public List getHttpResponseHeader(HttpRequestMetaData httpRequestMetaData, HttpResponseMetaData httpResponseMetaData, String name) { - return HeadersPropagatorGetter.getHeadersValue(name, httpResponseMetaData.headers()); + return getHeaderValues(httpResponseMetaData.headers(), name); } @Nullable @@ -66,7 +68,7 @@ public String getUrlFull(HttpRequestMetaData request) { return null; } String requestScheme = request.scheme() == null ? "http" : request.scheme(); - String hostAndPort = effectiveHostAndPort.hostName() + ":" + effectiveHostAndPort.port(); - return requestScheme + "://" + hostAndPort + "/" + request.requestTarget(); + String hostAndPort = effectiveHostAndPort.hostName() + ':' + effectiveHostAndPort.port(); + return requestScheme + "://" + hostAndPort + '/' + request.requestTarget(); } } diff --git a/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ServicetalkHttpServerAttributesGetter.java b/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ServicetalkHttpServerAttributesGetter.java index 94dd286833..60eb1c83ae 100644 --- a/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ServicetalkHttpServerAttributesGetter.java +++ b/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ServicetalkHttpServerAttributesGetter.java @@ -24,6 +24,8 @@ import java.util.List; import javax.annotation.Nullable; +import static io.servicetalk.opentelemetry.http.HeadersPropagatorGetter.getHeaderValues; + final class ServicetalkHttpServerAttributesGetter implements HttpServerAttributesGetter { @@ -56,7 +58,7 @@ public String getHttpRequestMethod(HttpRequestMetaData httpRequestMetaData) { @Override public List getHttpRequestHeader(HttpRequestMetaData httpRequestMetaData, String name) { - return HeadersPropagatorGetter.getHeadersValue(name, httpRequestMetaData.headers()); + return getHeaderValues(httpRequestMetaData.headers(), name); } @Override @@ -70,6 +72,6 @@ public Integer getHttpResponseStatusCode(HttpRequestMetaData httpRequestMetaData public List getHttpResponseHeader(HttpRequestMetaData httpRequestMetaData, HttpResponseMetaData httpResponseMetaData, String name) { - return HeadersPropagatorGetter.getHeadersValue(name, httpResponseMetaData.headers()); + return getHeaderValues(httpResponseMetaData.headers(), name); } } diff --git a/servicetalk-opentelemetry-http/src/test/java/io/servicetalk/opentelemetry/http/OpenTelemetryHttpRequestFilterTest.java b/servicetalk-opentelemetry-http/src/test/java/io/servicetalk/opentelemetry/http/OpenTelemetryHttpRequestFilterTest.java index 02ff1a866e..48199f0c22 100755 --- a/servicetalk-opentelemetry-http/src/test/java/io/servicetalk/opentelemetry/http/OpenTelemetryHttpRequestFilterTest.java +++ b/servicetalk-opentelemetry-http/src/test/java/io/servicetalk/opentelemetry/http/OpenTelemetryHttpRequestFilterTest.java @@ -43,7 +43,6 @@ import org.slf4j.LoggerFactory; import java.lang.invoke.MethodHandles; -import java.util.Collections; import static io.servicetalk.concurrent.api.Single.succeeded; import static io.servicetalk.http.netty.HttpClients.forSingleAddress; @@ -54,6 +53,7 @@ import static io.servicetalk.opentelemetry.http.TestUtils.TestTracingClientLoggerFilter; import static io.servicetalk.transport.netty.internal.AddressUtils.localAddress; import static io.servicetalk.transport.netty.internal.AddressUtils.serverHostAndPort; +import static java.util.Collections.singletonList; import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -63,7 +63,7 @@ class OpenTelemetryHttpRequestFilterTest { private static final Logger LOGGER = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); @RegisterExtension - private final OpenTelemetryExtension otelTesting = OpenTelemetryExtension.create(); + static final OpenTelemetryExtension otelTesting = OpenTelemetryExtension.create(); @BeforeEach public void setup() { @@ -210,9 +210,9 @@ void testCaptureHeader() throws Exception { try (ServerContext context = buildServer(openTelemetry, false)) { try (HttpClient client = forSingleAddress(serverHostAndPort(context)) .appendClientFilter(new OpenTelemetryHttpRequestFilter(openTelemetry, "testClient", - OpentelemetryOptions.newBuilder() - .addCaptureResponseHeaders(Collections.singletonList("my-header")) - .addCaptureRequestHeaders(Collections.singletonList("some-request-header")) + new OpenTelemetryOptions.Builder() + .capturedResponseHeaders(singletonList("my-header")) + .capturedRequestHeaders(singletonList("some-request-header")) .build())) .appendClientFilter(new TestTracingClientLoggerFilter(TRACING_TEST_LOG_LINE_PREFIX)).build()) { HttpResponse response = client.request(client.get(requestUrl) @@ -235,10 +235,10 @@ void testCaptureHeader() throws Exception { SpanData span = ta.getSpan(0); assertThat(span.getAttributes() .get(AttributeKey.stringArrayKey("http.response.header.my_header"))) - .isEqualTo(Collections.singletonList("header-value")); + .isEqualTo(singletonList("header-value")); assertThat(span.getAttributes() .get(AttributeKey.stringArrayKey("http.request.header.some_request_header"))) - .isEqualTo(Collections.singletonList("request-header-value")); + .isEqualTo(singletonList("request-header-value")); }); } } @@ -254,7 +254,7 @@ private static ServerContext buildServer(OpenTelemetry givenOpentelemetry, boole .listenAndAwait((ctx, request, responseFactory) -> { final ContextPropagators propagators = givenOpentelemetry.getPropagators(); final Context context = Context.root(); - io.opentelemetry.context.Context tracingContext = propagators.getTextMapPropagator() + Context tracingContext = propagators.getTextMapPropagator() .extract(context, request.headers(), HeadersPropagatorGetter.INSTANCE); Span span = Span.fromContext(tracingContext); return succeeded( diff --git a/servicetalk-opentelemetry-http/src/test/java/io/servicetalk/opentelemetry/http/OpenTelemetryHttpServerFilterTest.java b/servicetalk-opentelemetry-http/src/test/java/io/servicetalk/opentelemetry/http/OpenTelemetryHttpServerFilterTest.java index 0c017cc063..50eeff4f0f 100644 --- a/servicetalk-opentelemetry-http/src/test/java/io/servicetalk/opentelemetry/http/OpenTelemetryHttpServerFilterTest.java +++ b/servicetalk-opentelemetry-http/src/test/java/io/servicetalk/opentelemetry/http/OpenTelemetryHttpServerFilterTest.java @@ -43,7 +43,6 @@ import java.io.IOException; import java.net.HttpURLConnection; import java.net.URL; -import java.util.Collections; import static io.servicetalk.concurrent.api.Single.succeeded; import static io.servicetalk.http.netty.HttpClients.forSingleAddress; @@ -53,12 +52,13 @@ import static io.servicetalk.opentelemetry.http.TestUtils.TRACING_TEST_LOG_LINE_PREFIX; import static io.servicetalk.transport.netty.internal.AddressUtils.localAddress; import static io.servicetalk.transport.netty.internal.AddressUtils.serverHostAndPort; +import static java.util.Collections.singletonList; import static org.assertj.core.api.Assertions.assertThat; class OpenTelemetryHttpServerFilterTest { @RegisterExtension - final OpenTelemetryExtension otelTesting = OpenTelemetryExtension.create(); + static final OpenTelemetryExtension otelTesting = OpenTelemetryExtension.create(); @BeforeEach public void setup() { @@ -165,7 +165,7 @@ void testInjectWithNewTrace() throws Exception { .setAttribute(SemanticAttributes.HTTP_URL, url.toString()); HttpURLConnection con = (HttpURLConnection) url.openConnection(); - textMapPropagator.inject(io.opentelemetry.context.Context.root().with(span), con, setter); + textMapPropagator.inject(Context.root().with(span), con, setter); con.setRequestMethod("GET"); int responseCode = con.getResponseCode(); @@ -199,9 +199,9 @@ void testInjectWithNewTrace() throws Exception { void testCaptureHeaders() throws Exception { final String requestUrl = "/path"; try (ServerContext context = buildServer(otelTesting.getOpenTelemetry(), - OpentelemetryOptions.newBuilder() - .addCaptureResponseHeaders(Collections.singletonList("my-header")) - .addCaptureRequestHeaders(Collections.singletonList("some-request-header")) + new OpenTelemetryOptions.Builder() + .capturedResponseHeaders(singletonList("my-header")) + .capturedRequestHeaders(singletonList("some-request-header")) .build())) { try (HttpClient client = forSingleAddress(serverHostAndPort(context)).build()) { HttpResponse response = client.request(client.get(requestUrl) @@ -225,24 +225,24 @@ void testCaptureHeaders() throws Exception { SpanData span = ta.getSpan(0); assertThat( span.getAttributes().get(AttributeKey.stringArrayKey("http.response.header.my_header"))) - .isEqualTo(Collections.singletonList("header-value")); + .isEqualTo(singletonList("header-value")); assertThat(span.getAttributes() .get(AttributeKey.stringArrayKey("http.request.header.some_request_header"))) - .isEqualTo(Collections.singletonList("request-header-value")); + .isEqualTo(singletonList("request-header-value")); }); } } } private static ServerContext buildServer(OpenTelemetry givenOpentelemetry, - OpentelemetryOptions opentelemetryOptions) throws Exception { + OpenTelemetryOptions opentelemetryOptions) throws Exception { return HttpServers.forAddress(localAddress(0)) .appendServiceFilter(new OpenTelemetryHttpServerFilter(givenOpentelemetry, opentelemetryOptions)) .appendServiceFilter(new TestTracingServerLoggerFilter(TRACING_TEST_LOG_LINE_PREFIX)) .listenAndAwait((ctx, request, responseFactory) -> { final ContextPropagators propagators = givenOpentelemetry.getPropagators(); final Context context = Context.root(); - io.opentelemetry.context.Context tracingContext = propagators.getTextMapPropagator() + Context tracingContext = propagators.getTextMapPropagator() .extract(context, request.headers(), HeadersPropagatorGetter.INSTANCE); Span span = Span.current(); if (!span.getSpanContext().isValid()) { @@ -256,6 +256,6 @@ private static ServerContext buildServer(OpenTelemetry givenOpentelemetry, } private static ServerContext buildServer(OpenTelemetry givenOpentelemetry) throws Exception { - return buildServer(givenOpentelemetry, OpentelemetryOptions.newBuilder().build()); + return buildServer(givenOpentelemetry, new OpenTelemetryOptions.Builder().build()); } } From dd1d703c72af75b25344fcf13665f4d66e6eed89 Mon Sep 17 00:00:00 2001 From: Idel Pivnitskiy Date: Thu, 17 Aug 2023 14:27:01 -0700 Subject: [PATCH 6/7] Merge client/server attribute getters --- .../http/HeadersPropagatorGetter.java | 24 ---- .../http/OpenTelemetryHttpRequestFilter.java | 8 +- .../http/OpenTelemetryHttpServerFilter.java | 8 +- .../http/ServiceTalkHttpAttributesGetter.java | 114 ++++++++++++++++++ .../http/ServiceTalkNetAttributesGetter.java | 72 +++++++++++ ...ServicetalkHttpClientAttributesGetter.java | 74 ------------ ...ServicetalkHttpServerAttributesGetter.java | 77 ------------ .../ServicetalkNetClientAttributesGetter.java | 65 ---------- .../ServicetalkNetServerAttributesGetter.java | 66 ---------- 9 files changed, 192 insertions(+), 316 deletions(-) create mode 100644 servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ServiceTalkHttpAttributesGetter.java create mode 100644 servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ServiceTalkNetAttributesGetter.java delete mode 100644 servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ServicetalkHttpClientAttributesGetter.java delete mode 100644 servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ServicetalkHttpServerAttributesGetter.java delete mode 100644 servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ServicetalkNetClientAttributesGetter.java delete mode 100644 servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ServicetalkNetServerAttributesGetter.java diff --git a/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/HeadersPropagatorGetter.java b/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/HeadersPropagatorGetter.java index c14109efcc..d7de6a1a6f 100755 --- a/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/HeadersPropagatorGetter.java +++ b/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/HeadersPropagatorGetter.java @@ -20,16 +20,10 @@ import io.opentelemetry.context.propagation.TextMapGetter; -import java.util.ArrayList; import java.util.Iterator; -import java.util.List; import java.util.Map; import javax.annotation.Nullable; -import static java.util.Collections.emptyList; -import static java.util.Collections.singletonList; -import static java.util.Collections.unmodifiableList; - final class HeadersPropagatorGetter implements TextMapGetter { static final TextMapGetter INSTANCE = new HeadersPropagatorGetter(); @@ -73,22 +67,4 @@ public String get(@Nullable final HttpHeaders carrier, final String key) { final CharSequence value = carrier.get(key); return value == null ? null : value.toString(); } - - static List getHeaderValues(final HttpHeaders headers, final String name) { - final Iterator iterator = headers.valuesIterator(name); - if (!iterator.hasNext()) { - return emptyList(); - } - final CharSequence firstValue = iterator.next(); - if (!iterator.hasNext()) { - return singletonList(firstValue.toString()); - } - final List result = new ArrayList<>(2); - result.add(firstValue.toString()); - result.add(iterator.next().toString()); - while (iterator.hasNext()) { - result.add(iterator.next().toString()); - } - return unmodifiableList(result); - } } diff --git a/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/OpenTelemetryHttpRequestFilter.java b/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/OpenTelemetryHttpRequestFilter.java index aae7919241..91ac89cd8f 100755 --- a/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/OpenTelemetryHttpRequestFilter.java +++ b/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/OpenTelemetryHttpRequestFilter.java @@ -120,20 +120,18 @@ public OpenTelemetryHttpRequestFilter() { final OpenTelemetryOptions opentelemetryOptions) { super(openTelemetry); SpanNameExtractor serverSpanNameExtractor = - HttpSpanNameExtractor.create(ServicetalkHttpClientAttributesGetter.INSTANCE); + HttpSpanNameExtractor.create(ServiceTalkHttpAttributesGetter.INSTANCE); InstrumenterBuilder clientInstrumenterBuilder = Instrumenter.builder(openTelemetry, INSTRUMENTATION_SCOPE_NAME, serverSpanNameExtractor); clientInstrumenterBuilder.setSpanStatusExtractor(ServicetalkSpanStatusExtractor.INSTANCE); clientInstrumenterBuilder .addAttributesExtractor(HttpClientAttributesExtractor - .builder(ServicetalkHttpClientAttributesGetter.INSTANCE, - ServicetalkNetClientAttributesGetter.INSTANCE) + .builder(ServiceTalkHttpAttributesGetter.INSTANCE, ServiceTalkNetAttributesGetter.INSTANCE) .setCapturedRequestHeaders(opentelemetryOptions.capturedRequestHeaders()) .setCapturedResponseHeaders(opentelemetryOptions.capturedResponseHeaders()) .build()) - .addAttributesExtractor( - NetClientAttributesExtractor.create(ServicetalkNetClientAttributesGetter.INSTANCE)); + .addAttributesExtractor(NetClientAttributesExtractor.create(ServiceTalkNetAttributesGetter.INSTANCE)); if (opentelemetryOptions.enableMetrics()) { clientInstrumenterBuilder.addOperationMetrics(HttpClientMetrics.get()); } diff --git a/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/OpenTelemetryHttpServerFilter.java b/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/OpenTelemetryHttpServerFilter.java index cfcb15d124..bbf1da4be2 100755 --- a/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/OpenTelemetryHttpServerFilter.java +++ b/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/OpenTelemetryHttpServerFilter.java @@ -96,20 +96,18 @@ public OpenTelemetryHttpServerFilter(final OpenTelemetryOptions opentelemetryOpt OpenTelemetryHttpServerFilter(final OpenTelemetry openTelemetry, final OpenTelemetryOptions opentelemetryOptions) { super(openTelemetry); SpanNameExtractor serverSpanNameExtractor = - HttpSpanNameExtractor.create(ServicetalkHttpServerAttributesGetter.INSTANCE); + HttpSpanNameExtractor.create(ServiceTalkHttpAttributesGetter.INSTANCE); InstrumenterBuilder serverInstrumenterBuilder = Instrumenter.builder(openTelemetry, INSTRUMENTATION_SCOPE_NAME, serverSpanNameExtractor); serverInstrumenterBuilder.setSpanStatusExtractor(ServicetalkSpanStatusExtractor.INSTANCE); serverInstrumenterBuilder .addAttributesExtractor(HttpServerAttributesExtractor - .builder(ServicetalkHttpServerAttributesGetter.INSTANCE, - ServicetalkNetServerAttributesGetter.INSTANCE) + .builder(ServiceTalkHttpAttributesGetter.INSTANCE, ServiceTalkNetAttributesGetter.INSTANCE) .setCapturedRequestHeaders(opentelemetryOptions.capturedRequestHeaders()) .setCapturedResponseHeaders(opentelemetryOptions.capturedResponseHeaders()) .build()) - .addAttributesExtractor( - NetServerAttributesExtractor.create(ServicetalkNetServerAttributesGetter.INSTANCE)); + .addAttributesExtractor(NetServerAttributesExtractor.create(ServiceTalkNetAttributesGetter.INSTANCE)); if (opentelemetryOptions.enableMetrics()) { serverInstrumenterBuilder.addOperationMetrics(HttpServerMetrics.get()); } diff --git a/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ServiceTalkHttpAttributesGetter.java b/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ServiceTalkHttpAttributesGetter.java new file mode 100644 index 0000000000..26548bda76 --- /dev/null +++ b/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ServiceTalkHttpAttributesGetter.java @@ -0,0 +1,114 @@ +/* + * Copyright © 2023 Apple Inc. and the ServiceTalk project authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.servicetalk.opentelemetry.http; + +import io.servicetalk.http.api.HttpHeaders; +import io.servicetalk.http.api.HttpRequestMetaData; +import io.servicetalk.http.api.HttpResponseMetaData; +import io.servicetalk.transport.api.HostAndPort; + +import io.opentelemetry.instrumentation.api.instrumenter.http.HttpClientAttributesGetter; +import io.opentelemetry.instrumentation.api.instrumenter.http.HttpServerAttributesGetter; + +import java.util.ArrayList; +import java.util.Iterator; +import java.util.List; +import javax.annotation.Nullable; + +import static java.util.Collections.emptyList; +import static java.util.Collections.singletonList; +import static java.util.Collections.unmodifiableList; + +final class ServiceTalkHttpAttributesGetter implements + HttpClientAttributesGetter, + HttpServerAttributesGetter { + + static final ServiceTalkHttpAttributesGetter INSTANCE = new ServiceTalkHttpAttributesGetter(); + + private ServiceTalkHttpAttributesGetter() { + } + + @Override + public String getHttpRequestMethod(final HttpRequestMetaData httpRequestMetaData) { + return httpRequestMetaData.method().name(); + } + + @Override + public List getHttpRequestHeader(final HttpRequestMetaData httpRequestMetaData, final String name) { + return getHeaderValues(httpRequestMetaData.headers(), name); + } + + @Override + public Integer getHttpResponseStatusCode(final HttpRequestMetaData httpRequestMetaData, + final HttpResponseMetaData httpResponseMetaData, + @Nullable final Throwable error) { + return httpResponseMetaData.status().code(); + } + + @Override + public List getHttpResponseHeader(final HttpRequestMetaData httpRequestMetaData, + final HttpResponseMetaData httpResponseMetaData, + final String name) { + return getHeaderValues(httpResponseMetaData.headers(), name); + } + + @Nullable + @Override + public String getUrlFull(final HttpRequestMetaData request) { + HostAndPort effectiveHostAndPort = request.effectiveHostAndPort(); + if (effectiveHostAndPort == null) { + return null; + } + String requestScheme = request.scheme() == null ? "http" : request.scheme(); + String hostAndPort = effectiveHostAndPort.hostName() + ':' + effectiveHostAndPort.port(); + return requestScheme + "://" + hostAndPort + '/' + request.requestTarget(); + } + + @Override + public String getUrlScheme(final HttpRequestMetaData httpRequestMetaData) { + final String scheme = httpRequestMetaData.scheme(); + return scheme == null ? "http" : scheme; + } + + @Override + public String getUrlPath(final HttpRequestMetaData httpRequestMetaData) { + return httpRequestMetaData.path(); + } + + @Override + public String getUrlQuery(final HttpRequestMetaData httpRequestMetaData) { + return httpRequestMetaData.query(); + } + + private static List getHeaderValues(final HttpHeaders headers, final String name) { + final Iterator iterator = headers.valuesIterator(name); + if (!iterator.hasNext()) { + return emptyList(); + } + final CharSequence firstValue = iterator.next(); + if (!iterator.hasNext()) { + return singletonList(firstValue.toString()); + } + final List result = new ArrayList<>(2); + result.add(firstValue.toString()); + result.add(iterator.next().toString()); + while (iterator.hasNext()) { + result.add(iterator.next().toString()); + } + return unmodifiableList(result); + } +} diff --git a/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ServiceTalkNetAttributesGetter.java b/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ServiceTalkNetAttributesGetter.java new file mode 100644 index 0000000000..a6d6d13fd6 --- /dev/null +++ b/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ServiceTalkNetAttributesGetter.java @@ -0,0 +1,72 @@ +/* + * Copyright © 2023 Apple Inc. and the ServiceTalk project authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.servicetalk.opentelemetry.http; + +import io.servicetalk.http.api.HttpRequestMetaData; +import io.servicetalk.http.api.HttpResponseMetaData; +import io.servicetalk.transport.api.HostAndPort; + +import io.opentelemetry.instrumentation.api.instrumenter.net.NetClientAttributesGetter; +import io.opentelemetry.instrumentation.api.instrumenter.net.NetServerAttributesGetter; + +import javax.annotation.Nullable; + +final class ServiceTalkNetAttributesGetter implements + NetClientAttributesGetter, + NetServerAttributesGetter { + + static final ServiceTalkNetAttributesGetter INSTANCE = new ServiceTalkNetAttributesGetter(); + + private ServiceTalkNetAttributesGetter() { + } + + @Override + public String getNetworkProtocolName(final HttpRequestMetaData request, + @Nullable final HttpResponseMetaData response) { + return "http"; + } + + @Override + public String getNetworkProtocolVersion(final HttpRequestMetaData request, + @Nullable final HttpResponseMetaData response) { + if (response == null) { + return request.version().fullVersion(); + } + return response.version().fullVersion(); + } + + @Override + @Nullable + public String getServerAddress(final HttpRequestMetaData request) { + final HostAndPort effectiveHostAndPort = request.effectiveHostAndPort(); + return effectiveHostAndPort != null ? effectiveHostAndPort.hostName() : null; + } + + @Nullable + @Override + public Integer getServerPort(final HttpRequestMetaData request) { + final HostAndPort effectiveHostAndPort = request.effectiveHostAndPort(); + return effectiveHostAndPort != null ? effectiveHostAndPort.port() : null; + } + + @Nullable + @Override + public String getNetworkType(final HttpRequestMetaData requestMetaData, + @Nullable final HttpResponseMetaData metaData) { + return NetServerAttributesGetter.super.getNetworkType(requestMetaData, metaData); + } +} diff --git a/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ServicetalkHttpClientAttributesGetter.java b/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ServicetalkHttpClientAttributesGetter.java deleted file mode 100644 index e369c4c273..0000000000 --- a/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ServicetalkHttpClientAttributesGetter.java +++ /dev/null @@ -1,74 +0,0 @@ -/* - * Copyright © 2023 Apple Inc. and the ServiceTalk project authors - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package io.servicetalk.opentelemetry.http; - -import io.servicetalk.http.api.HttpRequestMetaData; -import io.servicetalk.http.api.HttpResponseMetaData; -import io.servicetalk.transport.api.HostAndPort; - -import io.opentelemetry.instrumentation.api.instrumenter.http.HttpClientAttributesGetter; - -import java.util.List; -import javax.annotation.Nullable; - -import static io.servicetalk.opentelemetry.http.HeadersPropagatorGetter.getHeaderValues; - -final class ServicetalkHttpClientAttributesGetter - implements HttpClientAttributesGetter { - - static final ServicetalkHttpClientAttributesGetter INSTANCE = - new ServicetalkHttpClientAttributesGetter(); - - private ServicetalkHttpClientAttributesGetter() { - } - - @Override - public String getHttpRequestMethod(HttpRequestMetaData httpRequestMetaData) { - return httpRequestMetaData.method().name(); - } - - @Override - public List getHttpRequestHeader(HttpRequestMetaData httpRequestMetaData, String name) { - return getHeaderValues(httpRequestMetaData.headers(), name); - } - - @Override - public Integer getHttpResponseStatusCode(HttpRequestMetaData httpRequestMetaData, - HttpResponseMetaData httpResponseMetaData, - @Nullable Throwable error) { - return httpResponseMetaData.status().code(); - } - - @Override - public List getHttpResponseHeader(HttpRequestMetaData httpRequestMetaData, - HttpResponseMetaData httpResponseMetaData, - String name) { - return getHeaderValues(httpResponseMetaData.headers(), name); - } - - @Nullable - @Override - public String getUrlFull(HttpRequestMetaData request) { - HostAndPort effectiveHostAndPort = request.effectiveHostAndPort(); - if (effectiveHostAndPort == null) { - return null; - } - String requestScheme = request.scheme() == null ? "http" : request.scheme(); - String hostAndPort = effectiveHostAndPort.hostName() + ':' + effectiveHostAndPort.port(); - return requestScheme + "://" + hostAndPort + '/' + request.requestTarget(); - } -} diff --git a/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ServicetalkHttpServerAttributesGetter.java b/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ServicetalkHttpServerAttributesGetter.java deleted file mode 100644 index 60eb1c83ae..0000000000 --- a/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ServicetalkHttpServerAttributesGetter.java +++ /dev/null @@ -1,77 +0,0 @@ -/* - * Copyright © 2023 Apple Inc. and the ServiceTalk project authors - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package io.servicetalk.opentelemetry.http; - -import io.servicetalk.http.api.HttpRequestMetaData; -import io.servicetalk.http.api.HttpResponseMetaData; - -import io.opentelemetry.instrumentation.api.instrumenter.http.HttpServerAttributesGetter; - -import java.util.List; -import javax.annotation.Nullable; - -import static io.servicetalk.opentelemetry.http.HeadersPropagatorGetter.getHeaderValues; - -final class ServicetalkHttpServerAttributesGetter - implements HttpServerAttributesGetter { - - static final ServicetalkHttpServerAttributesGetter INSTANCE = - new ServicetalkHttpServerAttributesGetter(); - - private ServicetalkHttpServerAttributesGetter() { - } - - @Nullable - @Override - public String getUrlScheme(HttpRequestMetaData httpRequestMetaData) { - return httpRequestMetaData.scheme() == null ? "http" : httpRequestMetaData.scheme(); - } - - @Override - public String getUrlPath(HttpRequestMetaData httpRequestMetaData) { - return httpRequestMetaData.path(); - } - - @Override - public String getUrlQuery(HttpRequestMetaData httpRequestMetaData) { - return httpRequestMetaData.query(); - } - - @Override - public String getHttpRequestMethod(HttpRequestMetaData httpRequestMetaData) { - return httpRequestMetaData.method().name(); - } - - @Override - public List getHttpRequestHeader(HttpRequestMetaData httpRequestMetaData, String name) { - return getHeaderValues(httpRequestMetaData.headers(), name); - } - - @Override - public Integer getHttpResponseStatusCode(HttpRequestMetaData httpRequestMetaData, - HttpResponseMetaData httpResponseMetaData, - @Nullable Throwable error) { - return httpResponseMetaData.status().code(); - } - - @Override - public List getHttpResponseHeader(HttpRequestMetaData httpRequestMetaData, - HttpResponseMetaData httpResponseMetaData, - String name) { - return getHeaderValues(httpResponseMetaData.headers(), name); - } -} diff --git a/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ServicetalkNetClientAttributesGetter.java b/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ServicetalkNetClientAttributesGetter.java deleted file mode 100644 index 1d274d667b..0000000000 --- a/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ServicetalkNetClientAttributesGetter.java +++ /dev/null @@ -1,65 +0,0 @@ -/* - * Copyright © 2023 Apple Inc. and the ServiceTalk project authors - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package io.servicetalk.opentelemetry.http; - -import io.servicetalk.http.api.HttpRequestMetaData; -import io.servicetalk.http.api.HttpResponseMetaData; -import io.servicetalk.transport.api.HostAndPort; - -import io.opentelemetry.instrumentation.api.instrumenter.net.NetClientAttributesGetter; - -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. - */ -final class ServicetalkNetClientAttributesGetter - implements NetClientAttributesGetter { - static final ServicetalkNetClientAttributesGetter INSTANCE = new ServicetalkNetClientAttributesGetter(); - - private ServicetalkNetClientAttributesGetter() { - } - - @Override - public String getNetworkProtocolName(HttpRequestMetaData request, @Nullable HttpResponseMetaData response) { - return "http"; - } - - @Override - public String getNetworkProtocolVersion(HttpRequestMetaData request, - @Nullable HttpResponseMetaData response) { - if (response == null) { - return request.version().fullVersion(); - } - return response.version().fullVersion(); - } - - @Override - @Nullable - public String getServerAddress(HttpRequestMetaData request) { - HostAndPort effectiveHostAndPort = request.effectiveHostAndPort(); - return effectiveHostAndPort != null ? effectiveHostAndPort.hostName() : null; - } - - @Nullable - @Override - public Integer getServerPort(HttpRequestMetaData request) { - HostAndPort effectiveHostAndPort = request.effectiveHostAndPort(); - return effectiveHostAndPort != null ? effectiveHostAndPort.port() : null; - } -} diff --git a/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ServicetalkNetServerAttributesGetter.java b/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ServicetalkNetServerAttributesGetter.java deleted file mode 100644 index f80db6a4cc..0000000000 --- a/servicetalk-opentelemetry-http/src/main/java/io/servicetalk/opentelemetry/http/ServicetalkNetServerAttributesGetter.java +++ /dev/null @@ -1,66 +0,0 @@ -/* - * Copyright © 2023 Apple Inc. and the ServiceTalk project authors - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package io.servicetalk.opentelemetry.http; - -import io.servicetalk.http.api.HttpRequestMetaData; -import io.servicetalk.http.api.HttpResponseMetaData; -import io.servicetalk.transport.api.HostAndPort; - -import io.opentelemetry.instrumentation.api.instrumenter.net.NetServerAttributesGetter; - -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. - */ -final class ServicetalkNetServerAttributesGetter - implements NetServerAttributesGetter { - - static final ServicetalkNetServerAttributesGetter INSTANCE = new ServicetalkNetServerAttributesGetter(); - - private ServicetalkNetServerAttributesGetter() { - } - - @Override - public String getNetworkProtocolName(HttpRequestMetaData request, @Nullable HttpResponseMetaData response) { - return "http"; - } - - @Override - public String getNetworkProtocolVersion(HttpRequestMetaData request, - @Nullable HttpResponseMetaData response) { - if (response != null) { - return response.version().fullVersion(); - } - return request.version().fullVersion(); - } - - @Override - @Nullable - public String getServerAddress(HttpRequestMetaData request) { - HostAndPort effectiveHostAndPort = request.effectiveHostAndPort(); - return effectiveHostAndPort != null ? effectiveHostAndPort.hostName() : null; - } - - @Override - @Nullable - public Integer getServerPort(HttpRequestMetaData request) { - HostAndPort effectiveHostAndPort = request.effectiveHostAndPort(); - return effectiveHostAndPort != null ? effectiveHostAndPort.port() : null; - } -} From 4a56f6ae3393cc32b098cbe656b866793cae5720 Mon Sep 17 00:00:00 2001 From: Idel Pivnitskiy Date: Thu, 17 Aug 2023 15:05:02 -0700 Subject: [PATCH 7/7] fix spotbugs --- .../gradle/spotbugs/main-exclusions.xml | 23 +++++++++++++++++++ .../gradle/spotbugs/test-exclusions.xml | 2 +- .../http/OpenTelemetryOptions.java | 11 +++++---- 3 files changed, 30 insertions(+), 6 deletions(-) create mode 100644 servicetalk-opentelemetry-http/gradle/spotbugs/main-exclusions.xml diff --git a/servicetalk-opentelemetry-http/gradle/spotbugs/main-exclusions.xml b/servicetalk-opentelemetry-http/gradle/spotbugs/main-exclusions.xml new file mode 100644 index 0000000000..7374177c69 --- /dev/null +++ b/servicetalk-opentelemetry-http/gradle/spotbugs/main-exclusions.xml @@ -0,0 +1,23 @@ + + + + + + + + + diff --git a/servicetalk-opentelemetry-http/gradle/spotbugs/test-exclusions.xml b/servicetalk-opentelemetry-http/gradle/spotbugs/test-exclusions.xml index a10553e0a3..b566540c05 100644 --- a/servicetalk-opentelemetry-http/gradle/spotbugs/test-exclusions.xml +++ b/servicetalk-opentelemetry-http/gradle/spotbugs/test-exclusions.xml @@ -1,6 +1,6 @@