Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

use semantic convention APIs for opentelemetry #2662

Merged
merged 7 commits into from
Aug 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion gradle.properties
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ jacksonVersion=2.14.3
openTracingVersion=0.33.0
zipkinReporterVersion=2.16.4
opentelemetryVersion=1.28.0
opentelemetryInstrumentationVersion=1.28.0-alpha

# gRPC
protobufGradlePluginVersion=0.9.4
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ public final class HttpProtocolVersion implements Protocol, Comparable<HttpProto
private final int minor;
private final String httpVersion;
private final Buffer encodedAsBuffer;
private final String fullVersion;

private HttpProtocolVersion(final int major, final int minor) {
if (major < 0 || major > 9) {
Expand All @@ -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);
}

Expand Down Expand Up @@ -149,6 +150,14 @@ 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() {
return fullVersion;
}

/**
* Determine if the protocol version is {@link #major()} is {@code 1} and trailers are supported.
* @param version The version to check.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand All @@ -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);
}
Expand All @@ -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);
}

Expand Down
5 changes: 4 additions & 1 deletion servicetalk-opentelemetry-http/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ dependencies {

api project(":servicetalk-http-api")
api "io.opentelemetry:opentelemetry-api:$opentelemetryVersion"
implementation("io.opentelemetry.instrumentation:opentelemetry-instrumentation-api-semconv:" +
"$opentelemetryInstrumentationVersion")

implementation project(":servicetalk-annotations")
implementation project(":servicetalk-http-utils")
Expand All @@ -36,7 +38,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:" +
"$opentelemetryInstrumentationVersion")
testImplementation "org.junit.jupiter:junit-jupiter-api"
testImplementation "org.assertj:assertj-core:$assertJCoreVersion"
testImplementation "org.mockito:mockito-core:$mockitoCoreVersion"
Expand Down
23 changes: 23 additions & 0 deletions servicetalk-opentelemetry-http/gradle/spotbugs/main-exclusions.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<?xml version="1.0" encoding="UTF-8"?>
<!--
~ 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.
-->
<FindBugsFilter>
<!-- List is copied at the builder level -->
<Match>
<Class name="io.servicetalk.opentelemetry.http.OpenTelemetryOptions"/>
<Bug pattern="EI_EXPOSE_REP"/>
</Match>
</FindBugsFilter>
23 changes: 23 additions & 0 deletions servicetalk-opentelemetry-http/gradle/spotbugs/test-exclusions.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<?xml version="1.0" encoding="UTF-8"?>
<!--
~ 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.
-->
<FindBugsFilter>
<!-- Fields are usually initialized in @BeforeClass/@Before methods instead of constructors for tests -->
<Match>
<Source name="~.*Test\.java"/>
<Bug pattern="NP_NONNULL_FIELD_NOT_INITIALIZED_IN_CONSTRUCTOR"/>
</Match>
</FindBugsFilter>
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -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;
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -60,7 +60,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;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -20,22 +20,31 @@
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;
import io.servicetalk.http.api.StreamingHttpConnectionFilterFactory;
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;

Expand All @@ -54,34 +63,85 @@
public final class OpenTelemetryHttpRequestFilter extends AbstractOpenTelemetryFilter
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we should rename this to OpenTelemetryHttpClientFilter to have proper alignment with the service?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's too late. We cannot break the contract with clients, renaming will cause a breaking change.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not in this PR, but in a follow up it will be nice to rename it to OpenTelemetryHttpRequesterFilter to be consistent with all other filters. We use "requester" on the client-side because all clients and connections implement HttpRequester interface.

For backward compatibility, we can deprecate current OpenTelemetryHttpRequestFilter and link to OpenTelemetryHttpRequesterFilter as a replacement.

implements StreamingHttpClientFilterFactory, StreamingHttpConnectionFilterFactory {

private final String componentName;
private final Instrumenter<HttpRequestMetaData, HttpResponseMetaData> instrumenter;

/**
* Create a new instance.
*
* @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.
*/
@Deprecated // FIXME: 0.43 - remove deprecated ctor
@SuppressWarnings("DeprecatedIsStillUsed")
public OpenTelemetryHttpRequestFilter(final OpenTelemetry openTelemetry, String componentName) {
super(openTelemetry);
this.componentName = componentName.trim();
this(openTelemetry, componentName, DEFAULT_OPTIONS);
}

/**
* Create a new instance, searching for any instance of an opentelemetry available.
*
* @param componentName The component name used during building new spans.
*/
public OpenTelemetryHttpRequestFilter(String componentName) {
this(GlobalOpenTelemetry.get(), componentName);
public OpenTelemetryHttpRequestFilter(final String componentName) {
this(componentName, DEFAULT_OPTIONS);
}

/**
* 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(final String componentName, final OpenTelemetryOptions opentelemetryOptions) {
this(GlobalOpenTelemetry.get(), componentName, opentelemetryOptions);
}

/**
* Create a new instance, searching for any instance of an opentelemetry available,
* using the hostname as the component name.
*/
public OpenTelemetryHttpRequestFilter() {
this(GlobalOpenTelemetry.get(), "");
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<HttpRequestMetaData> serverSpanNameExtractor =
HttpSpanNameExtractor.create(ServiceTalkHttpAttributesGetter.INSTANCE);
InstrumenterBuilder<HttpRequestMetaData, HttpResponseMetaData> clientInstrumenterBuilder =
Instrumenter.builder(openTelemetry, INSTRUMENTATION_SCOPE_NAME, serverSpanNameExtractor);
clientInstrumenterBuilder.setSpanStatusExtractor(ServicetalkSpanStatusExtractor.INSTANCE);

clientInstrumenterBuilder
.addAttributesExtractor(HttpClientAttributesExtractor
.builder(ServiceTalkHttpAttributesGetter.INSTANCE, ServiceTalkNetAttributesGetter.INSTANCE)
.setCapturedRequestHeaders(opentelemetryOptions.capturedRequestHeaders())
.setCapturedResponseHeaders(opentelemetryOptions.capturedResponseHeaders())
.build())
.addAttributesExtractor(NetClientAttributesExtractor.create(ServiceTalkNetAttributesGetter.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
Expand All @@ -108,34 +168,18 @@ public Single<StreamingHttpResponse> request(final StreamingHttpRequest request)

private Single<StreamingHttpResponse> 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();
final Context context = instrumenter.start(parentContext, request);

final Scope scope = context.makeCurrent();
final ScopeTracker tracker = new ScopeTracker(scope, context, request, instrumenter);
Single<StreamingHttpResponse> response;
try {
propagators.getTextMapPropagator().inject(Context.current(), request.headers(),
HeadersPropagatorSetter.INSTANCE);
response = delegate.request(request);
} catch (Throwable t) {
tracker.onError(t);
return Single.failed(t);
}
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();
}
}
Loading