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

Make net.transport an optional attribute #8279

Merged
merged 4 commits into from
Apr 20, 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
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,9 @@
public interface NetClientAttributesGetter<REQUEST, RESPONSE> {

@Nullable
String getTransport(REQUEST request, @Nullable RESPONSE response);
default String getTransport(REQUEST request, @Nullable RESPONSE response) {
return null;
}

/**
* Returns the application protocol used.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,9 @@
public interface NetServerAttributesGetter<REQUEST> {

@Nullable
String getTransport(REQUEST request);
default String getTransport(REQUEST request) {
return null;
}

/**
* Returns the application protocol used.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
package io.opentelemetry.instrumentation.api.instrumenter.http;

import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.assertThat;
import static io.opentelemetry.semconv.trace.attributes.SemanticAttributes.NetTransportValues.IP_TCP;
import static java.util.Arrays.asList;
import static java.util.Collections.emptyList;
import static java.util.Collections.singletonList;
Expand Down Expand Up @@ -72,13 +71,6 @@ public List<String> getResponseHeader(
static class TestNetClientAttributesGetter
implements NetClientAttributesGetter<Map<String, String>, Map<String, String>> {

@Nullable
@Override
public String getTransport(
Map<String, String> request, @Nullable Map<String, String> response) {
return response == null ? null : response.get("transport");
}

@Nullable
@Override
public String getProtocolName(
Expand Down Expand Up @@ -124,7 +116,6 @@ void normal() {
response.put("statusCode", "202");
response.put("header.content-length", "20");
response.put("header.custom-response-header", "654,321");
response.put("transport", IP_TCP);

ToIntFunction<Context> resendCountFromContext = context -> 2;

Expand Down Expand Up @@ -160,7 +151,6 @@ void normal() {
entry(
AttributeKey.stringArrayKey("http.response.header.custom_response_header"),
asList("654", "321")),
entry(SemanticAttributes.NET_TRANSPORT, IP_TCP),
entry(NetAttributes.NET_PROTOCOL_NAME, "http"),
entry(NetAttributes.NET_PROTOCOL_VERSION, "1.1"));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,11 +81,6 @@ public List<String> getResponseHeader(

static class TestNetServerAttributesGetter
implements NetServerAttributesGetter<Map<String, Object>> {
@Nullable
@Override
public String getTransport(Map<String, Object> request) {
return (String) request.get("transport");
}

@Nullable
@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
package io.opentelemetry.instrumentation.api.instrumenter.net;

import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.assertThat;
import static org.assertj.core.api.Assertions.entry;

import io.opentelemetry.api.common.Attributes;
import io.opentelemetry.api.common.AttributesBuilder;
Expand All @@ -25,10 +24,6 @@ class InetSocketAddressNetClientAttributesGetterTest {
private final InetSocketAddressNetClientAttributesGetter<InetSocketAddress, InetSocketAddress>
getter =
new InetSocketAddressNetClientAttributesGetter<InetSocketAddress, InetSocketAddress>() {
@Override
public String getTransport(InetSocketAddress request, InetSocketAddress response) {
return SemanticAttributes.NetTransportValues.IP_TCP;
}

@Override
public String getPeerName(InetSocketAddress request) {
Expand Down Expand Up @@ -56,9 +51,7 @@ void noInetSocketAddress() {

AttributesBuilder attributes = Attributes.builder();
extractor.onEnd(attributes, Context.root(), null, null, null);
assertThat(attributes.build())
.containsOnly(
entry(SemanticAttributes.NET_TRANSPORT, SemanticAttributes.NetTransportValues.IP_TCP));
assertThat(attributes.build()).isEmpty();
}

@Test
Expand All @@ -82,7 +75,6 @@ void fullAddress() {
assertThat(startAttributes.build()).isEmpty();

AttributesBuilder builder = Attributes.builder();
builder.put(SemanticAttributes.NET_TRANSPORT, SemanticAttributes.NetTransportValues.IP_TCP);
builder.put(SemanticAttributes.NET_SOCK_PEER_ADDR, address.getAddress().getHostAddress());
if (!ipv4) {
builder.put(SemanticAttributes.NET_SOCK_FAMILY, "inet6");
Expand Down Expand Up @@ -111,8 +103,6 @@ void unresolved() {
// then
assertThat(startAttributes.build()).isEmpty();

assertThat(endAttributes.build())
.containsOnly(
entry(SemanticAttributes.NET_TRANSPORT, SemanticAttributes.NetTransportValues.IP_TCP));
assertThat(endAttributes.build()).isEmpty();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
package io.opentelemetry.instrumentation.api.instrumenter.net;

import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.assertThat;
import static org.assertj.core.api.Assertions.entry;

import io.opentelemetry.api.common.Attributes;
import io.opentelemetry.api.common.AttributesBuilder;
Expand All @@ -25,11 +24,6 @@ class InetSocketAddressNetServerAttributesGetterTest {
final InetSocketAddressNetServerAttributesGetter<Addresses> getter =
new InetSocketAddressNetServerAttributesGetter<Addresses>() {

@Override
public String getTransport(Addresses request) {
return SemanticAttributes.NetTransportValues.IP_TCP;
}

@Override
public String getHostName(Addresses request) {
// net.host.name and net.host.port are tested in NetClientAttributesExtractorTest
Expand Down Expand Up @@ -59,9 +53,7 @@ protected InetSocketAddress getHostSocketAddress(Addresses request) {
void noInetSocketAddress() {
AttributesBuilder attributes = Attributes.builder();
extractor.onStart(attributes, Context.root(), new Addresses(null, null));
assertThat(attributes.build())
.containsOnly(
entry(SemanticAttributes.NET_TRANSPORT, SemanticAttributes.NetTransportValues.IP_TCP));
assertThat(attributes.build()).isEmpty();
}

@Test
Expand All @@ -84,7 +76,6 @@ void fullAddress() {

// then
AttributesBuilder builder = Attributes.builder();
builder.put(SemanticAttributes.NET_TRANSPORT, SemanticAttributes.NetTransportValues.IP_TCP);
if (!request.isIpv4()) {
builder.put(SemanticAttributes.NET_SOCK_FAMILY, "inet6");
}
Expand Down Expand Up @@ -118,9 +109,7 @@ void unresolved() {
extractor.onEnd(endAttributes, context, request, request, null);

// then
assertThat(startAttributes.build())
.containsOnly(
entry(SemanticAttributes.NET_TRANSPORT, SemanticAttributes.NetTransportValues.IP_TCP));
assertThat(startAttributes.build()).isEmpty();

assertThat(endAttributes.build()).isEmpty();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
import io.opentelemetry.instrumentation.api.instrumenter.http.HttpClientAttributesGetter;
import io.opentelemetry.instrumentation.api.instrumenter.http.HttpSpanNameExtractor;
import io.opentelemetry.instrumentation.api.instrumenter.net.InetSocketAddressNetClientAttributesGetter;
import io.opentelemetry.semconv.trace.attributes.SemanticAttributes;
import java.net.InetSocketAddress;
import java.util.Collections;
import java.util.List;
Expand Down Expand Up @@ -95,12 +94,6 @@ static class ConstantNetAttributesGetter
private static final InetSocketAddress PEER_ADDRESS =
InetSocketAddress.createUnresolved("localhost", 8080);

@Nullable
@Override
public String getTransport(Void request, @Nullable Void response) {
return SemanticAttributes.NetTransportValues.IP_TCP;
}

@Nullable
@Override
public String getProtocolName(Void unused, @Nullable Void unused2) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,10 @@
import akka.http.scaladsl.model.HttpResponse;
import io.opentelemetry.instrumentation.api.instrumenter.net.NetClientAttributesGetter;
import io.opentelemetry.javaagent.instrumentation.akkahttp.AkkaHttpUtil;
import io.opentelemetry.semconv.trace.attributes.SemanticAttributes;
import javax.annotation.Nullable;

class AkkaHttpNetAttributesGetter implements NetClientAttributesGetter<HttpRequest, HttpResponse> {

@Override
public String getTransport(HttpRequest httpRequest, @Nullable HttpResponse httpResponse) {
return SemanticAttributes.NetTransportValues.IP_TCP;
}

@Nullable
@Override
public String getProtocolName(HttpRequest httpRequest, @Nullable HttpResponse httpResponse) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,6 @@
// TODO (trask) capture net attributes?
class AkkaNetServerAttributesGetter implements NetServerAttributesGetter<HttpRequest> {

@Nullable
@Override
public String getTransport(HttpRequest request) {
return null;
}

@Nullable
@Override
public String getProtocolName(HttpRequest request) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,6 @@
public final class DubboNetClientAttributesGetter
extends InetSocketAddressNetClientAttributesGetter<DubboRequest, Result> {

@Override
@Nullable
public String getTransport(DubboRequest request, @Nullable Result response) {
return null;
}

@Nullable
@Override
public String getPeerName(DubboRequest request) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,6 @@
public final class DubboNetServerAttributesGetter
extends InetSocketAddressNetServerAttributesGetter<DubboRequest> {

@Override
@Nullable
public String getTransport(DubboRequest request) {
return null;
}

@Nullable
@Override
public String getHostName(DubboRequest request) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,19 +6,13 @@
package io.opentelemetry.javaagent.instrumentation.apachehttpasyncclient;

import io.opentelemetry.instrumentation.api.instrumenter.net.InetSocketAddressNetClientAttributesGetter;
import io.opentelemetry.semconv.trace.attributes.SemanticAttributes;
import java.net.InetSocketAddress;
import javax.annotation.Nullable;
import org.apache.http.HttpResponse;

final class ApacheHttpAsyncClientNetAttributesGetter
extends InetSocketAddressNetClientAttributesGetter<ApacheHttpClientRequest, HttpResponse> {

@Override
public String getTransport(ApacheHttpClientRequest request, @Nullable HttpResponse response) {
return SemanticAttributes.NetTransportValues.IP_TCP;
}

@Override
public String getProtocolName(ApacheHttpClientRequest request, @Nullable HttpResponse response) {
return request.getProtocolName();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
package io.opentelemetry.javaagent.instrumentation.apachehttpclient.v2_0;

import io.opentelemetry.instrumentation.api.instrumenter.net.NetClientAttributesGetter;
import io.opentelemetry.semconv.trace.attributes.SemanticAttributes;
import javax.annotation.Nullable;
import org.apache.commons.httpclient.HostConfiguration;
import org.apache.commons.httpclient.HttpMethod;
Expand All @@ -15,11 +14,6 @@
final class ApacheHttpClientNetAttributesGetter
implements NetClientAttributesGetter<HttpMethod, HttpMethod> {

@Override
public String getTransport(HttpMethod request, @Nullable HttpMethod response) {
return SemanticAttributes.NetTransportValues.IP_TCP;
}

@Override
public String getProtocolName(HttpMethod request, @Nullable HttpMethod response) {
return "http";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,18 +6,12 @@
package io.opentelemetry.javaagent.instrumentation.apachehttpclient.v4_0;

import io.opentelemetry.instrumentation.api.instrumenter.net.NetClientAttributesGetter;
import io.opentelemetry.semconv.trace.attributes.SemanticAttributes;
import javax.annotation.Nullable;
import org.apache.http.HttpResponse;

final class ApacheHttpClientNetAttributesGetter
implements NetClientAttributesGetter<ApacheHttpClientRequest, HttpResponse> {

@Override
public String getTransport(ApacheHttpClientRequest request, @Nullable HttpResponse response) {
return SemanticAttributes.NetTransportValues.IP_TCP;
}

@Override
public String getProtocolName(ApacheHttpClientRequest request, @Nullable HttpResponse response) {
return request.getProtocolName();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,19 +6,13 @@
package io.opentelemetry.instrumentation.apachehttpclient.v4_3;

import io.opentelemetry.instrumentation.api.instrumenter.net.InetSocketAddressNetClientAttributesGetter;
import io.opentelemetry.semconv.trace.attributes.SemanticAttributes;
import java.net.InetSocketAddress;
import javax.annotation.Nullable;
import org.apache.http.HttpResponse;

final class ApacheHttpClientNetAttributesGetter
extends InetSocketAddressNetClientAttributesGetter<ApacheHttpClientRequest, HttpResponse> {

@Override
public String getTransport(ApacheHttpClientRequest request, @Nullable HttpResponse response) {
return SemanticAttributes.NetTransportValues.IP_TCP;
}

@Override
public String getProtocolName(ApacheHttpClientRequest request, @Nullable HttpResponse response) {
return request.getProtocolName();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
package io.opentelemetry.javaagent.instrumentation.apachehttpclient.v5_0;

import io.opentelemetry.instrumentation.api.instrumenter.net.NetClientAttributesGetter;
import io.opentelemetry.semconv.trace.attributes.SemanticAttributes;
import javax.annotation.Nullable;
import org.apache.hc.core5.http.HttpRequest;
import org.apache.hc.core5.http.HttpResponse;
Expand All @@ -15,11 +14,6 @@
final class ApacheHttpClientNetAttributesGetter
implements NetClientAttributesGetter<HttpRequest, HttpResponse> {

@Override
public String getTransport(HttpRequest request, @Nullable HttpResponse response) {
return SemanticAttributes.NetTransportValues.IP_TCP;
}

@Nullable
@Override
public String getProtocolName(HttpRequest request, @Nullable HttpResponse response) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,19 +8,13 @@
import com.linecorp.armeria.common.RequestContext;
import com.linecorp.armeria.common.SessionProtocol;
import io.opentelemetry.instrumentation.api.instrumenter.net.InetSocketAddressNetServerAttributesGetter;
import io.opentelemetry.semconv.trace.attributes.SemanticAttributes;
import java.net.InetSocketAddress;
import java.net.SocketAddress;
import javax.annotation.Nullable;

final class ArmeriaNetServerAttributesGetter
extends InetSocketAddressNetServerAttributesGetter<RequestContext> {

@Override
public String getTransport(RequestContext ctx) {
return SemanticAttributes.NetTransportValues.IP_TCP;
}

@Override
public String getProtocolName(RequestContext ctx) {
return "http";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
import com.linecorp.armeria.common.SessionProtocol;
import com.linecorp.armeria.common.logging.RequestLog;
import io.opentelemetry.instrumentation.api.instrumenter.net.InetSocketAddressNetClientAttributesGetter;
import io.opentelemetry.semconv.trace.attributes.SemanticAttributes;
import java.net.InetSocketAddress;
import java.net.SocketAddress;
import javax.annotation.Nullable;
Expand All @@ -22,11 +21,6 @@
public final class ArmeriaNetClientAttributesGetter
extends InetSocketAddressNetClientAttributesGetter<RequestContext, RequestLog> {

@Override
public String getTransport(RequestContext ctx, @Nullable RequestLog requestLog) {
return SemanticAttributes.NetTransportValues.IP_TCP;
}

@Override
public String getProtocolName(RequestContext ctx, @Nullable RequestLog requestLog) {
return "http";
Expand Down
Loading