Minor fixes to the `server.*` attributes extrator (#8772)
This commit is contained in:
parent
d38a00633a
commit
f197befb50
|
@ -45,25 +45,22 @@ public final class InternalServerAttributesExtractor<REQUEST, RESPONSE> {
|
|||
|
||||
public void onStart(AttributesBuilder attributes, REQUEST request) {
|
||||
String serverAddress = extractServerAddress(request);
|
||||
if (emitStableUrlAttributes) {
|
||||
internalSet(attributes, NetworkAttributes.SERVER_ADDRESS, serverAddress);
|
||||
}
|
||||
if (emitOldHttpAttributes) {
|
||||
internalSet(attributes, oldSemconvMode.address, serverAddress);
|
||||
}
|
||||
|
||||
if (serverAddress != null) {
|
||||
Integer serverPort = extractServerPort(request);
|
||||
if (serverPort != null
|
||||
&& serverPort > 0
|
||||
&& captureServerPortCondition.test(serverPort, request)) {
|
||||
if (emitStableUrlAttributes) {
|
||||
internalSet(attributes, NetworkAttributes.SERVER_ADDRESS, serverAddress);
|
||||
internalSet(attributes, NetworkAttributes.SERVER_PORT, (long) serverPort);
|
||||
}
|
||||
if (emitOldHttpAttributes) {
|
||||
internalSet(attributes, oldSemconvMode.address, serverAddress);
|
||||
}
|
||||
|
||||
Integer serverPort = extractServerPort(request);
|
||||
if (serverPort != null
|
||||
&& serverPort > 0
|
||||
&& captureServerPortCondition.test(serverPort, request)) {
|
||||
if (emitStableUrlAttributes) {
|
||||
internalSet(attributes, NetworkAttributes.SERVER_PORT, (long) serverPort);
|
||||
}
|
||||
if (emitOldHttpAttributes) {
|
||||
internalSet(attributes, oldSemconvMode.port, (long) serverPort);
|
||||
}
|
||||
internalSet(attributes, oldSemconvMode.port, (long) serverPort);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
@ -79,28 +76,26 @@ public final class InternalServerAttributesExtractor<REQUEST, RESPONSE> {
|
|||
if (emitOldHttpAttributes) {
|
||||
internalSet(attributes, oldSemconvMode.socketAddress, serverSocketAddress);
|
||||
}
|
||||
}
|
||||
|
||||
Integer serverPort = extractServerPort(request);
|
||||
Integer serverSocketPort = getter.getServerSocketPort(request, response);
|
||||
if (serverSocketPort != null
|
||||
&& serverSocketPort > 0
|
||||
&& !serverSocketPort.equals(serverPort)) {
|
||||
if (emitStableUrlAttributes) {
|
||||
internalSet(attributes, NetworkAttributes.SERVER_SOCKET_PORT, (long) serverSocketPort);
|
||||
}
|
||||
if (emitOldHttpAttributes) {
|
||||
internalSet(attributes, oldSemconvMode.socketPort, (long) serverSocketPort);
|
||||
}
|
||||
Integer serverPort = extractServerPort(request);
|
||||
Integer serverSocketPort = getter.getServerSocketPort(request, response);
|
||||
if (serverSocketPort != null && serverSocketPort > 0 && !serverSocketPort.equals(serverPort)) {
|
||||
if (emitStableUrlAttributes) {
|
||||
internalSet(attributes, NetworkAttributes.SERVER_SOCKET_PORT, (long) serverSocketPort);
|
||||
}
|
||||
if (emitOldHttpAttributes) {
|
||||
internalSet(attributes, oldSemconvMode.socketPort, (long) serverSocketPort);
|
||||
}
|
||||
}
|
||||
|
||||
String serverSocketDomain = getter.getServerSocketDomain(request, response);
|
||||
if (serverSocketDomain != null && !serverSocketDomain.equals(serverAddress)) {
|
||||
if (emitStableUrlAttributes) {
|
||||
internalSet(attributes, NetworkAttributes.SERVER_SOCKET_DOMAIN, serverSocketDomain);
|
||||
}
|
||||
if (emitOldHttpAttributes && oldSemconvMode.socketDomain != null) {
|
||||
internalSet(attributes, oldSemconvMode.socketDomain, serverSocketDomain);
|
||||
}
|
||||
String serverSocketDomain = getter.getServerSocketDomain(request, response);
|
||||
if (serverSocketDomain != null && !serverSocketDomain.equals(serverAddress)) {
|
||||
if (emitStableUrlAttributes) {
|
||||
internalSet(attributes, NetworkAttributes.SERVER_SOCKET_DOMAIN, serverSocketDomain);
|
||||
}
|
||||
if (emitOldHttpAttributes && oldSemconvMode.socketDomain != null) {
|
||||
internalSet(attributes, oldSemconvMode.socketDomain, serverSocketDomain);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
@ -6,6 +6,7 @@
|
|||
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;
|
||||
|
@ -103,6 +104,9 @@ class InetSocketAddressNetClientAttributesGetterTest {
|
|||
// then
|
||||
assertThat(startAttributes.build()).isEmpty();
|
||||
|
||||
assertThat(endAttributes.build()).isEmpty();
|
||||
assertThat(endAttributes.build())
|
||||
.containsOnly(
|
||||
entry(SemanticAttributes.NET_SOCK_PEER_NAME, "api.github.com"),
|
||||
entry(SemanticAttributes.NET_SOCK_PEER_PORT, 456L));
|
||||
}
|
||||
}
|
||||
|
|
|
@ -117,7 +117,9 @@ class InetSocketAddressNetServerAttributesGetterTest {
|
|||
assertThat(startAttributes.build()).isEmpty();
|
||||
|
||||
assertThat(endAttributes.build())
|
||||
.containsOnly(entry(SemanticAttributes.NET_SOCK_PEER_PORT, 123L));
|
||||
.containsOnly(
|
||||
entry(SemanticAttributes.NET_SOCK_PEER_PORT, 123L),
|
||||
entry(SemanticAttributes.NET_SOCK_HOST_PORT, 456L));
|
||||
}
|
||||
|
||||
static final class Addresses {
|
||||
|
|
|
@ -157,7 +157,8 @@ class NetClientAttributesExtractorTest {
|
|||
}
|
||||
|
||||
@Test
|
||||
@DisplayName("does not set any net.sock.* attributes when net.peer.name = net.sock.peer.addr")
|
||||
@DisplayName(
|
||||
"does not set those net.sock.peer.* attributes that duplicate corresponding net.peer.* attributes")
|
||||
void doesNotSetDuplicates1() {
|
||||
// given
|
||||
Map<String, String> map = new HashMap<>();
|
||||
|
@ -184,7 +185,11 @@ class NetClientAttributesExtractorTest {
|
|||
entry(SemanticAttributes.NET_PEER_NAME, "1:2:3:4::"),
|
||||
entry(SemanticAttributes.NET_PEER_PORT, 42L));
|
||||
|
||||
assertThat(endAttributes.build()).containsOnly(entry(SemanticAttributes.NET_TRANSPORT, IP_TCP));
|
||||
assertThat(endAttributes.build())
|
||||
.containsOnly(
|
||||
entry(SemanticAttributes.NET_TRANSPORT, IP_TCP),
|
||||
entry(SemanticAttributes.NET_SOCK_PEER_NAME, "proxy.opentelemetry.io"),
|
||||
entry(SemanticAttributes.NET_SOCK_PEER_PORT, 123L));
|
||||
}
|
||||
|
||||
@Test
|
||||
|
|
|
@ -166,7 +166,7 @@ class NetServerAttributesExtractorTest {
|
|||
|
||||
@Test
|
||||
@DisplayName(
|
||||
"does not set any net.sock.host.* attributes when net.host.name = net.sock.host.addr")
|
||||
"does not set those net.sock.host.* attributes that duplicate corresponding net.host.* attributes")
|
||||
void doesNotSetDuplicates1() {
|
||||
// given
|
||||
Map<String, String> map = new HashMap<>();
|
||||
|
@ -193,7 +193,8 @@ class NetServerAttributesExtractorTest {
|
|||
entry(SemanticAttributes.NET_HOST_NAME, "4:3:2:1::"),
|
||||
entry(SemanticAttributes.NET_HOST_PORT, 80L));
|
||||
|
||||
assertThat(endAttributes.build()).isEmpty();
|
||||
assertThat(endAttributes.build())
|
||||
.containsOnly(entry(SemanticAttributes.NET_SOCK_HOST_PORT, 8080L));
|
||||
}
|
||||
|
||||
@Test
|
||||
|
|
|
@ -121,6 +121,27 @@ class ServerAttributesExtractorTest {
|
|||
.containsOnly(entry(NetworkAttributes.SERVER_SOCKET_ADDRESS, "1.2.3.4"));
|
||||
}
|
||||
|
||||
// TODO: add more test cases around duplicate data once
|
||||
// https://github.com/open-telemetry/semantic-conventions/issues/85 clears up
|
||||
@Test
|
||||
void doesNotSetDuplicates() {
|
||||
Map<String, String> request = new HashMap<>();
|
||||
request.put("address", "1.2.3.4");
|
||||
request.put("port", "80");
|
||||
request.put("socketDomain", "1.2.3.4");
|
||||
request.put("socketAddress", "1.2.3.4");
|
||||
request.put("socketPort", "80");
|
||||
|
||||
AttributesExtractor<Map<String, String>, Void> extractor =
|
||||
ServerAttributesExtractor.create(new TestServerAttributesGetter());
|
||||
|
||||
AttributesBuilder startAttributes = Attributes.builder();
|
||||
extractor.onStart(startAttributes, Context.root(), request);
|
||||
assertThat(startAttributes.build())
|
||||
.containsOnly(
|
||||
entry(NetworkAttributes.SERVER_ADDRESS, "1.2.3.4"),
|
||||
entry(NetworkAttributes.SERVER_PORT, 80L));
|
||||
|
||||
AttributesBuilder endAttributes = Attributes.builder();
|
||||
extractor.onEnd(endAttributes, Context.root(), request, null, null);
|
||||
assertThat(endAttributes.build()).isEmpty();
|
||||
}
|
||||
}
|
||||
|
|
|
@ -116,6 +116,7 @@ abstract class AbstractDubboTest extends InstrumentationSpecification {
|
|||
"$SemanticAttributes.RPC_METHOD" "hello"
|
||||
"$SemanticAttributes.NET_SOCK_PEER_ADDR" String
|
||||
"$SemanticAttributes.NET_SOCK_PEER_PORT" Long
|
||||
"$SemanticAttributes.NET_SOCK_HOST_PORT" Long
|
||||
"$SemanticAttributes.NET_SOCK_FAMILY" { it == SemanticAttributes.NetSockFamilyValues.INET6 || it == null }
|
||||
}
|
||||
}
|
||||
|
@ -189,6 +190,7 @@ abstract class AbstractDubboTest extends InstrumentationSpecification {
|
|||
"$SemanticAttributes.RPC_METHOD" "hello"
|
||||
"$SemanticAttributes.NET_SOCK_PEER_ADDR" String
|
||||
"$SemanticAttributes.NET_SOCK_PEER_PORT" Long
|
||||
"$SemanticAttributes.NET_SOCK_HOST_PORT" Long
|
||||
"$SemanticAttributes.NET_SOCK_FAMILY" { it == SemanticAttributes.NetSockFamilyValues.INET6 || it == null }
|
||||
}
|
||||
}
|
||||
|
|
|
@ -152,6 +152,7 @@ abstract class AbstractDubboTraceChainTest extends InstrumentationSpecification
|
|||
"$SemanticAttributes.RPC_METHOD" "hello"
|
||||
"$SemanticAttributes.NET_SOCK_PEER_ADDR" String
|
||||
"$SemanticAttributes.NET_SOCK_PEER_PORT" Long
|
||||
"$SemanticAttributes.NET_SOCK_HOST_PORT" Long
|
||||
"$SemanticAttributes.NET_SOCK_FAMILY" { it == SemanticAttributes.NetSockFamilyValues.INET6 || it == null }
|
||||
}
|
||||
}
|
||||
|
@ -180,6 +181,7 @@ abstract class AbstractDubboTraceChainTest extends InstrumentationSpecification
|
|||
"$SemanticAttributes.RPC_METHOD" "hello"
|
||||
"$SemanticAttributes.NET_SOCK_PEER_ADDR" String
|
||||
"$SemanticAttributes.NET_SOCK_PEER_PORT" Long
|
||||
"$SemanticAttributes.NET_SOCK_HOST_PORT" Long
|
||||
"$SemanticAttributes.NET_SOCK_FAMILY" { it == SemanticAttributes.NetSockFamilyValues.INET6 || it == null }
|
||||
}
|
||||
}
|
||||
|
@ -265,6 +267,7 @@ abstract class AbstractDubboTraceChainTest extends InstrumentationSpecification
|
|||
"$SemanticAttributes.RPC_METHOD" "hello"
|
||||
"$SemanticAttributes.NET_SOCK_PEER_ADDR" String
|
||||
"$SemanticAttributes.NET_SOCK_PEER_PORT" Long
|
||||
"$SemanticAttributes.NET_SOCK_HOST_PORT" Long
|
||||
"$SemanticAttributes.NET_SOCK_FAMILY" { it == SemanticAttributes.NetSockFamilyValues.INET6 || it == null }
|
||||
}
|
||||
}
|
||||
|
|
|
@ -8,7 +8,6 @@ package io.opentelemetry.javaagent.instrumentation.vertx.v3_0.client;
|
|||
import io.opentelemetry.instrumentation.api.instrumenter.net.NetClientAttributesGetter;
|
||||
import io.vertx.core.http.HttpClientRequest;
|
||||
import io.vertx.core.http.HttpClientResponse;
|
||||
import io.vertx.core.net.SocketAddress;
|
||||
import javax.annotation.Nullable;
|
||||
|
||||
enum Vertx3NetAttributesGetter
|
||||
|
@ -25,26 +24,4 @@ enum Vertx3NetAttributesGetter
|
|||
public Integer getServerPort(HttpClientRequest request) {
|
||||
return null;
|
||||
}
|
||||
|
||||
@Nullable
|
||||
@Override
|
||||
public String getServerSocketDomain(
|
||||
HttpClientRequest request, @Nullable HttpClientResponse response) {
|
||||
if (response == null) {
|
||||
return null;
|
||||
}
|
||||
SocketAddress socketAddress = response.netSocket().remoteAddress();
|
||||
return socketAddress == null ? null : socketAddress.host();
|
||||
}
|
||||
|
||||
@Nullable
|
||||
@Override
|
||||
public Integer getServerSocketPort(
|
||||
HttpClientRequest request, @Nullable HttpClientResponse response) {
|
||||
if (response == null) {
|
||||
return null;
|
||||
}
|
||||
SocketAddress socketAddress = response.netSocket().remoteAddress();
|
||||
return socketAddress == null ? null : socketAddress.port();
|
||||
}
|
||||
}
|
||||
|
|
|
@ -19,6 +19,7 @@ import io.vertx.core.http.HttpMethod
|
|||
import spock.lang.Shared
|
||||
|
||||
import java.util.concurrent.CompletableFuture
|
||||
import java.util.concurrent.TimeUnit
|
||||
|
||||
import static io.opentelemetry.api.common.AttributeKey.stringKey
|
||||
|
||||
|
@ -54,7 +55,7 @@ class VertxHttpClientTest extends HttpClientTest<HttpClientRequest> implements A
|
|||
@Override
|
||||
int sendRequest(HttpClientRequest request, String method, URI uri, Map<String, String> headers) {
|
||||
// Vertx doesn't seem to provide any synchronous API so bridge through a callback
|
||||
return sendRequest(request).get()
|
||||
return sendRequest(request).get(30, TimeUnit.SECONDS)
|
||||
}
|
||||
|
||||
@Override
|
||||
|
|
Loading…
Reference in New Issue