open-telemetry
/
opentelemetry-java-instrumentation
mirror of https://github.com/open-telemetry/opentelemetry-java-instrumentation.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Follow http.client_ip spec clarification (#4063)

This commit is contained in:
Trask Stalnaker 2021-09-08 07:54:59 -07:00 committed by GitHub
parent 1a994b9845
commit 400d99467a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
12 changed files with 7 additions and 95 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

22
instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/ServerInstrumenter.java
View File

@ -10,7 +10,6 @@ import io.opentelemetry.context.Context;
import io.opentelemetry.context.propagation.ContextPropagators;
import io.opentelemetry.context.propagation.TextMapGetter;
import io.opentelemetry.instrumentation.api.instrumenter.http.HttpAttributesExtractor;
import io.opentelemetry.instrumentation.api.instrumenter.net.NetAttributesExtractor;
import io.opentelemetry.instrumentation.api.internal.ContextPropagationDebug;
import io.opentelemetry.semconv.trace.attributes.SemanticAttributes;
import org.checkerframework.checker.nullness.qual.Nullable;
@ -38,12 +37,9 @@ final class ServerInstrumenter<REQUEST, RESPONSE> extends Instrumenter<REQUEST,
private static <REQUEST, RESPONSE> InstrumenterBuilder<REQUEST, RESPONSE> addClientIpExtractor(
InstrumenterBuilder<REQUEST, RESPONSE> builder, TextMapGetter<REQUEST> getter) {
HttpAttributesExtractor<REQUEST, RESPONSE> httpAttributesExtractor = null;
NetAttributesExtractor<REQUEST, RESPONSE> netAttributesExtractor = null;
for (AttributesExtractor<? super REQUEST, ? super RESPONSE> extractor :
builder.attributesExtractors) {
if (extractor instanceof NetAttributesExtractor) {
netAttributesExtractor = (NetAttributesExtractor<REQUEST, RESPONSE>) extractor;
} else if (extractor instanceof HttpAttributesExtractor) {
if (extractor instanceof HttpAttributesExtractor) {
httpAttributesExtractor = (HttpAttributesExtractor<REQUEST, RESPONSE>) extractor;
}
}
@ -51,7 +47,7 @@ final class ServerInstrumenter<REQUEST, RESPONSE> extends Instrumenter<REQUEST,
// Don't add HTTP_CLIENT_IP if there are no HTTP attributes registered.
return builder;
}
builder.addAttributesExtractor(new HttpClientIpExtractor(getter, netAttributesExtractor));
builder.addAttributesExtractor(new HttpClientIpExtractor<>(getter));
return builder;
}
@ -59,13 +55,9 @@ final class ServerInstrumenter<REQUEST, RESPONSE> extends Instrumenter<REQUEST,
extends AttributesExtractor<REQUEST, RESPONSE> {
private final TextMapGetter<REQUEST> getter;
@Nullable private final NetAttributesExtractor<REQUEST, RESPONSE> netAttributesExtractor;
HttpClientIpExtractor(
TextMapGetter<REQUEST> getter,
@Nullable NetAttributesExtractor<REQUEST, RESPONSE> netAttributesExtractor) {
HttpClientIpExtractor(TextMapGetter<REQUEST> getter) {
this.getter = getter;
this.netAttributesExtractor = netAttributesExtractor;
}
@Override
@ -75,9 +67,6 @@ final class ServerInstrumenter<REQUEST, RESPONSE> extends Instrumenter<REQUEST,
protected void onEnd(
AttributesBuilder attributes, REQUEST request, @Nullable RESPONSE response) {
String clientIp = getForwardedClientIp(request);
if (clientIp == null && netAttributesExtractor != null) {
clientIp = netAttributesExtractor.peerIp(request, response);
}
set(attributes, SemanticAttributes.HTTP_CLIENT_IP, clientIp);
}
@ -96,10 +85,7 @@ final class ServerInstrumenter<REQUEST, RESPONSE> extends Instrumenter<REQUEST,
// try X-Forwarded-For
forwarded = getter.get(request, "X-Forwarded-For");
if (forwarded != null) {
forwarded = extractForwardedFor(forwarded);
if (forwarded != null) {
return forwarded;
}
return extractForwardedFor(forwarded);
}
return null;

7
instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/HttpServerTracer.java
View File

@ -199,10 +199,10 @@ public abstract class HttpServerTracer<REQUEST, RESPONSE, CONNECTION, STORAGE> e
}
spanBuilder.setAttribute(SemanticAttributes.HTTP_FLAVOR, flavor);
}
spanBuilder.setAttribute(SemanticAttributes.HTTP_CLIENT_IP, clientIp(connection, request));
spanBuilder.setAttribute(SemanticAttributes.HTTP_CLIENT_IP, clientIp(request));
}
private String clientIp(CONNECTION connection, REQUEST request) {
private String clientIp(REQUEST request) {
// try Forwarded
String forwarded = requestHeader(request, "Forwarded");
if (forwarded != null) {
@ -221,8 +221,7 @@ public abstract class HttpServerTracer<REQUEST, RESPONSE, CONNECTION, STORAGE> e
}
}
// fallback to peer IP if there are no proxy headers
return peerHostIp(connection);
return null;
}
// VisibleForTesting

