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

Factor out NetServerAttributesGetter #5194

Merged
merged 4 commits into from
Jan 22, 2022
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 @@ -10,7 +10,8 @@
import io.opentelemetry.instrumentation.api.instrumenter.http.CapturedHttpHeaders;
import io.opentelemetry.instrumentation.api.instrumenter.http.HttpClientAttributesExtractor;
import io.opentelemetry.instrumentation.api.instrumenter.http.HttpSpanNameExtractor;
import io.opentelemetry.instrumentation.api.instrumenter.net.InetSocketAddressNetServerAttributesExtractor;
import io.opentelemetry.instrumentation.api.instrumenter.net.InetSocketAddressNetServerAttributesGetter;
import io.opentelemetry.instrumentation.api.instrumenter.net.NetServerAttributesExtractor;
import io.opentelemetry.semconv.trace.attributes.SemanticAttributes;
import java.net.InetSocketAddress;
import java.util.Collections;
Expand Down Expand Up @@ -41,7 +42,8 @@ public class InstrumenterBenchmark {
"benchmark",
HttpSpanNameExtractor.create(ConstantHttpAttributesExtractor.INSTANCE))
.addAttributesExtractor(ConstantHttpAttributesExtractor.INSTANCE)
.addAttributesExtractor(new ConstantNetAttributesExtractor())
.addAttributesExtractor(
NetServerAttributesExtractor.create(new ConstantNetAttributesGetter()))
.newInstrumenter();

@Benchmark
Expand Down Expand Up @@ -126,8 +128,8 @@ protected List<String> responseHeader(Void unused, Void unused2, String name) {
}
}

