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

Update net semantic convention changes #6268

Merged
merged 16 commits into from
Aug 18, 2022
Merged
Original file line number Diff line number Diff line change
Expand Up @@ -76,10 +76,6 @@ public void onEnd(

String peerName = attributesGetter.peerName(request, response);
String peerService = mapToPeerService(peerName);
if (peerService == null) {
String peerIp = attributesGetter.peerIp(request, response);
peerService = mapToPeerService(peerIp);
}
Comment on lines -79 to -82
Copy link
Member Author

Choose a reason for hiding this comment

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

alternatively we could check both net.sock.peer.addr and net.sock.peer.name, but now that we are putting IP address into net.peer.name when there's no hostname I don't think there is a need(?)

if (peerService != null) {
attributes.put(SemanticAttributes.PEER_SERVICE, peerService);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

package io.opentelemetry.instrumentation.api.instrumenter.net;

import java.net.Inet6Address;
import java.net.InetAddress;
import java.net.InetSocketAddress;
import javax.annotation.Nullable;
Expand Down Expand Up @@ -44,7 +45,7 @@ public final Integer peerPort(REQUEST request, @Nullable RESPONSE response) {

@Override
@Nullable
public final String peerIp(REQUEST request, @Nullable RESPONSE response) {
public final String sockPeerAddr(REQUEST request, @Nullable RESPONSE response) {
InetSocketAddress address = getAddress(request, response);
if (address == null) {
return null;
Expand All @@ -55,4 +56,28 @@ public final String peerIp(REQUEST request, @Nullable RESPONSE response) {
}
return null;
}

@Nullable
@Override
public Integer sockPeerPort(REQUEST request, @Nullable RESPONSE response) {
InetSocketAddress address = getAddress(request, response);
if (address == null) {
return null;
}
return address.getPort();
}

@Nullable
@Override
public String sockFamily(REQUEST request, @Nullable RESPONSE response) {
InetSocketAddress address = getAddress(request, response);
if (address == null) {
return null;
}
InetAddress remoteAddress = address.getAddress();
if (remoteAddress instanceof Inet6Address) {
return "inet6";
}
return null;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ public abstract class InetSocketAddressNetServerAttributesGetter<REQUEST>

@Override
@Nullable
public final Integer peerPort(REQUEST request) {
public final Integer sockPeerPort(REQUEST request) {
InetSocketAddress address = getAddress(request);
if (address == null) {
return null;
Expand All @@ -34,7 +34,7 @@ public final Integer peerPort(REQUEST request) {

@Override
@Nullable
public final String peerIp(REQUEST request) {
public final String sockPeerAddr(REQUEST request) {
InetSocketAddress address = getAddress(request);
if (address == null) {
return null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

import static io.opentelemetry.instrumentation.api.internal.AttributesExtractorUtil.internalSet;

import io.opentelemetry.api.common.AttributeKey;
import io.opentelemetry.api.common.AttributesBuilder;
import io.opentelemetry.context.Context;
import io.opentelemetry.instrumentation.api.instrumenter.AttributesExtractor;
Expand All @@ -25,6 +26,15 @@
public final class NetClientAttributesExtractor<REQUEST, RESPONSE>
implements AttributesExtractor<REQUEST, RESPONSE> {

private static final AttributeKey<String> NET_SOCK_PEER_ADDR =
AttributeKey.stringKey("net.sock.peer.addr");
public static final AttributeKey<Long> NET_SOCK_PEER_PORT =
AttributeKey.longKey("net.sock.peer.port");
public static final AttributeKey<String> NET_SOCK_FAMILY =
AttributeKey.stringKey("net.sock.family");
public static final AttributeKey<String> NET_SOCK_PEER_NAME =
AttributeKey.stringKey("net.sock.peer.name");

private final NetClientAttributesGetter<REQUEST, RESPONSE> getter;

public static <REQUEST, RESPONSE> NetClientAttributesExtractor<REQUEST, RESPONSE> create(
Expand All @@ -49,17 +59,30 @@ public void onEnd(

internalSet(attributes, SemanticAttributes.NET_TRANSPORT, getter.transport(request, response));

String peerIp = getter.peerIp(request, response);
String peerName = getter.peerName(request, response);

if (peerName != null && !peerName.equals(peerIp)) {
Integer peerPort = getter.peerPort(request, response);
if (peerName != null) {
internalSet(attributes, SemanticAttributes.NET_PEER_NAME, peerName);
if (peerPort != null && peerPort > 0) {
internalSet(attributes, SemanticAttributes.NET_PEER_PORT, (long) peerPort);
}
}
internalSet(attributes, SemanticAttributes.NET_PEER_IP, peerIp);

Integer peerPort = getter.peerPort(request, response);
if (peerPort != null && peerPort > 0) {
internalSet(attributes, SemanticAttributes.NET_PEER_PORT, (long) peerPort);
String sockPeerAddr = getter.sockPeerAddr(request, response);
if (sockPeerAddr != null && !sockPeerAddr.equals(peerName)) {
internalSet(attributes, NET_SOCK_PEER_ADDR, sockPeerAddr);

Integer sockPeerPort = getter.sockPeerPort(request, response);
if (sockPeerPort != null && sockPeerPort > 0 && !sockPeerPort.equals(peerPort)) {
internalSet(attributes, NET_SOCK_PEER_PORT, (long) sockPeerPort);
}

internalSet(attributes, NET_SOCK_FAMILY, getter.sockFamily(request, response));

String sockPeerName = getter.sockPeerName(request, response);
if (sockPeerName != null && !sockPeerName.equals(peerName)) {
internalSet(attributes, NET_SOCK_PEER_NAME, sockPeerName);
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,32 @@ public interface NetClientAttributesGetter<REQUEST, RESPONSE> {
@Nullable
Integer peerPort(REQUEST request, @Nullable RESPONSE response);

/**
* @deprecated implement {@link #sockPeerAddr(Object, Object)} instead.
*/
@Deprecated
@Nullable
String peerIp(REQUEST request, @Nullable RESPONSE response);
trask marked this conversation as resolved.
Show resolved Hide resolved
default String peerIp(REQUEST request, @Nullable RESPONSE response) {
return null;
}

@Nullable
default String sockPeerAddr(REQUEST request, @Nullable RESPONSE response) {
return peerIp(request, response);
}

@Nullable
default Integer sockPeerPort(REQUEST request, @Nullable RESPONSE response) {
return null;
}

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

@Nullable
default String sockPeerName(REQUEST request, @Nullable RESPONSE response) {
return null;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

import static io.opentelemetry.instrumentation.api.internal.AttributesExtractorUtil.internalSet;

import io.opentelemetry.api.common.AttributeKey;
import io.opentelemetry.api.common.AttributesBuilder;
import io.opentelemetry.context.Context;
import io.opentelemetry.instrumentation.api.instrumenter.AttributesExtractor;
Expand All @@ -22,6 +23,10 @@
public final class NetServerAttributesExtractor<REQUEST, RESPONSE>
implements AttributesExtractor<REQUEST, RESPONSE> {

public static final AttributeKey<String> NET_SOCK_PEER_ADDR =
AttributeKey.stringKey("net.sock.peer.addr");
public static final AttributeKey<Long> NET_SOCK_PEER_PORT =
AttributeKey.longKey("net.sock.peer.port");
private final NetServerAttributesGetter<REQUEST> getter;

public static <REQUEST, RESPONSE> NetServerAttributesExtractor<REQUEST, RESPONSE> create(
Expand All @@ -37,13 +42,13 @@ private NetServerAttributesExtractor(NetServerAttributesGetter<REQUEST> getter)
public void onStart(AttributesBuilder attributes, Context parentContext, REQUEST request) {
internalSet(attributes, SemanticAttributes.NET_TRANSPORT, getter.transport(request));

String peerIp = getter.peerIp(request);
String sockPeerAddr = getter.sockPeerAddr(request);

internalSet(attributes, SemanticAttributes.NET_PEER_IP, peerIp);
internalSet(attributes, NET_SOCK_PEER_ADDR, sockPeerAddr);

Integer peerPort = getter.peerPort(request);
if (peerPort != null && peerPort > 0) {
internalSet(attributes, SemanticAttributes.NET_PEER_PORT, (long) peerPort);
Integer sockPeerPort = getter.sockPeerPort(request);
if (sockPeerPort != null && sockPeerPort > 0) {
internalSet(attributes, NET_SOCK_PEER_PORT, (long) sockPeerPort);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@
import javax.annotation.Nullable;

/**
* An interface for getting server-based network attributes. It adapts a vendor-specific request
* type into the 3 common attributes (transport, peerPort, peerIp).
* An interface for getting server-based network attributes. It adapts an instrumentation-specific
* request type into the 3 common attributes (transport, sockPeerPort, sockPeerAddr).
*
* <p>Instrumentation authors will create implementations of this interface for their specific
* server library/framework. It will be used by the {@link NetServerAttributesExtractor} to obtain
Expand All @@ -20,9 +20,21 @@ public interface NetServerAttributesGetter<REQUEST> {
@Nullable
String transport(REQUEST request);

/**
* @deprecated implement {@link #sockPeerAddr(Object)} instead.
*/
@Deprecated
@Nullable
Integer peerPort(REQUEST request);
trask marked this conversation as resolved.
Show resolved Hide resolved
default String peerIp(REQUEST request) {
return null;
}

@Nullable
String peerIp(REQUEST request);
Integer sockPeerPort(REQUEST request);
mateuszrzeszutek marked this conversation as resolved.
Show resolved Hide resolved

@Nullable
default String sockPeerAddr(REQUEST request) {
// TODO (trask) remove default after removing peerIp() method
return peerIp(request);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ final class MetricsView {

private static final Set<AttributeKey> alwaysInclude = buildAlwaysInclude();
private static final Set<AttributeKey> clientView = buildClientView();
private static final Set<AttributeKey> clientFallbackView = buildClientFallbackView();
private static final Set<AttributeKey> serverView = buildServerView();
private static final Set<AttributeKey> serverFallbackView = buildServerFallbackView();

Expand All @@ -44,16 +43,6 @@ private static Set<AttributeKey> buildClientView() {
return view;
}

private static Set<AttributeKey> buildClientFallbackView() {
// the list of rpc client metrics attributes is from
// https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/semantic_conventions/rpc.md#attributes
Set<AttributeKey> view = new HashSet<>(alwaysInclude);
view.add(SemanticAttributes.NET_PEER_IP);
view.add(SemanticAttributes.NET_PEER_PORT);
view.add(SemanticAttributes.NET_TRANSPORT);
return view;
}

private static Set<AttributeKey> buildServerView() {
// the list of rpc server metrics attributes is from
// https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/semantic_conventions/rpc.md#attributes
Expand All @@ -78,11 +67,7 @@ private static <T> boolean containsAttribute(
}

static Attributes applyClientView(Attributes startAttributes, Attributes endAttributes) {
Set<AttributeKey> fullSet = clientView;
if (!containsAttribute(SemanticAttributes.NET_PEER_NAME, startAttributes, endAttributes)) {
fullSet = clientFallbackView;
}
return applyView(fullSet, startAttributes, endAttributes);
return applyView(clientView, startAttributes, endAttributes);
}

static Attributes applyServerView(Attributes startAttributes, Attributes endAttributes) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,29 +70,6 @@ void shouldNotSetAnyValueIfPeerNameDoesNotMatch() {
assertTrue(endAttributes.build().isEmpty());
}

@Test
void shouldNotSetAnyValueIfPeerIpDoesNotMatch() {
// given
Map<String, String> peerServiceMapping = singletonMap("1.2.3.4", "myService");

PeerServiceAttributesExtractor<String, String> underTest =
new PeerServiceAttributesExtractor<>(netAttributesExtractor, peerServiceMapping);

given(netAttributesExtractor.peerIp(any(), any())).willReturn("1.2.3.5");

Context context = Context.root();

// when
AttributesBuilder startAttributes = Attributes.builder();
underTest.onStart(startAttributes, context, "request");
AttributesBuilder endAttributes = Attributes.builder();
underTest.onEnd(endAttributes, context, "request", "response", null);

// then
assertTrue(startAttributes.build().isEmpty());
assertTrue(endAttributes.build().isEmpty());
}

@Test
void shouldSetPeerNameIfItMatches() {
// given
Expand All @@ -118,31 +95,4 @@ void shouldSetPeerNameIfItMatches() {
assertThat(endAttributes.build())
.containsOnly(entry(SemanticAttributes.PEER_SERVICE, "myService"));
}

@Test
void shouldSetPeerIpIfItMatchesAndNameDoesNot() {
// given
Map<String, String> peerServiceMapping = new HashMap<>();
peerServiceMapping.put("example.com", "myService");
peerServiceMapping.put("1.2.3.4", "someOtherService");

PeerServiceAttributesExtractor<String, String> underTest =
new PeerServiceAttributesExtractor<>(netAttributesExtractor, peerServiceMapping);

given(netAttributesExtractor.peerName(any(), any())).willReturn("test.com");
given(netAttributesExtractor.peerIp(any(), any())).willReturn("1.2.3.4");

Context context = Context.root();

// when
AttributesBuilder startAttributes = Attributes.builder();
underTest.onStart(startAttributes, context, "request");
AttributesBuilder endAttributes = Attributes.builder();
underTest.onEnd(endAttributes, context, "request", "response", null);

// then
assertThat(startAttributes.build()).isEmpty();
assertThat(endAttributes.build())
.containsOnly(entry(SemanticAttributes.PEER_SERVICE, "someOtherService"));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ void collectsMetrics() {
.put("http.target", "/")
.put("http.scheme", "https")
.put("net.peer.name", "localhost")
.put("net.peer.ip", "0.0.0.0")
.put("net.peer.port", 1234)
.put("http.request_content_length", 100)
.build();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ void shouldApplyClientDurationAndSizeView() {
Attributes.builder()
.put(SemanticAttributes.HTTP_STATUS_CODE, 500)
.put(SemanticAttributes.NET_PEER_NAME, "somehost2")
.put(SemanticAttributes.NET_PEER_IP, "127.0.0.1")
.put(SemanticAttributes.NET_PEER_PORT, 443)
.build();

Expand Down Expand Up @@ -70,7 +69,6 @@ void shouldApplyServerDurationAndSizeView() {
Attributes.builder()
.put(SemanticAttributes.HTTP_STATUS_CODE, 500)
.put(SemanticAttributes.NET_PEER_NAME, "somehost2")
.put(SemanticAttributes.NET_PEER_IP, "127.0.0.1")
.put(SemanticAttributes.NET_PEER_PORT, 443)
.build();

Expand Down
Loading