42
instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/InstrumenterTest.java
View File

@ -333,48 +333,6 @@ class InstrumenterTest {
SemanticAttributes.HTTP_CLIENT_IP, "1.1.1.1"))));
}
@Test
void server_http_noForwarded() {
Instrumenter<Map<String, String>, Map<String, String>> instrumenter =
Instrumenter.<Map<String, String>, Map<String, String>>newBuilder(
otelTesting.getOpenTelemetry(), "test", unused -> "span")
.addAttributesExtractors(
mockHttpAttributes,
mockNetAttributes,
new AttributesExtractor1(),
new AttributesExtractor2())
.addSpanLinksExtractor(new LinksExtractor())
.newServerInstrumenter(new MapGetter());
Map<String, String> request = new HashMap<>(REQUEST);
request.remove("Forwarded");
when(mockNetAttributes.peerIp(request, null)).thenReturn("2.2.2.2");
when(mockNetAttributes.peerIp(request, RESPONSE)).thenReturn("2.2.2.2");
Context context = instrumenter.start(Context.root(), request);
SpanContext spanContext = Span.fromContext(context).getSpanContext();
assertThat(spanContext.isValid()).isTrue();
assertThat(SpanKey.SERVER.fromContextOrNull(context).getSpanContext()).isEqualTo(spanContext);
instrumenter.end(context, request, RESPONSE, null);
otelTesting
.assertTraces()
.hasTracesSatisfyingExactly(
trace ->
trace.hasSpansSatisfyingExactly(
span ->
span.hasName("span")
.hasAttributesSatisfying(
attributes ->
assertThat(attributes)
.containsEntry(SemanticAttributes.NET_PEER_IP, "2.2.2.2")
.containsEntry(
SemanticAttributes.HTTP_CLIENT_IP, "2.2.2.2"))));
}
@Test
void client() {
Instrumenter<Map<String, String>, Map<String, String>> instrumenter =

1
instrumentation/apache-camel-2.20/javaagent/src/test/groovy/io/opentelemetry/javaagent/instrumentation/apachecamel/RestCamelTest.groovy
View File

@ -89,7 +89,6 @@ class RestCamelTest extends AgentInstrumentationSpecification implements RetryOn
attributes {
"$SemanticAttributes.HTTP_URL.key" "http://localhost:$port/api/firstModule/unit/unitOne"
"$SemanticAttributes.HTTP_STATUS_CODE.key" 200
"$SemanticAttributes.HTTP_CLIENT_IP.key" "127.0.0.1"
"$SemanticAttributes.HTTP_USER_AGENT.key" String
"$SemanticAttributes.HTTP_FLAVOR.key" "1.1"
"$SemanticAttributes.HTTP_METHOD.key" "GET"

1
instrumentation/apache-camel-2.20/javaagent/src/test/groovy/io/opentelemetry/javaagent/instrumentation/apachecamel/TwoServicesWithDirectClientCamelTest.groovy
View File

@ -129,7 +129,6 @@ class TwoServicesWithDirectClientCamelTest extends AgentInstrumentationSpecifica
"$SemanticAttributes.NET_PEER_IP.key" "127.0.0.1"
"$SemanticAttributes.HTTP_USER_AGENT.key" "Jakarta Commons-HttpClient/3.1"
"$SemanticAttributes.HTTP_FLAVOR.key" "1.1"
"$SemanticAttributes.HTTP_CLIENT_IP.key" "127.0.0.1"
}
}
it.span(5) {

8
instrumentation/jsp-2.3/javaagent/src/test/groovy/JspInstrumentationBasicTests.groovy
View File

@ -93,7 +93,6 @@ class JspInstrumentationBasicTests extends AgentInstrumentationSpecification {
"${SemanticAttributes.HTTP_STATUS_CODE.key}" 200
"${SemanticAttributes.HTTP_FLAVOR.key}" "1.1"
"${SemanticAttributes.HTTP_USER_AGENT.key}" String
"${SemanticAttributes.HTTP_CLIENT_IP.key}" "127.0.0.1"
}
}
span(1) {
@ -144,7 +143,6 @@ class JspInstrumentationBasicTests extends AgentInstrumentationSpecification {
"${SemanticAttributes.HTTP_STATUS_CODE.key}" 200
"${SemanticAttributes.HTTP_FLAVOR.key}" "1.1"
"${SemanticAttributes.HTTP_USER_AGENT.key}" String
"${SemanticAttributes.HTTP_CLIENT_IP.key}" "127.0.0.1"
}
}
span(1) {
@ -191,7 +189,6 @@ class JspInstrumentationBasicTests extends AgentInstrumentationSpecification {
"${SemanticAttributes.HTTP_STATUS_CODE.key}" 200
"${SemanticAttributes.HTTP_FLAVOR.key}" "1.1"
"${SemanticAttributes.HTTP_USER_AGENT.key}" String
"${SemanticAttributes.HTTP_CLIENT_IP.key}" "127.0.0.1"
}
}
span(1) {
@ -247,7 +244,6 @@ class JspInstrumentationBasicTests extends AgentInstrumentationSpecification {
"${SemanticAttributes.HTTP_STATUS_CODE.key}" 500
"${SemanticAttributes.HTTP_FLAVOR.key}" "1.1"
"${SemanticAttributes.HTTP_USER_AGENT.key}" String
"${SemanticAttributes.HTTP_CLIENT_IP.key}" "127.0.0.1"
}
}
span(1) {
@ -308,7 +304,6 @@ class JspInstrumentationBasicTests extends AgentInstrumentationSpecification {
"${SemanticAttributes.HTTP_STATUS_CODE.key}" 200
"${SemanticAttributes.HTTP_FLAVOR.key}" "1.1"
"${SemanticAttributes.HTTP_USER_AGENT.key}" String
"${SemanticAttributes.HTTP_CLIENT_IP.key}" "127.0.0.1"
}
}
span(1) {
@ -350,7 +345,6 @@ class JspInstrumentationBasicTests extends AgentInstrumentationSpecification {
"${SemanticAttributes.HTTP_STATUS_CODE.key}" 200
"${SemanticAttributes.HTTP_FLAVOR.key}" "1.1"
"${SemanticAttributes.HTTP_USER_AGENT.key}" String
"${SemanticAttributes.HTTP_CLIENT_IP.key}" "127.0.0.1"
}
}
span(1) {
@ -424,7 +418,6 @@ class JspInstrumentationBasicTests extends AgentInstrumentationSpecification {
"${SemanticAttributes.HTTP_STATUS_CODE.key}" 500
"${SemanticAttributes.HTTP_FLAVOR.key}" "1.1"
"${SemanticAttributes.HTTP_USER_AGENT.key}" String
"${SemanticAttributes.HTTP_CLIENT_IP.key}" "127.0.0.1"
}
}
span(1) {
@ -467,7 +460,6 @@ class JspInstrumentationBasicTests extends AgentInstrumentationSpecification {
"${SemanticAttributes.HTTP_STATUS_CODE.key}" 200
"${SemanticAttributes.HTTP_FLAVOR.key}" "1.1"
"${SemanticAttributes.HTTP_USER_AGENT.key}" String
"${SemanticAttributes.HTTP_CLIENT_IP.key}" "127.0.0.1"
}
}
}