static class ConstantNetAttributesExtractor
extends InetSocketAddressNetServerAttributesExtractor<Void, Void> {
static class ConstantNetAttributesGetter
extends InetSocketAddressNetServerAttributesGetter<Void> {

private static final InetSocketAddress ADDRESS =
InetSocketAddress.createUnresolved("localhost", 8080);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@
* {@link InetSocketAddress} so this is a convenient alternative to {@link
* NetServerAttributesExtractor}. There is no meaning to implement both in the same instrumentation.
*/
public abstract class InetSocketAddressNetServerAttributesExtractor<REQUEST, RESPONSE>
extends NetServerAttributesExtractor<REQUEST, RESPONSE> {
public abstract class InetSocketAddressNetServerAttributesGetter<REQUEST>
implements NetServerAttributesGetter<REQUEST> {

@Nullable
public abstract InetSocketAddress getAddress(REQUEST request);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,45 +14,44 @@
* Extractor of <a
* href="https:/open-telemetry/opentelemetry-specification/blob/main/specification/trace/semantic_conventions/span-general.md#general-network-connection-attributes">Network
* attributes</a>. It is common to have access to {@link java.net.InetSocketAddress}, in which case
* it is more convenient to use {@link InetSocketAddressNetServerAttributesExtractor}.
* it is more convenient to use {@link InetSocketAddressNetServerAttributesGetter}.
*/
public abstract class NetServerAttributesExtractor<REQUEST, RESPONSE>
public final class NetServerAttributesExtractor<REQUEST, RESPONSE>
implements AttributesExtractor<REQUEST, RESPONSE> {

private final NetServerAttributesGetter<REQUEST> getter;

public static <REQUEST, RESPONSE> NetServerAttributesExtractor<REQUEST, RESPONSE> create(
NetServerAttributesGetter<REQUEST> getter) {
return new NetServerAttributesExtractor<>(getter);
}

private NetServerAttributesExtractor(NetServerAttributesGetter<REQUEST> getter) {
this.getter = getter;
}

@Override
public final void onStart(AttributesBuilder attributes, REQUEST request) {
set(attributes, SemanticAttributes.NET_TRANSPORT, transport(request));
set(attributes, SemanticAttributes.NET_TRANSPORT, getter.transport(request));

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

if (peerName != null && !peerName.equals(peerIp)) {
set(attributes, SemanticAttributes.NET_PEER_NAME, peerName);
}
set(attributes, SemanticAttributes.NET_PEER_IP, peerIp);

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

@Override
public final void onEnd(
public void onEnd(
AttributesBuilder attributes,
REQUEST request,
@Nullable RESPONSE response,
@Nullable Throwable error) {}

@Nullable
public abstract String transport(REQUEST request);

@Nullable
public abstract String peerName(REQUEST request);

@Nullable
public abstract Integer peerPort(REQUEST request);

@Nullable
public abstract String peerIp(REQUEST request);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/

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

import javax.annotation.Nullable;

/**
* An interface for getting server-based network attributes. It adapts a vendor-specific request
* type into the 4 common attributes (transport, peerName, peerPort, peerIp).
*
* <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
* the various network attributes in a type-generic way.
*/
public interface NetServerAttributesGetter<REQUEST> {

@Nullable
String transport(REQUEST request);

@Nullable
String peerName(REQUEST request);

@Nullable
Integer peerPort(REQUEST request);

@Nullable
String peerIp(REQUEST request);
}
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import io.opentelemetry.instrumentation.api.instrumenter.messaging.MessageOperation;
import io.opentelemetry.instrumentation.api.instrumenter.messaging.MessagingAttributesExtractor;
import io.opentelemetry.instrumentation.api.instrumenter.net.NetServerAttributesExtractor;
import io.opentelemetry.instrumentation.api.instrumenter.net.NetServerAttributesGetter;
import io.opentelemetry.instrumentation.api.instrumenter.rpc.RpcAttributesExtractor;
import io.opentelemetry.instrumentation.api.internal.SpanKey;
import io.opentelemetry.sdk.common.InstrumentationLibraryInfo;
Expand Down Expand Up @@ -290,7 +291,7 @@ void server_http() {
otelTesting.getOpenTelemetry(), "test", unused -> "span")
.addAttributesExtractors(
mockHttpServerAttributes,
new ConstantNetPeerIpExtractor<>("2.2.2.2"),
NetServerAttributesExtractor.create(new ConstantNetPeerIpGetter<>("2.2.2.2")),
new AttributesExtractor1(),
new AttributesExtractor2())
.addSpanLinksExtractor(new LinksExtractor())
Expand Down Expand Up @@ -765,12 +766,12 @@ private static LinkData expectedSpanLink() {
LINK_TRACE_ID, LINK_SPAN_ID, TraceFlags.getSampled(), TraceState.getDefault()));
}

private static final class ConstantNetPeerIpExtractor<REQUEST, RESPONSE>
extends NetServerAttributesExtractor<REQUEST, RESPONSE> {
private static final class ConstantNetPeerIpGetter<REQUEST>
implements NetServerAttributesGetter<REQUEST> {

private final String peerIp;

private ConstantNetPeerIpExtractor(String peerIp) {
private ConstantNetPeerIpGetter(String peerIp) {
this.peerIp = peerIp;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,11 @@
import org.mockito.junit.jupiter.MockitoExtension;

@ExtendWith(MockitoExtension.class)
class InetSocketAddressNetServerAttributesExtractorTest {
class InetSocketAddressNetServerAttributesGetterTest {

private final InetSocketAddressNetServerAttributesExtractor<InetSocketAddress, InetSocketAddress>
extractor =
new InetSocketAddressNetServerAttributesExtractor<
InetSocketAddress, InetSocketAddress>() {
private final NetServerAttributesExtractor<InetSocketAddress, InetSocketAddress> extractor =
NetServerAttributesExtractor.create(
new InetSocketAddressNetServerAttributesGetter<InetSocketAddress>() {
@Override
public InetSocketAddress getAddress(InetSocketAddress request) {
return request;
Expand All @@ -32,7 +31,7 @@ public InetSocketAddress getAddress(InetSocketAddress request) {
public String transport(InetSocketAddress request) {
return SemanticAttributes.NetTransportValues.IP_TCP;
}
};
});

@Test
void noInetSocketAddress() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@

class NetServerAttributesExtractorTest {

static class TestNetServerAttributesExtractor
extends NetServerAttributesExtractor<Map<String, String>, Map<String, String>> {
static class TestNetServerAttributesGetter
implements NetServerAttributesGetter<Map<String, String>> {

@Override
public String transport(Map<String, String> request) {
Expand Down Expand Up @@ -55,7 +55,8 @@ void normal() {
response.put("peerPort", "42");
response.put("peerIp", "4.3.2.1");

TestNetServerAttributesExtractor extractor = new TestNetServerAttributesExtractor();
NetServerAttributesExtractor<Map<String, String>, Map<String, String>> extractor =
createTestExtractor();

// when
AttributesBuilder startAttributes = Attributes.builder();
Expand Down Expand Up @@ -89,7 +90,8 @@ public void doesNotSetDuplicateAttributes() {
response.put("peerPort", "42");
response.put("peerIp", "4.3.2.1");

TestNetServerAttributesExtractor extractor = new TestNetServerAttributesExtractor();
NetServerAttributesExtractor<Map<String, String>, Map<String, String>> extractor =
createTestExtractor();

// when
AttributesBuilder startAttributes = Attributes.builder();
Expand Down Expand Up @@ -117,7 +119,8 @@ public void doesNotSetNegativePort() {
Map<String, String> response = new HashMap<>();
response.put("peerPort", "-1");

TestNetServerAttributesExtractor extractor = new TestNetServerAttributesExtractor();
NetServerAttributesExtractor<Map<String, String>, Map<String, String>> extractor =
createTestExtractor();

// when
AttributesBuilder startAttributes = Attributes.builder();
Expand All @@ -130,4 +133,9 @@ public void doesNotSetNegativePort() {
assertThat(startAttributes.build()).isEmpty();
assertThat(endAttributes.build()).isEmpty();
}

private static NetServerAttributesExtractor<Map<String, String>, Map<String, String>>
createTestExtractor() {
return NetServerAttributesExtractor.create(new TestNetServerAttributesGetter());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,14 @@

import io.opentelemetry.api.OpenTelemetry;
import io.opentelemetry.instrumentation.apachedubbo.v2_7.internal.DubboNetClientAttributesGetter;
import io.opentelemetry.instrumentation.apachedubbo.v2_7.internal.DubboNetServerAttributesExtractor;
import io.opentelemetry.instrumentation.apachedubbo.v2_7.internal.DubboNetServerAttributesGetter;
import io.opentelemetry.instrumentation.api.instrumenter.AttributesExtractor;
import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter;
import io.opentelemetry.instrumentation.api.instrumenter.InstrumenterBuilder;
import io.opentelemetry.instrumentation.api.instrumenter.PeerServiceAttributesExtractor;
import io.opentelemetry.instrumentation.api.instrumenter.SpanNameExtractor;
import io.opentelemetry.instrumentation.api.instrumenter.net.NetClientAttributesExtractor;
import io.opentelemetry.instrumentation.api.instrumenter.net.NetServerAttributesExtractor;
import io.opentelemetry.instrumentation.api.instrumenter.rpc.RpcSpanNameExtractor;
import io.opentelemetry.semconv.trace.attributes.SemanticAttributes;
import java.util.ArrayList;
Expand Down Expand Up @@ -73,7 +74,8 @@ public DubboTracing build() {
.addAttributesExtractors(rpcAttributesExtractor)
.addAttributesExtractors(attributesExtractors));

serverInstrumenterBuilder.addAttributesExtractor(new DubboNetServerAttributesExtractor());
serverInstrumenterBuilder.addAttributesExtractor(
NetServerAttributesExtractor.create(new DubboNetServerAttributesGetter()));
clientInstrumenterBuilder.addAttributesExtractor(netClientAttributesExtractor);

if (peerService != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,12 @@
package io.opentelemetry.instrumentation.apachedubbo.v2_7.internal;

import io.opentelemetry.instrumentation.apachedubbo.v2_7.DubboRequest;
import io.opentelemetry.instrumentation.api.instrumenter.net.InetSocketAddressNetServerAttributesExtractor;
import io.opentelemetry.instrumentation.api.instrumenter.net.InetSocketAddressNetServerAttributesGetter;
import java.net.InetSocketAddress;
import javax.annotation.Nullable;
import org.apache.dubbo.rpc.Result;

public final class DubboNetServerAttributesExtractor
extends InetSocketAddressNetServerAttributesExtractor<DubboRequest, Result> {
public final class DubboNetServerAttributesGetter
extends InetSocketAddressNetServerAttributesGetter<DubboRequest> {

@Override
@Nullable
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,14 @@
package io.opentelemetry.instrumentation.armeria.v1_3;

import com.linecorp.armeria.common.RequestContext;
import com.linecorp.armeria.common.logging.RequestLog;
import io.opentelemetry.instrumentation.api.instrumenter.net.InetSocketAddressNetServerAttributesExtractor;
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 ArmeriaNetServerAttributesExtractor
extends InetSocketAddressNetServerAttributesExtractor<RequestContext, RequestLog> {
final class ArmeriaNetServerAttributesGetter
extends InetSocketAddressNetServerAttributesGetter<RequestContext> {

@Override
public String transport(RequestContext ctx) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import io.opentelemetry.instrumentation.api.instrumenter.http.HttpSpanNameExtractor;
import io.opentelemetry.instrumentation.api.instrumenter.http.HttpSpanStatusExtractor;
import io.opentelemetry.instrumentation.api.instrumenter.net.NetClientAttributesExtractor;
import io.opentelemetry.instrumentation.api.instrumenter.net.NetServerAttributesExtractor;
import io.opentelemetry.instrumentation.api.server.ServerSpanNaming;
import io.opentelemetry.semconv.trace.attributes.SemanticAttributes;
import java.util.ArrayList;
Expand Down Expand Up @@ -137,7 +138,8 @@ public ArmeriaTracing build() {
.setSpanStatusExtractor(
statusExtractorTransformer.apply(
HttpSpanStatusExtractor.create(serverAttributesExtractor)))
.addAttributesExtractor(new ArmeriaNetServerAttributesExtractor())
.addAttributesExtractor(
NetServerAttributesExtractor.create(new ArmeriaNetServerAttributesGetter()))
.addAttributesExtractor(serverAttributesExtractor)
.addRequestMetrics(HttpServerMetrics.get())
.addContextCustomizer(ServerSpanNaming.get());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,11 @@

package io.opentelemetry.javaagent.instrumentation.grizzly;

import io.opentelemetry.instrumentation.api.instrumenter.net.NetServerAttributesExtractor;
import io.opentelemetry.instrumentation.api.instrumenter.net.NetServerAttributesGetter;
import javax.annotation.Nullable;
import org.glassfish.grizzly.http.HttpRequestPacket;
import org.glassfish.grizzly.http.HttpResponsePacket;

final class GrizzlyNetAttributesExtractor
extends NetServerAttributesExtractor<HttpRequestPacket, HttpResponsePacket> {
final class GrizzlyNetAttributesGetter implements NetServerAttributesGetter<HttpRequestPacket> {

@Nullable
@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import io.opentelemetry.instrumentation.api.instrumenter.http.HttpServerMetrics;
import io.opentelemetry.instrumentation.api.instrumenter.http.HttpSpanNameExtractor;
import io.opentelemetry.instrumentation.api.instrumenter.http.HttpSpanStatusExtractor;
import io.opentelemetry.instrumentation.api.instrumenter.net.NetServerAttributesExtractor;
import io.opentelemetry.instrumentation.api.server.ServerSpanNaming;
import org.glassfish.grizzly.http.HttpRequestPacket;
import org.glassfish.grizzly.http.HttpResponsePacket;
Expand All @@ -20,7 +21,7 @@ public final class GrizzlySingletons {

static {
GrizzlyHttpAttributesExtractor httpAttributesExtractor = new GrizzlyHttpAttributesExtractor();
GrizzlyNetAttributesExtractor netAttributesExtractor = new GrizzlyNetAttributesExtractor();
GrizzlyNetAttributesGetter netAttributesGetter = new GrizzlyNetAttributesGetter();

INSTRUMENTER =
Instrumenter.<HttpRequestPacket, HttpResponsePacket>builder(
Expand All @@ -29,7 +30,7 @@ public final class GrizzlySingletons {
HttpSpanNameExtractor.create(httpAttributesExtractor))
.setSpanStatusExtractor(HttpSpanStatusExtractor.create(httpAttributesExtractor))
.addAttributesExtractor(httpAttributesExtractor)
.addAttributesExtractor(netAttributesExtractor)
.addAttributesExtractor(NetServerAttributesExtractor.create(netAttributesGetter))
.addRequestMetrics(HttpServerMetrics.get())
.addContextCustomizer(
(context, httpRequestPacket, startAttributes) -> GrizzlyErrorHolder.init(context))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,9 @@
import io.opentelemetry.instrumentation.api.instrumenter.PeerServiceAttributesExtractor;
import io.opentelemetry.instrumentation.api.instrumenter.SpanKindExtractor;
import io.opentelemetry.instrumentation.api.instrumenter.net.NetClientAttributesExtractor;
import io.opentelemetry.instrumentation.api.instrumenter.net.NetServerAttributesExtractor;
import io.opentelemetry.instrumentation.grpc.v1_6.internal.GrpcNetClientAttributesGetter;
import io.opentelemetry.instrumentation.grpc.v1_6.internal.GrpcNetServerAttributesExtractor;
import io.opentelemetry.instrumentation.grpc.v1_6.internal.GrpcNetServerAttributesGetter;
import io.opentelemetry.semconv.trace.attributes.SemanticAttributes;
import java.util.ArrayList;
import java.util.List;
Expand Down Expand Up @@ -85,7 +86,8 @@ public GrpcTracing build() {

clientInstrumenterBuilder.addAttributesExtractor(
NetClientAttributesExtractor.create(netClientAttributesExtractor));
serverInstrumenterBuilder.addAttributesExtractor(new GrpcNetServerAttributesExtractor());
serverInstrumenterBuilder.addAttributesExtractor(
NetServerAttributesExtractor.create(new GrpcNetServerAttributesGetter()));

if (peerService != null) {
clientInstrumenterBuilder.addAttributesExtractor(
Expand Down
Loading