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.

Sign up for GitHub

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

Jump to bottom

Net attributes getters changes (in preparation for HTTP spec impl) #6503

Merged
merged 7 commits into from
Sep 12, 2022
Merged

Net attributes getters changes (in preparation for HTTP spec impl) #6503

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
3abaf5e
Net attributes getters changes: instrumentation-api-semconv changes
Aug 24, 2022
e925d1b
Net attributes getters changes: getter implementations
Aug 24, 2022
6574cbe
Net attributes getters changes: test fixes
Aug 25, 2022
47b0731
Remove net.sock.host.name
Aug 31, 2022
2957133
code review comments
Aug 31, 2022
11eaf00
default getter methods & getPeerSocketAddress() method name
Sep 8, 2022
84f87a7
set authority in grpc earlier
Sep 9, 2022
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
42 changes: 16 additions & 26 deletions ...etry/instrumentation/api/instrumenter/net/InetSocketAddressNetClientAttributesGetter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,32 +21,26 @@ public abstract class InetSocketAddressNetClientAttributesGetter<REQUEST, RESPON
implements NetClientAttributesGetter<REQUEST, RESPONSE> {

@Nullable
public abstract InetSocketAddress getAddress(REQUEST request, @Nullable RESPONSE response);
public abstract InetSocketAddress getPeerAddress(REQUEST request, @Nullable RESPONSE response);
mateuszrzeszutek marked this conversation as resolved.
Show resolved Hide resolved

@Override
@Nullable
public final String peerName(REQUEST request, @Nullable RESPONSE response) {
InetSocketAddress address = getAddress(request, response);
if (address == null) {
return null;
}
return address.getHostString();
}

@Override
@Nullable
public final Integer peerPort(REQUEST request, @Nullable RESPONSE response) {
InetSocketAddress address = getAddress(request, response);
public String sockFamily(REQUEST request, @Nullable RESPONSE response) {
InetSocketAddress address = getPeerAddress(request, response);
if (address == null) {
return null;
}
return address.getPort();
InetAddress remoteAddress = address.getAddress();
if (remoteAddress instanceof Inet6Address) {
return "inet6";
}
return null;
Copy link
Member

Choose a reason for hiding this comment

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

@lmolkova should we report inet when that is known? the spec says we don't need to, but it feels like we may be losing something by not reporting it?

Copy link
Member

Choose a reason for hiding this comment

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

I think I understand now.

It's conditionally required

If different than inet and if any of net.sock.peer.addr or net.sock.host.addr are set. Consumers of telemetry SHOULD accept both IPv4 and IPv6 formats for the address in net.sock.peer.addr if net.sock.family is not set. This is to support instrumentations that follow previous versions of this document.

So if consumers see net.sock.peer.addr or net.sock.host.addr, and they do not see any net.sock.family, then they are allowed to assume that the net.sock.family is inet.

}

@Override
@Nullable
public final String sockPeerAddr(REQUEST request, @Nullable RESPONSE response) {
InetSocketAddress address = getAddress(request, response);
InetSocketAddress address = getPeerAddress(request, response);
if (address == null) {
return null;
}
Expand All @@ -57,27 +51,23 @@ public final String sockPeerAddr(REQUEST request, @Nullable RESPONSE response) {
return null;
}

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

@Nullable
@Override
public String sockFamily(REQUEST request, @Nullable RESPONSE response) {
InetSocketAddress address = getAddress(request, response);
public Integer sockPeerPort(REQUEST request, @Nullable RESPONSE response) {
InetSocketAddress address = getPeerAddress(request, response);
if (address == null) {
return null;
}
InetAddress remoteAddress = address.getAddress();
if (remoteAddress instanceof Inet6Address) {
return "inet6";
}
return null;
return address.getPort();
}
}
62 changes: 57 additions & 5 deletions ...etry/instrumentation/api/instrumenter/net/InetSocketAddressNetServerAttributesGetter.java
View file
Open in desktop
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 All @@ -20,22 +21,65 @@ public abstract class InetSocketAddressNetServerAttributesGetter<REQUEST>
implements NetServerAttributesGetter<REQUEST> {

@Nullable
public abstract InetSocketAddress getAddress(REQUEST request);
public abstract InetSocketAddress getPeerAddress(REQUEST request);

// optional
@Nullable
public abstract InetSocketAddress getHostAddress(REQUEST request);

@Nullable
@Override
public String sockFamily(REQUEST request) {
InetSocketAddress address = getPeerAddress(request);
if (address == null) {
address = getHostAddress(request);
}
if (address == null) {
return null;
}
InetAddress inetAddress = address.getAddress();
if (inetAddress instanceof Inet6Address) {
return "inet6";
}
return null;
}

@Override
@Nullable
public final String sockPeerAddr(REQUEST request) {
return getAddress(getPeerAddress(request));
}

@Override
@Nullable
public final Integer sockPeerPort(REQUEST request) {
InetSocketAddress address = getAddress(request);
return getPort(getPeerAddress(request));
}

@Nullable
@Override
public String sockHostAddr(REQUEST request) {
return getAddress(getHostAddress(request));
}

@Nullable
@Override
public String sockHostName(REQUEST request) {
InetSocketAddress address = getHostAddress(request);
if (address == null) {
return null;
}
return address.getPort();
return address.getHostString();
}

@Nullable
@Override
public Integer sockHostPort(REQUEST request) {
return getPort(getHostAddress(request));
}

@Nullable
public final String sockPeerAddr(REQUEST request) {
InetSocketAddress address = getAddress(request);
private static String getAddress(InetSocketAddress address) {
if (address == null) {
return null;
}
Expand All @@ -45,4 +89,12 @@ public final String sockPeerAddr(REQUEST request) {
}
return null;
}

@Nullable
private static Integer getPort(InetSocketAddress address) {
if (address == null) {
return null;
}
return address.getPort();
}
}
28 changes: 28 additions & 0 deletions ...nv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/net/NetAttributes.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/

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

import io.opentelemetry.api.common.AttributeKey;

// this class will be removed once SemanticAttributes contains all new net.* attributes
final class NetAttributes {

static final AttributeKey<String> NET_SOCK_FAMILY = AttributeKey.stringKey("net.sock.family");
static final AttributeKey<String> NET_SOCK_PEER_ADDR =
AttributeKey.stringKey("net.sock.peer.addr");
static final AttributeKey<String> NET_SOCK_PEER_NAME =
AttributeKey.stringKey("net.sock.peer.name");
static final AttributeKey<Long> NET_SOCK_PEER_PORT = AttributeKey.longKey("net.sock.peer.port");
static final AttributeKey<String> NET_SOCK_HOST_ADDR =
AttributeKey.stringKey("net.sock.host.addr");
static final AttributeKey<String> NET_SOCK_HOST_NAME =
AttributeKey.stringKey("net.sock.host.name");
mateuszrzeszutek marked this conversation as resolved.
Show resolved Hide resolved
static final AttributeKey<Long> NET_SOCK_HOST_PORT = AttributeKey.longKey("net.sock.host.port");

static final String SOCK_FAMILY_INET = "inet";

private NetAttributes() {}
}
21 changes: 7 additions & 14 deletions ...a/io/opentelemetry/instrumentation/api/instrumenter/net/NetClientAttributesExtractor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@

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 @@ -26,15 +25,6 @@
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 Down Expand Up @@ -70,18 +60,21 @@ public void onEnd(

String sockPeerAddr = getter.sockPeerAddr(request, response);
if (sockPeerAddr != null && !sockPeerAddr.equals(peerName)) {
internalSet(attributes, NET_SOCK_PEER_ADDR, sockPeerAddr);
internalSet(attributes, NetAttributes.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, NetAttributes.NET_SOCK_PEER_PORT, (long) sockPeerPort);
}

internalSet(attributes, NET_SOCK_FAMILY, getter.sockFamily(request, response));
String sockFamily = getter.sockFamily(request, response);
if (sockFamily != null && !NetAttributes.SOCK_FAMILY_INET.equals(sockFamily)) {
internalSet(attributes, NetAttributes.NET_SOCK_FAMILY, sockFamily);
}

String sockPeerName = getter.sockPeerName(request, response);
if (sockPeerName != null && !sockPeerName.equals(peerName)) {
internalSet(attributes, NET_SOCK_PEER_NAME, sockPeerName);
internalSet(attributes, NetAttributes.NET_SOCK_PEER_NAME, sockPeerName);
}
}
}
Expand Down
18 changes: 6 additions & 12 deletions ...java/io/opentelemetry/instrumentation/api/instrumenter/net/NetClientAttributesGetter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,29 +20,23 @@ public interface NetClientAttributesGetter<REQUEST, RESPONSE> {
@Nullable
String transport(REQUEST request, @Nullable RESPONSE response);

// TODO: peerName and peerPort should be extracted onStart

@Nullable
String peerName(REQUEST request, @Nullable RESPONSE response);

@Nullable
Integer peerPort(REQUEST request, @Nullable RESPONSE response);

@Nullable
default String sockPeerAddr(REQUEST request, @Nullable RESPONSE response) {
return null;
}
String sockFamily(REQUEST request, @Nullable RESPONSE response);
mateuszrzeszutek marked this conversation as resolved.
Show resolved Hide resolved

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

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

@Nullable
default String sockPeerName(REQUEST request, @Nullable RESPONSE response) {
return null;
}
Integer sockPeerPort(REQUEST request, @Nullable RESPONSE response);
}
52 changes: 43 additions & 9 deletions ...a/io/opentelemetry/instrumentation/api/instrumenter/net/NetServerAttributesExtractor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@

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 @@ -23,10 +22,6 @@
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 @@ -42,13 +37,52 @@ private NetServerAttributesExtractor(NetServerAttributesGetter<REQUEST> getter)
public void onStart(AttributesBuilder attributes, Context parentContext, REQUEST request) {
internalSet(attributes, SemanticAttributes.NET_TRANSPORT, getter.transport(request));

boolean setSockFamily = false;

String sockPeerAddr = getter.sockPeerAddr(request);
if (sockPeerAddr != null) {
setSockFamily = true;

internalSet(attributes, NetAttributes.NET_SOCK_PEER_ADDR, sockPeerAddr);

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

String hostName = getter.hostName(request);
Integer hostPort = getter.hostPort(request);
if (hostName != null) {
internalSet(attributes, SemanticAttributes.NET_HOST_NAME, hostName);

internalSet(attributes, NET_SOCK_PEER_ADDR, sockPeerAddr);
if (hostPort != null && hostPort > 0) {
internalSet(attributes, SemanticAttributes.NET_HOST_PORT, (long) hostPort);
}
}

String sockHostAddr = getter.sockHostAddr(request);
if (sockHostAddr != null && !sockHostAddr.equals(hostName)) {
setSockFamily = true;

internalSet(attributes, NetAttributes.NET_SOCK_HOST_ADDR, sockHostAddr);

Integer sockHostPort = getter.sockHostPort(request);
if (sockHostPort != null && sockHostPort > 0 && !sockHostPort.equals(hostPort)) {
internalSet(attributes, NetAttributes.NET_SOCK_HOST_PORT, (long) sockHostPort);
}

String sockHostName = getter.sockHostName(request);
if (sockHostName != null && !sockHostName.equals(hostName)) {
internalSet(attributes, NetAttributes.NET_SOCK_HOST_NAME, sockHostName);
}
}

Integer sockPeerPort = getter.sockPeerPort(request);
if (sockPeerPort != null && sockPeerPort > 0) {
internalSet(attributes, NET_SOCK_PEER_PORT, (long) sockPeerPort);
if (setSockFamily) {
String sockFamily = getter.sockFamily(request);
if (sockFamily != null && !NetAttributes.SOCK_FAMILY_INET.equals(sockFamily)) {
internalSet(attributes, NetAttributes.NET_SOCK_FAMILY, sockFamily);
}
}
}

Expand Down
20 changes: 19 additions & 1 deletion ...java/io/opentelemetry/instrumentation/api/instrumenter/net/NetServerAttributesGetter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,26 @@ public interface NetServerAttributesGetter<REQUEST> {
String transport(REQUEST request);

@Nullable
Integer sockPeerPort(REQUEST request);
String hostName(REQUEST request);

@Nullable
Integer hostPort(REQUEST request);

@Nullable
String sockFamily(REQUEST request);

@Nullable
String sockPeerAddr(REQUEST request);

@Nullable
Integer sockPeerPort(REQUEST request);

@Nullable
String sockHostAddr(REQUEST request);

@Nullable
String sockHostName(REQUEST request);

@Nullable
Integer sockHostPort(REQUEST request);
}
Loading