Skip to content

Commit

Permalink
Use direct peer address in client.address when X-Forwarded-For is n…
Browse files Browse the repository at this point in the history
…ot present (#10370)

Co-authored-by: heyams <heya@microsoft.com>
  • Loading branch information
trask and heyams committed Feb 6, 2024
1 parent 5b1ef39 commit d4435c9
Show file tree
Hide file tree
Showing 13 changed files with 49 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,11 @@
import io.opentelemetry.instrumentation.api.semconv.network.internal.AddressAndPortExtractor;
import java.util.Locale;

final class ForwardedForAddressAndPortExtractor<REQUEST>
implements AddressAndPortExtractor<REQUEST> {
final class HttpServerAddressAndPortExtractor<REQUEST> implements AddressAndPortExtractor<REQUEST> {

private final HttpServerAttributesGetter<REQUEST, ?> getter;

ForwardedForAddressAndPortExtractor(HttpServerAttributesGetter<REQUEST, ?> getter) {
HttpServerAddressAndPortExtractor(HttpServerAttributesGetter<REQUEST, ?> getter) {
this.getter = getter;
}

Expand All @@ -34,6 +33,13 @@ public void extract(AddressPortSink sink, REQUEST request) {
return;
}
}

// use network.peer.address and network.peer.port
sink.setAddress(getter.getNetworkPeerAddress(request, null));
Integer port = getter.getNetworkPeerPort(request, null);
if (port != null && port > 0) {
sink.setPort(port);
}
}

private static boolean extractFromForwardedHeader(AddressPortSink sink, String forwarded) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ public final class HttpServerAttributesExtractorBuilder<REQUEST, RESPONSE> {

clientAddressPortExtractor =
new ClientAddressAndPortExtractor<>(
httpAttributesGetter, new ForwardedForAddressAndPortExtractor<>(httpAttributesGetter));
httpAttributesGetter, new HttpServerAddressAndPortExtractor<>(httpAttributesGetter));
serverAddressPortExtractor = new ForwardedHostAddressAndPortExtractor<>(httpAttributesGetter);
}

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

@ExtendWith(MockitoExtension.class)
class ForwardedForAddressAndPortExtractorTest {
class HttpServerAddressAndPortExtractorTest {

@Mock HttpServerAttributesGetter<String, String> getter;

@InjectMocks ForwardedForAddressAndPortExtractor<String> underTest;
@InjectMocks HttpServerAddressAndPortExtractor<String> underTest;

@ParameterizedTest
@ArgumentsSource(ForwardedArgs.class)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ void restComponentServerAndClientCallWithJettyBackend() {
equalTo(SemanticAttributes.NETWORK_PROTOCOL_VERSION, "1.1"),
equalTo(SemanticAttributes.SERVER_ADDRESS, "localhost"),
equalTo(SemanticAttributes.SERVER_PORT, Long.valueOf(port)),
equalTo(SemanticAttributes.CLIENT_ADDRESS, "127.0.0.1"),
equalTo(NetworkAttributes.NETWORK_PEER_ADDRESS, "127.0.0.1"),
satisfies(
SemanticAttributes.USER_AGENT_ORIGINAL,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,7 @@ void twoCamelServiceSpans() throws Exception {
equalTo(SemanticAttributes.NETWORK_PROTOCOL_VERSION, "1.1"),
equalTo(SemanticAttributes.SERVER_ADDRESS, "127.0.0.1"),
equalTo(SemanticAttributes.SERVER_PORT, portTwo),
equalTo(SemanticAttributes.CLIENT_ADDRESS, "127.0.0.1"),
equalTo(NetworkAttributes.NETWORK_PEER_ADDRESS, "127.0.0.1"),
satisfies(
NetworkAttributes.NETWORK_PEER_PORT,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ class JspInstrumentationBasicTests extends AgentInstrumentationSpecification {
"$SemanticAttributes.NETWORK_PROTOCOL_VERSION" "1.1"
"$SemanticAttributes.SERVER_ADDRESS" "localhost"
"$SemanticAttributes.SERVER_PORT" port
"$SemanticAttributes.CLIENT_ADDRESS" "127.0.0.1"
"$NetworkAttributes.NETWORK_PEER_ADDRESS" "127.0.0.1"
"$NetworkAttributes.NETWORK_PEER_PORT" Long
}
Expand Down Expand Up @@ -156,6 +157,7 @@ class JspInstrumentationBasicTests extends AgentInstrumentationSpecification {
"$SemanticAttributes.NETWORK_PROTOCOL_VERSION" "1.1"
"$SemanticAttributes.SERVER_ADDRESS" "localhost"
"$SemanticAttributes.SERVER_PORT" port
"$SemanticAttributes.CLIENT_ADDRESS" "127.0.0.1"
"$NetworkAttributes.NETWORK_PEER_ADDRESS" "127.0.0.1"
"$NetworkAttributes.NETWORK_PEER_PORT" Long
}
Expand Down Expand Up @@ -208,7 +210,9 @@ class JspInstrumentationBasicTests extends AgentInstrumentationSpecification {
"$SemanticAttributes.NETWORK_PROTOCOL_VERSION" "1.1"
"$SemanticAttributes.SERVER_ADDRESS" "localhost"
"$SemanticAttributes.SERVER_PORT" port
"$SemanticAttributes.CLIENT_ADDRESS" "127.0.0.1"
"$NetworkAttributes.NETWORK_PEER_ADDRESS" "127.0.0.1"

"$NetworkAttributes.NETWORK_PEER_PORT" Long
}
}
Expand Down Expand Up @@ -269,6 +273,7 @@ class JspInstrumentationBasicTests extends AgentInstrumentationSpecification {
"$SemanticAttributes.NETWORK_PROTOCOL_VERSION" "1.1"
"$SemanticAttributes.SERVER_ADDRESS" "localhost"
"$SemanticAttributes.SERVER_PORT" port
"$SemanticAttributes.CLIENT_ADDRESS" "127.0.0.1"
"$NetworkAttributes.NETWORK_PEER_ADDRESS" "127.0.0.1"
"$NetworkAttributes.NETWORK_PEER_PORT" Long
"$SemanticAttributes.ERROR_TYPE" "500"
Expand Down Expand Up @@ -336,6 +341,7 @@ class JspInstrumentationBasicTests extends AgentInstrumentationSpecification {
"$SemanticAttributes.NETWORK_PROTOCOL_VERSION" "1.1"
"$SemanticAttributes.SERVER_ADDRESS" "localhost"
"$SemanticAttributes.SERVER_PORT" port
"$SemanticAttributes.CLIENT_ADDRESS" "127.0.0.1"
"$NetworkAttributes.NETWORK_PEER_ADDRESS" "127.0.0.1"
"$NetworkAttributes.NETWORK_PEER_PORT" Long
}
Expand Down Expand Up @@ -383,6 +389,7 @@ class JspInstrumentationBasicTests extends AgentInstrumentationSpecification {
"$SemanticAttributes.NETWORK_PROTOCOL_VERSION" "1.1"
"$SemanticAttributes.SERVER_ADDRESS" "localhost"
"$SemanticAttributes.SERVER_PORT" port
"$SemanticAttributes.CLIENT_ADDRESS" "127.0.0.1"
"$NetworkAttributes.NETWORK_PEER_ADDRESS" "127.0.0.1"
"$NetworkAttributes.NETWORK_PEER_PORT" Long
}
Expand Down Expand Up @@ -462,6 +469,7 @@ class JspInstrumentationBasicTests extends AgentInstrumentationSpecification {
"$SemanticAttributes.NETWORK_PROTOCOL_VERSION" "1.1"
"$SemanticAttributes.SERVER_ADDRESS" "localhost"
"$SemanticAttributes.SERVER_PORT" port
"$SemanticAttributes.CLIENT_ADDRESS" "127.0.0.1"
"$NetworkAttributes.NETWORK_PEER_ADDRESS" "127.0.0.1"
"$NetworkAttributes.NETWORK_PEER_PORT" Long
"$SemanticAttributes.ERROR_TYPE" "500"
Expand Down Expand Up @@ -511,6 +519,7 @@ class JspInstrumentationBasicTests extends AgentInstrumentationSpecification {
"$SemanticAttributes.NETWORK_PROTOCOL_VERSION" "1.1"
"$SemanticAttributes.SERVER_ADDRESS" "localhost"
"$SemanticAttributes.SERVER_PORT" port
"$SemanticAttributes.CLIENT_ADDRESS" "127.0.0.1"
"$NetworkAttributes.NETWORK_PEER_ADDRESS" "127.0.0.1"
"$NetworkAttributes.NETWORK_PEER_PORT" Long
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ class JspInstrumentationForwardTests extends AgentInstrumentationSpecification {
"$SemanticAttributes.NETWORK_PROTOCOL_VERSION" "1.1"
"$SemanticAttributes.SERVER_ADDRESS" "localhost"
"$SemanticAttributes.SERVER_PORT" port
"$SemanticAttributes.CLIENT_ADDRESS" "127.0.0.1"
"$NetworkAttributes.NETWORK_PEER_ADDRESS" "127.0.0.1"
"$NetworkAttributes.NETWORK_PEER_PORT" Long
}
Expand Down Expand Up @@ -165,6 +166,7 @@ class JspInstrumentationForwardTests extends AgentInstrumentationSpecification {
"$SemanticAttributes.NETWORK_PROTOCOL_VERSION" "1.1"
"$SemanticAttributes.SERVER_ADDRESS" "localhost"
"$SemanticAttributes.SERVER_PORT" port
"$SemanticAttributes.CLIENT_ADDRESS" "127.0.0.1"
"$NetworkAttributes.NETWORK_PEER_ADDRESS" "127.0.0.1"
"$NetworkAttributes.NETWORK_PEER_PORT" Long
}
Expand Down Expand Up @@ -212,6 +214,7 @@ class JspInstrumentationForwardTests extends AgentInstrumentationSpecification {
"$SemanticAttributes.NETWORK_PROTOCOL_VERSION" "1.1"
"$SemanticAttributes.SERVER_ADDRESS" "localhost"
"$SemanticAttributes.SERVER_PORT" port
"$SemanticAttributes.CLIENT_ADDRESS" "127.0.0.1"
"$NetworkAttributes.NETWORK_PEER_ADDRESS" "127.0.0.1"
"$NetworkAttributes.NETWORK_PEER_PORT" Long
}
Expand Down Expand Up @@ -307,6 +310,7 @@ class JspInstrumentationForwardTests extends AgentInstrumentationSpecification {
"$SemanticAttributes.NETWORK_PROTOCOL_VERSION" "1.1"
"$SemanticAttributes.SERVER_ADDRESS" "localhost"
"$SemanticAttributes.SERVER_PORT" port
"$SemanticAttributes.CLIENT_ADDRESS" "127.0.0.1"
"$NetworkAttributes.NETWORK_PEER_ADDRESS" "127.0.0.1"
"$NetworkAttributes.NETWORK_PEER_PORT" Long
}
Expand Down Expand Up @@ -388,6 +392,7 @@ class JspInstrumentationForwardTests extends AgentInstrumentationSpecification {
"$SemanticAttributes.NETWORK_PROTOCOL_VERSION" "1.1"
"$SemanticAttributes.SERVER_ADDRESS" "localhost"
"$SemanticAttributes.SERVER_PORT" port
"$SemanticAttributes.CLIENT_ADDRESS" "127.0.0.1"
"$NetworkAttributes.NETWORK_PEER_ADDRESS" "127.0.0.1"
"$NetworkAttributes.NETWORK_PEER_PORT" Long
"$SemanticAttributes.ERROR_TYPE" "500"
Expand Down Expand Up @@ -449,6 +454,7 @@ class JspInstrumentationForwardTests extends AgentInstrumentationSpecification {
"$SemanticAttributes.NETWORK_PROTOCOL_VERSION" "1.1"
"$SemanticAttributes.SERVER_ADDRESS" "localhost"
"$SemanticAttributes.SERVER_PORT" port
"$SemanticAttributes.CLIENT_ADDRESS" "127.0.0.1"
"$NetworkAttributes.NETWORK_PEER_ADDRESS" "127.0.0.1"
"$NetworkAttributes.NETWORK_PEER_PORT" Long
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ abstract class AbstractRatpackRoutesTest extends InstrumentationSpecification {
"$SemanticAttributes.NETWORK_PROTOCOL_VERSION" "1.1"
"$SemanticAttributes.SERVER_ADDRESS" { it == "localhost" || it == null }
"$SemanticAttributes.SERVER_PORT" { it == app.bindPort || it == null }
"$SemanticAttributes.CLIENT_ADDRESS" { it == "127.0.0.1" || it == null }
"$NetworkAttributes.NETWORK_PEER_ADDRESS" { it == "127.0.0.1" || it == null }
"$NetworkAttributes.NETWORK_PEER_PORT" { it instanceof Long || it == null }
"$SemanticAttributes.HTTP_REQUEST_METHOD" "GET"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ void generatesSpans() {
equalTo(SemanticAttributes.NETWORK_PROTOCOL_VERSION, "1.1"),
equalTo(SemanticAttributes.SERVER_ADDRESS, "localhost"),
equalTo(SemanticAttributes.SERVER_PORT, port),
equalTo(SemanticAttributes.CLIENT_ADDRESS, "127.0.0.1"),
equalTo(NetworkAttributes.NETWORK_PEER_ADDRESS, "127.0.0.1"),
satisfies(
NetworkAttributes.NETWORK_PEER_PORT,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.assertThat;
import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.equalTo;
import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.satisfies;
import static io.opentelemetry.semconv.SemanticAttributes.CLIENT_ADDRESS;
import static io.opentelemetry.semconv.SemanticAttributes.ERROR_TYPE;
import static io.opentelemetry.semconv.SemanticAttributes.EXCEPTION_EVENT_NAME;
import static io.opentelemetry.semconv.SemanticAttributes.EXCEPTION_MESSAGE;
Expand Down Expand Up @@ -119,6 +120,7 @@ void basicGetTest(Parameter parameter) {
val -> val.isInstanceOf(Long.class)),
equalTo(SERVER_ADDRESS, "localhost"),
satisfies(SERVER_PORT, val -> val.isInstanceOf(Long.class)),
equalTo(CLIENT_ADDRESS, "127.0.0.1"),
equalTo(URL_PATH, parameter.urlPath),
equalTo(HTTP_REQUEST_METHOD, "GET"),
equalTo(HTTP_RESPONSE_STATUS_CODE, 200),
Expand Down Expand Up @@ -239,6 +241,7 @@ void getAsyncResponseTest(Parameter parameter) {
val -> val.isInstanceOf(Long.class)),
equalTo(SERVER_ADDRESS, "localhost"),
satisfies(SERVER_PORT, val -> val.isInstanceOf(Long.class)),
equalTo(CLIENT_ADDRESS, "127.0.0.1"),
equalTo(URL_PATH, parameter.urlPath),
equalTo(HTTP_REQUEST_METHOD, "GET"),
equalTo(HTTP_RESPONSE_STATUS_CODE, 200),
Expand Down Expand Up @@ -346,6 +349,7 @@ void createSpanDuringHandlerFunctionTest(Parameter parameter) {
val -> val.isInstanceOf(Long.class)),
equalTo(SERVER_ADDRESS, "localhost"),
satisfies(SERVER_PORT, val -> val.isInstanceOf(Long.class)),
equalTo(CLIENT_ADDRESS, "127.0.0.1"),
equalTo(URL_PATH, parameter.urlPath),
equalTo(HTTP_REQUEST_METHOD, "GET"),
equalTo(HTTP_RESPONSE_STATUS_CODE, 200),
Expand Down Expand Up @@ -418,6 +422,7 @@ void get404Test() {
val -> val.isInstanceOf(Long.class)),
equalTo(SERVER_ADDRESS, "localhost"),
satisfies(SERVER_PORT, val -> val.isInstanceOf(Long.class)),
equalTo(CLIENT_ADDRESS, "127.0.0.1"),
equalTo(URL_PATH, "/notfoundgreet"),
equalTo(HTTP_REQUEST_METHOD, "GET"),
equalTo(HTTP_RESPONSE_STATUS_CODE, 404),
Expand Down Expand Up @@ -478,6 +483,7 @@ void basicPostTest() {
val -> val.isInstanceOf(Long.class)),
equalTo(SERVER_ADDRESS, "localhost"),
satisfies(SERVER_PORT, val -> val.isInstanceOf(Long.class)),
equalTo(CLIENT_ADDRESS, "127.0.0.1"),
equalTo(URL_PATH, "/echo"),
equalTo(HTTP_REQUEST_METHOD, "POST"),
equalTo(HTTP_RESPONSE_STATUS_CODE, 202),
Expand Down Expand Up @@ -518,6 +524,7 @@ void getToBadEndpointTest(Parameter parameter) {
val -> val.isInstanceOf(Long.class)),
equalTo(SERVER_ADDRESS, "localhost"),
satisfies(SERVER_PORT, val -> val.isInstanceOf(Long.class)),
equalTo(CLIENT_ADDRESS, "127.0.0.1"),
equalTo(URL_PATH, parameter.urlPath),
equalTo(HTTP_REQUEST_METHOD, "GET"),
equalTo(HTTP_RESPONSE_STATUS_CODE, 500),
Expand Down Expand Up @@ -598,6 +605,7 @@ void redirectTest() {
val -> val.isInstanceOf(Long.class)),
equalTo(SERVER_ADDRESS, "localhost"),
satisfies(SERVER_PORT, val -> val.isInstanceOf(Long.class)),
equalTo(CLIENT_ADDRESS, "127.0.0.1"),
equalTo(URL_PATH, "/double-greet-redirect"),
equalTo(HTTP_REQUEST_METHOD, "GET"),
equalTo(HTTP_RESPONSE_STATUS_CODE, 307),
Expand Down Expand Up @@ -626,6 +634,7 @@ void redirectTest() {
val -> val.isInstanceOf(Long.class)),
equalTo(SERVER_ADDRESS, "localhost"),
satisfies(SERVER_PORT, val -> val.isInstanceOf(Long.class)),
equalTo(CLIENT_ADDRESS, "127.0.0.1"),
equalTo(URL_PATH, "/double-greet"),
equalTo(HTTP_REQUEST_METHOD, "GET"),
equalTo(HTTP_RESPONSE_STATUS_CODE, 200),
Expand Down Expand Up @@ -677,6 +686,7 @@ void multipleGetsToDelayingRoute(Parameter parameter) {
val -> val.isInstanceOf(Long.class)),
equalTo(SERVER_ADDRESS, "localhost"),
satisfies(SERVER_PORT, val -> val.isInstanceOf(Long.class)),
equalTo(CLIENT_ADDRESS, "127.0.0.1"),
equalTo(URL_PATH, parameter.urlPath),
equalTo(HTTP_REQUEST_METHOD, "GET"),
equalTo(HTTP_RESPONSE_STATUS_CODE, 200),
Expand Down Expand Up @@ -758,6 +768,7 @@ void cancelRequestTest() throws Exception {
val -> val.isInstanceOf(Long.class)),
equalTo(SERVER_ADDRESS, "localhost"),
satisfies(SERVER_PORT, val -> val.isInstanceOf(Long.class)),
equalTo(CLIENT_ADDRESS, "127.0.0.1"),
equalTo(URL_PATH, "/slow"),
equalTo(HTTP_REQUEST_METHOD, "GET"),
equalTo(URL_SCHEME, "http"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ class VertxReactivePropagationTest extends AgentInstrumentationSpecification {
"$NetworkAttributes.NETWORK_PEER_PORT" Long
"$SemanticAttributes.SERVER_ADDRESS" "localhost"
"$SemanticAttributes.SERVER_PORT" Long
"$SemanticAttributes.CLIENT_ADDRESS" "127.0.0.1"
"$SemanticAttributes.URL_PATH" "/listProducts"
"$SemanticAttributes.HTTP_REQUEST_METHOD" "GET"
"$SemanticAttributes.HTTP_RESPONSE_STATUS_CODE" 200
Expand Down Expand Up @@ -158,6 +159,7 @@ class VertxReactivePropagationTest extends AgentInstrumentationSpecification {
"$NetworkAttributes.NETWORK_PEER_PORT" Long
"$SemanticAttributes.SERVER_ADDRESS" "localhost"
"$SemanticAttributes.SERVER_PORT" Long
"$SemanticAttributes.CLIENT_ADDRESS" "127.0.0.1"
"$SemanticAttributes.URL_PATH" baseUrl
"$SemanticAttributes.URL_QUERY" "$TEST_REQUEST_ID_PARAMETER=$requestId"
"$SemanticAttributes.HTTP_REQUEST_METHOD" "GET"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ class VertxReactivePropagationTest extends AgentInstrumentationSpecification {
"$NetworkAttributes.NETWORK_PEER_PORT" Long
"$SemanticAttributes.SERVER_ADDRESS" "localhost"
"$SemanticAttributes.SERVER_PORT" Long
"$SemanticAttributes.CLIENT_ADDRESS" "127.0.0.1"
"$SemanticAttributes.URL_PATH" "/listProducts"
"$SemanticAttributes.HTTP_REQUEST_METHOD" "GET"
"$SemanticAttributes.HTTP_RESPONSE_STATUS_CODE" 200
Expand Down Expand Up @@ -158,6 +159,7 @@ class VertxReactivePropagationTest extends AgentInstrumentationSpecification {
"$NetworkAttributes.NETWORK_PEER_PORT" Long
"$SemanticAttributes.SERVER_ADDRESS" "localhost"
"$SemanticAttributes.SERVER_PORT" Long
"$SemanticAttributes.CLIENT_ADDRESS" "127.0.0.1"
"$SemanticAttributes.URL_PATH" baseUrl
"$SemanticAttributes.URL_QUERY" "$TEST_REQUEST_ID_PARAMETER=$requestId"
"$SemanticAttributes.HTTP_REQUEST_METHOD" "GET"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,9 @@ abstract class AppServerTest extends SmokeTest {
and: "Server span for the remote call"
traces.countFilteredAttributes(SemanticAttributes.URL_PATH.key, "/app/headers") == 1

and: "Number of spans with client address"
traces.countFilteredAttributes(SemanticAttributes.CLIENT_ADDRESS.key, "127.0.0.1") == 1

and: "Number of spans with http protocol version"
traces.countFilteredAttributes(SemanticAttributes.NETWORK_PROTOCOL_VERSION.key, "1.1") == 3

Expand Down

0 comments on commit d4435c9

Please sign in to comment.