Skip to content

Commit

Permalink
Add network peer address attributes to the span for the okhttp-3.0
Browse files Browse the repository at this point in the history
…instrumentation
  • Loading branch information
serkan-ozal committed Aug 15, 2024
1 parent 23a110e commit 4589148
Show file tree
Hide file tree
Showing 9 changed files with 77 additions and 38 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,12 @@
import io.opentelemetry.instrumentation.okhttp.v3_0.internal.TracingInterceptor;
import io.opentelemetry.javaagent.bootstrap.internal.JavaagentHttpClientInstrumenters;
import okhttp3.Interceptor;
import okhttp3.Request;
import okhttp3.Response;

/** Holder of singleton interceptors for adding to instrumented clients. */
public final class OkHttp3Singletons {

private static final Instrumenter<Request, Response> INSTRUMENTER =
private static final Instrumenter<Interceptor.Chain, Response> INSTRUMENTER =
JavaagentHttpClientInstrumenters.create(
OkHttpClientInstrumenterBuilderFactory.create(GlobalOpenTelemetry.get()));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
import okhttp3.Callback;
import okhttp3.Interceptor;
import okhttp3.OkHttpClient;
import okhttp3.Request;
import okhttp3.Response;

/** Entrypoint for instrumenting OkHttp clients. */
Expand All @@ -32,10 +31,11 @@ public static OkHttpTelemetryBuilder builder(OpenTelemetry openTelemetry) {
return new OkHttpTelemetryBuilder(openTelemetry);
}

private final Instrumenter<Request, Response> instrumenter;
private final Instrumenter<Interceptor.Chain, Response> instrumenter;
private final ContextPropagators propagators;

OkHttpTelemetry(Instrumenter<Request, Response> instrumenter, ContextPropagators propagators) {
OkHttpTelemetry(
Instrumenter<Interceptor.Chain, Response> instrumenter, ContextPropagators propagators) {
this.instrumenter = instrumenter;
this.propagators = propagators;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,13 @@
import java.util.List;
import java.util.Set;
import java.util.function.Function;
import okhttp3.Request;
import okhttp3.Interceptor;
import okhttp3.Response;

/** A builder of {@link OkHttpTelemetry}. */
public final class OkHttpTelemetryBuilder {

private final DefaultHttpClientInstrumenterBuilder<Request, Response> builder;
private final DefaultHttpClientInstrumenterBuilder<Interceptor.Chain, Response> builder;

OkHttpTelemetryBuilder(OpenTelemetry openTelemetry) {
builder = OkHttpClientInstrumenterBuilderFactory.create(openTelemetry);
Expand All @@ -33,7 +33,7 @@ public final class OkHttpTelemetryBuilder {
*/
@CanIgnoreReturnValue
public OkHttpTelemetryBuilder addAttributeExtractor(
AttributesExtractor<? super Request, ? super Response> attributesExtractor) {
AttributesExtractor<? super Interceptor.Chain, ? super Response> attributesExtractor) {
builder.addAttributeExtractor(attributesExtractor);
return this;
}
Expand Down Expand Up @@ -95,7 +95,9 @@ public OkHttpTelemetryBuilder setEmitExperimentalHttpClientMetrics(
/** Sets custom {@link SpanNameExtractor} via transform function. */
@CanIgnoreReturnValue
public OkHttpTelemetryBuilder setSpanNameExtractor(
Function<SpanNameExtractor<? super Request>, ? extends SpanNameExtractor<? super Request>>
Function<
SpanNameExtractor<? super Interceptor.Chain>,
? extends SpanNameExtractor<? super Interceptor.Chain>>
spanNameExtractorTransformer) {
builder.setSpanNameExtractor(spanNameExtractorTransformer);
return this;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,9 @@
*/
public final class ConnectionErrorSpanInterceptor implements Interceptor {

private final Instrumenter<Request, Response> instrumenter;
private final Instrumenter<Chain, Response> instrumenter;

public ConnectionErrorSpanInterceptor(Instrumenter<Request, Response> instrumenter) {
public ConnectionErrorSpanInterceptor(Instrumenter<Chain, Response> instrumenter) {
this.instrumenter = instrumenter;
}

Expand All @@ -43,9 +43,9 @@ public Response intercept(Chain chain) throws IOException {
} finally {
// only create a span when there wasn't any HTTP request
if (HttpClientRequestResendCount.get(parentContext) == 0) {
if (instrumenter.shouldStart(parentContext, request)) {
if (instrumenter.shouldStart(parentContext, chain)) {
InstrumenterUtil.startAndEnd(
instrumenter, parentContext, request, response, error, startTime, Instant.now());
instrumenter, parentContext, chain, response, error, startTime, Instant.now());
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,47 +6,52 @@
package io.opentelemetry.instrumentation.okhttp.v3_0.internal;

import io.opentelemetry.instrumentation.api.semconv.http.HttpClientAttributesGetter;
import java.net.InetSocketAddress;
import java.net.SocketAddress;
import java.util.List;
import javax.annotation.Nullable;
import okhttp3.Request;
import okhttp3.Connection;
import okhttp3.Interceptor;
import okhttp3.Response;

/**
* This class is internal and is hence not for public use. Its APIs are unstable and can change at
* any time.
*/
public enum OkHttpAttributesGetter implements HttpClientAttributesGetter<Request, Response> {
public enum OkHttpAttributesGetter
implements HttpClientAttributesGetter<Interceptor.Chain, Response> {
INSTANCE;

@Override
public String getHttpRequestMethod(Request request) {
return request.method();
public String getHttpRequestMethod(Interceptor.Chain chain) {
return chain.request().method();
}

@Override
public String getUrlFull(Request request) {
return request.url().toString();
public String getUrlFull(Interceptor.Chain chain) {
return chain.request().url().toString();
}

@Override
public List<String> getHttpRequestHeader(Request request, String name) {
return request.headers(name);
public List<String> getHttpRequestHeader(Interceptor.Chain chain, String name) {
return chain.request().headers(name);
}

@Override
public Integer getHttpResponseStatusCode(
Request request, Response response, @Nullable Throwable error) {
Interceptor.Chain chain, Response response, @Nullable Throwable error) {
return response.code();
}

@Override
public List<String> getHttpResponseHeader(Request request, Response response, String name) {
public List<String> getHttpResponseHeader(
Interceptor.Chain chain, Response response, String name) {
return response.headers(name);
}

@Nullable
@Override
public String getNetworkProtocolName(Request request, @Nullable Response response) {
public String getNetworkProtocolName(Interceptor.Chain chain, @Nullable Response response) {
if (response == null) {
return null;
}
Expand All @@ -67,7 +72,7 @@ public String getNetworkProtocolName(Request request, @Nullable Response respons

@Nullable
@Override
public String getNetworkProtocolVersion(Request request, @Nullable Response response) {
public String getNetworkProtocolVersion(Interceptor.Chain chain, @Nullable Response response) {
if (response == null) {
return null;
}
Expand All @@ -90,12 +95,28 @@ public String getNetworkProtocolVersion(Request request, @Nullable Response resp

@Override
@Nullable
public String getServerAddress(Request request) {
return request.url().host();
public String getServerAddress(Interceptor.Chain chain) {
return chain.request().url().host();
}

@Override
public Integer getServerPort(Request request) {
return request.url().port();
public Integer getServerPort(Interceptor.Chain chain) {
return chain.request().url().port();
}

@Nullable
@Override
public InetSocketAddress getNetworkPeerInetSocketAddress(
Interceptor.Chain chain, @Nullable Response response) {
Connection connection = chain.connection();
if (connection == null) {
return null;
}
SocketAddress socketAddress = connection.socket().getRemoteSocketAddress();
if (socketAddress instanceof InetSocketAddress) {
return (InetSocketAddress) socketAddress;
} else {
return null;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

import io.opentelemetry.api.OpenTelemetry;
import io.opentelemetry.instrumentation.api.incubator.builder.internal.DefaultHttpClientInstrumenterBuilder;
import okhttp3.Request;
import okhttp3.Interceptor;
import okhttp3.Response;

/**
Expand All @@ -19,7 +19,7 @@ public class OkHttpClientInstrumenterBuilderFactory {

private OkHttpClientInstrumenterBuilderFactory() {}

public static DefaultHttpClientInstrumenterBuilder<Request, Response> create(
public static DefaultHttpClientInstrumenterBuilder<Interceptor.Chain, Response> create(
OpenTelemetry openTelemetry) {
return new DefaultHttpClientInstrumenterBuilder<>(
INSTRUMENTATION_NAME, openTelemetry, OkHttpAttributesGetter.INSTANCE);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,11 @@
*/
public final class TracingInterceptor implements Interceptor {

private final Instrumenter<Request, Response> instrumenter;
private final Instrumenter<Chain, Response> instrumenter;
private final ContextPropagators propagators;

public TracingInterceptor(
Instrumenter<Request, Response> instrumenter, ContextPropagators propagators) {
Instrumenter<Chain, Response> instrumenter, ContextPropagators propagators) {
this.instrumenter = instrumenter;
this.propagators = propagators;
}
Expand All @@ -34,11 +34,11 @@ public Response intercept(Chain chain) throws IOException {
Request request = chain.request();
Context parentContext = Context.current();

if (!instrumenter.shouldStart(parentContext, request)) {
if (!instrumenter.shouldStart(parentContext, chain)) {
return chain.proceed(chain.request());
}

Context context = instrumenter.start(parentContext, request);
Context context = instrumenter.start(parentContext, chain);
request = injectContextToRequest(request, context);

Response response = null;
Expand All @@ -50,7 +50,7 @@ public Response intercept(Chain chain) throws IOException {
error = e;
throw e;
} finally {
instrumenter.end(context, request, response, error);
instrumenter.end(context, chain, response, error);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import io.opentelemetry.instrumentation.testing.junit.http.AbstractHttpClientTest;
import io.opentelemetry.instrumentation.testing.junit.http.HttpClientResult;
import io.opentelemetry.instrumentation.testing.junit.http.HttpClientTestOptions;
import io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions;
import io.opentelemetry.semconv.NetworkAttributes;
import java.io.IOException;
import java.net.URI;
Expand Down Expand Up @@ -180,7 +181,16 @@ public void onResponse(Call call, Response response) {
trace -> {
trace.hasSpansSatisfyingExactly(
span -> span.hasName("parent").hasKind(SpanKind.INTERNAL).hasNoParent(),
span -> span.hasName("GET").hasKind(SpanKind.CLIENT).hasParent(trace.getSpan(0)),
span ->
span.hasName("GET")
.hasKind(SpanKind.CLIENT)
.hasParent(trace.getSpan(0))
.hasAttribute(
OpenTelemetryAssertions.satisfies(
NetworkAttributes.NETWORK_PEER_ADDRESS,
val -> val.isIn("127.0.0.1", "0:0:0:0:0:0:0:1")))
.hasAttribute(
NetworkAttributes.NETWORK_PEER_PORT, (long) serverAddress().getPort()),
span ->
span.hasName("test-http-server")
.hasKind(SpanKind.SERVER)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import io.opentelemetry.semconv.ServerAttributes;
import io.opentelemetry.semconv.UrlAttributes;
import io.opentelemetry.semconv.UserAgentAttributes;
import java.net.InetSocketAddress;
import java.net.URI;
import java.time.Duration;
import java.util.ArrayList;
Expand Down Expand Up @@ -87,6 +88,10 @@ void setupOptions() {
options = builder.build();
}

protected InetSocketAddress serverAddress() {
return server.httpSocketAddress();
}

/**
* Override this method to configure the {@link HttpClientTestOptions} for the tested HTTP client.
*/
Expand Down Expand Up @@ -999,7 +1004,9 @@ SpanDataAssert assertClientSpan(
// TODO: Move to test knob rather than always treating as optional
if (attrs.get(NetworkAttributes.NETWORK_PEER_ADDRESS) != null) {
assertThat(attrs)
.containsEntry(NetworkAttributes.NETWORK_PEER_ADDRESS, "127.0.0.1");
.hasEntrySatisfying(
NetworkAttributes.NETWORK_PEER_ADDRESS,
addr -> assertThat(addr).isIn("127.0.0.1", "0:0:0:0:0:0:0:1"));
}
if (attrs.get(NetworkAttributes.NETWORK_PEER_PORT) != null) {
assertThat(attrs)
Expand Down

0 comments on commit 4589148

Please sign in to comment.