6
instrumentation/jsp-2.3/javaagent/src/test/groovy/JspInstrumentationForwardTests.groovy
View File

@ -90,7 +90,6 @@ class JspInstrumentationForwardTests extends AgentInstrumentationSpecification {
"${SemanticAttributes.HTTP_STATUS_CODE.key}" 200
"${SemanticAttributes.HTTP_FLAVOR.key}" "1.1"
"${SemanticAttributes.HTTP_USER_AGENT.key}" String
"${SemanticAttributes.HTTP_CLIENT_IP.key}" "127.0.0.1"
}
}
span(1) {
@ -153,7 +152,6 @@ class JspInstrumentationForwardTests extends AgentInstrumentationSpecification {
"${SemanticAttributes.HTTP_STATUS_CODE.key}" 200
"${SemanticAttributes.HTTP_FLAVOR.key}" "1.1"
"${SemanticAttributes.HTTP_USER_AGENT.key}" String
"${SemanticAttributes.HTTP_CLIENT_IP.key}" "127.0.0.1"
}
}
span(1) {
@ -195,7 +193,6 @@ class JspInstrumentationForwardTests extends AgentInstrumentationSpecification {
"${SemanticAttributes.HTTP_STATUS_CODE.key}" 200
"${SemanticAttributes.HTTP_FLAVOR.key}" "1.1"
"${SemanticAttributes.HTTP_USER_AGENT.key}" String
"${SemanticAttributes.HTTP_CLIENT_IP.key}" "127.0.0.1"
}
}
span(1) {
@ -285,7 +282,6 @@ class JspInstrumentationForwardTests extends AgentInstrumentationSpecification {
"${SemanticAttributes.HTTP_STATUS_CODE.key}" 200
"${SemanticAttributes.HTTP_FLAVOR.key}" "1.1"
"${SemanticAttributes.HTTP_USER_AGENT.key}" String
"${SemanticAttributes.HTTP_CLIENT_IP.key}" "127.0.0.1"
}
}
span(1) {
@ -361,7 +357,6 @@ class JspInstrumentationForwardTests extends AgentInstrumentationSpecification {
"${SemanticAttributes.HTTP_STATUS_CODE.key}" 500
"${SemanticAttributes.HTTP_FLAVOR.key}" "1.1"
"${SemanticAttributes.HTTP_USER_AGENT.key}" String
"${SemanticAttributes.HTTP_CLIENT_IP.key}" "127.0.0.1"
}
}
span(1) {
@ -416,7 +411,6 @@ class JspInstrumentationForwardTests extends AgentInstrumentationSpecification {
"${SemanticAttributes.HTTP_STATUS_CODE.key}" 404
"${SemanticAttributes.HTTP_FLAVOR.key}" "1.1"
"${SemanticAttributes.HTTP_USER_AGENT.key}" String
"${SemanticAttributes.HTTP_CLIENT_IP.key}" "127.0.0.1"
}
}
span(1) {

1
instrumentation/ratpack-1.4/testing/src/main/groovy/io/opentelemetry/instrumentation/ratpack/server/AbstractRatpackRoutesTest.groovy
View File

@ -109,7 +109,6 @@ abstract class AbstractRatpackRoutesTest extends InstrumentationSpecification {
"${SemanticAttributes.HTTP_STATUS_CODE.key}" 200
"${SemanticAttributes.HTTP_FLAVOR.key}" "1.1"
"${SemanticAttributes.HTTP_USER_AGENT.key}" String
"${SemanticAttributes.HTTP_CLIENT_IP.key}" { it == null || it == "127.0.0.1" }
if (extraAttributes.contains(SemanticAttributes.HTTP_HOST)) {
"${SemanticAttributes.HTTP_HOST}" "localhost:${app.bindPort}"

1
instrumentation/spark-2.3/javaagent/src/test/groovy/SparkJavaBasedTest.groovy
View File

@ -53,7 +53,6 @@ class SparkJavaBasedTest extends AgentInstrumentationSpecification {
"${SemanticAttributes.HTTP_STATUS_CODE.key}" 200
"${SemanticAttributes.HTTP_FLAVOR.key}" "1.1"
"${SemanticAttributes.HTTP_USER_AGENT.key}" String
"${SemanticAttributes.HTTP_CLIENT_IP.key}" "127.0.0.1"
}
}
}

9
instrumentation/spring/spring-webflux-5.0/javaagent/src/test/groovy/SpringWebfluxTest.groovy
View File

@ -90,7 +90,6 @@ class SpringWebfluxTest extends AgentInstrumentationSpecification {
"${SemanticAttributes.HTTP_STATUS_CODE.key}" 200
"${SemanticAttributes.HTTP_FLAVOR.key}" "1.1"
"${SemanticAttributes.HTTP_USER_AGENT.key}" String
"${SemanticAttributes.HTTP_CLIENT_IP.key}" "127.0.0.1"
}
}
span(1) {
@ -153,7 +152,6 @@ class SpringWebfluxTest extends AgentInstrumentationSpecification {
"${SemanticAttributes.HTTP_STATUS_CODE.key}" 200
"${SemanticAttributes.HTTP_FLAVOR.key}" "1.1"
"${SemanticAttributes.HTTP_USER_AGENT.key}" String
"${SemanticAttributes.HTTP_CLIENT_IP.key}" "127.0.0.1"
}
}
span(1) {
@ -235,7 +233,6 @@ class SpringWebfluxTest extends AgentInstrumentationSpecification {
"${SemanticAttributes.HTTP_STATUS_CODE.key}" 200
"${SemanticAttributes.HTTP_FLAVOR.key}" "1.1"
"${SemanticAttributes.HTTP_USER_AGENT.key}" String
"${SemanticAttributes.HTTP_CLIENT_IP.key}" "127.0.0.1"
}
}
span(1) {
@ -296,7 +293,6 @@ class SpringWebfluxTest extends AgentInstrumentationSpecification {
"${SemanticAttributes.HTTP_STATUS_CODE.key}" 404
"${SemanticAttributes.HTTP_FLAVOR.key}" "1.1"
"${SemanticAttributes.HTTP_USER_AGENT.key}" String
"${SemanticAttributes.HTTP_CLIENT_IP.key}" "127.0.0.1"
}
}
span(1) {
@ -336,7 +332,6 @@ class SpringWebfluxTest extends AgentInstrumentationSpecification {
"${SemanticAttributes.HTTP_STATUS_CODE.key}" 202
"${SemanticAttributes.HTTP_FLAVOR.key}" "1.1"
"${SemanticAttributes.HTTP_USER_AGENT.key}" String
"${SemanticAttributes.HTTP_CLIENT_IP.key}" "127.0.0.1"
}
}
span(1) {
@ -381,7 +376,6 @@ class SpringWebfluxTest extends AgentInstrumentationSpecification {
"${SemanticAttributes.HTTP_STATUS_CODE.key}" 500
"${SemanticAttributes.HTTP_FLAVOR.key}" "1.1"
"${SemanticAttributes.HTTP_USER_AGENT.key}" String
"${SemanticAttributes.HTTP_CLIENT_IP.key}" "127.0.0.1"
}
}
span(1) {
@ -444,7 +438,6 @@ class SpringWebfluxTest extends AgentInstrumentationSpecification {
"${SemanticAttributes.HTTP_STATUS_CODE.key}" 307
"${SemanticAttributes.HTTP_FLAVOR.key}" "1.1"
"${SemanticAttributes.HTTP_USER_AGENT.key}" String
"${SemanticAttributes.HTTP_CLIENT_IP.key}" "127.0.0.1"
}
}
span(1) {
@ -472,7 +465,6 @@ class SpringWebfluxTest extends AgentInstrumentationSpecification {
"${SemanticAttributes.HTTP_STATUS_CODE.key}" 200
"${SemanticAttributes.HTTP_FLAVOR.key}" "1.1"
"${SemanticAttributes.HTTP_USER_AGENT.key}" String
"${SemanticAttributes.HTTP_CLIENT_IP.key}" "127.0.0.1"
}
}
span(1) {
@ -515,7 +507,6 @@ class SpringWebfluxTest extends AgentInstrumentationSpecification {
"${SemanticAttributes.HTTP_STATUS_CODE.key}" 200
"${SemanticAttributes.HTTP_FLAVOR.key}" "1.1"
"${SemanticAttributes.HTTP_USER_AGENT.key}" String
"${SemanticAttributes.HTTP_CLIENT_IP.key}" "127.0.0.1"
}
}
span(1) {

2
instrumentation/vertx-reactive-3.5/javaagent/src/latestDepTest/groovy/VertxReactivePropagationTest.groovy
View File

@ -68,7 +68,6 @@ class VertxReactivePropagationTest extends AgentInstrumentationSpecification {
"${SemanticAttributes.HTTP_STATUS_CODE.key}" 200
"${SemanticAttributes.HTTP_FLAVOR.key}" "1.1"
"${SemanticAttributes.HTTP_USER_AGENT.key}" String
"${SemanticAttributes.HTTP_CLIENT_IP.key}" "127.0.0.1"
}
}
span(1) {
@ -155,7 +154,6 @@ class VertxReactivePropagationTest extends AgentInstrumentationSpecification {
"${SemanticAttributes.HTTP_STATUS_CODE.key}" 200
"${SemanticAttributes.HTTP_FLAVOR.key}" "1.1"
"${SemanticAttributes.HTTP_USER_AGENT.key}" String
"${SemanticAttributes.HTTP_CLIENT_IP.key}" "127.0.0.1"
"${TEST_REQUEST_ID_ATTRIBUTE}" requestId
}
}

2
instrumentation/vertx-reactive-3.5/javaagent/src/version35Test/groovy/VertxReactivePropagationTest.groovy
View File

@ -68,7 +68,6 @@ class VertxReactivePropagationTest extends AgentInstrumentationSpecification {
"${SemanticAttributes.HTTP_STATUS_CODE.key}" 200
"${SemanticAttributes.HTTP_FLAVOR.key}" "1.1"
"${SemanticAttributes.HTTP_USER_AGENT.key}" String
"${SemanticAttributes.HTTP_CLIENT_IP.key}" "127.0.0.1"
}
}
span(1) {
@ -155,7 +154,6 @@ class VertxReactivePropagationTest extends AgentInstrumentationSpecification {
"${SemanticAttributes.HTTP_STATUS_CODE.key}" 200
"${SemanticAttributes.HTTP_FLAVOR.key}" "1.1"
"${SemanticAttributes.HTTP_USER_AGENT.key}" String
"${SemanticAttributes.HTTP_CLIENT_IP.key}" "127.0.0.1"
"${TEST_REQUEST_ID_ATTRIBUTE}" requestId
}
}