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

config_consistency: test ClientIPHeader #3045

Merged
merged 10 commits into from
Sep 19, 2024
Merged

config_consistency: test ClientIPHeader #3045

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
cf028ed
config_consistency: test ClientIPHeader
mabdinur Sep 17, 2024
cd44891
simplify tests and update all manifest files
mabdinur Sep 17, 2024
feb1fa2
Merge branch 'main' into APMAPI-351/munir-test-ip-header-consistency
mabdinur Sep 17, 2024
652035a
skip cpp
mabdinur Sep 18, 2024
bba8ac0
Merge branch 'main' into APMAPI-351/munir-test-ip-header-consistency
mabdinur Sep 18, 2024
bc423c0
fix return in _get_span_by_tags
mabdinur Sep 18, 2024
98af0dc
clean up usages of _get_span_by_tags in error status tests
mabdinur Sep 18, 2024
92331ea
add test coverage for all supported ip headers
mabdinur Sep 18, 2024
90004f7
test lastest nodejs
mabdinur Sep 19, 2024
5c3f2c4
node version
mabdinur Sep 19, 2024
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
1 change: 1 addition & 0 deletions manifests/cpp.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,7 @@ tests/:
test_miscs.py:
Test_Miscs: missing_feature
test_config_consistency.py:
Test_Config_ClientIPHeader_Configured: missing_feature (DD_TRACE_CLIENT_IP_HEADER not implemented)
Test_Config_ClientTagQueryString_Configured: missing_feature
Test_Config_ClientTagQueryString_Empty: missing_feature (test can not capture span with the expected http.url tag)
Test_Config_HttpClientErrorStatuses_Default: missing_feature
Expand Down
1 change: 1 addition & 0 deletions manifests/dotnet.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -365,6 +365,7 @@ tests/:
Test_RemoteConfigurationUpdateSequenceLiveDebugging: v2.15.0
Test_RemoteConfigurationUpdateSequenceLiveDebuggingNoCache: irrelevant (cache is implemented)
test_config_consistency.py:
Test_Config_ClientIPHeader_Configured: v2.48.0
Test_Config_ClientTagQueryString_Configured: missing_feature (configuration DNE)
Test_Config_ClientTagQueryString_Empty: v2.53.0
Test_Config_HttpClientErrorStatuses_Default: missing_feature
Expand Down
1 change: 1 addition & 0 deletions manifests/golang.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -485,6 +485,7 @@ tests/:
Test_RemoteConfigurationUpdateSequenceLiveDebugging: missing_feature
Test_RemoteConfigurationUpdateSequenceLiveDebuggingNoCache: irrelevant (cache is implemented)
test_config_consistency.py:
Test_Config_ClientIPHeader_Configured: v1.60.0
Test_Config_ClientTagQueryString_Configured: missing_feature (supports DD_TRACE_HTTP_URL_QUERY_STRING_DISABLED)
Test_Config_ClientTagQueryString_Empty: v1.60.0
Test_Config_HttpClientErrorStatuses_Default: missing_feature
Expand Down
1 change: 1 addition & 0 deletions manifests/java.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1235,6 +1235,7 @@ tests/:
Test_Mock: v0.0.99
Test_NotReleased: missing_feature
test_config_consistency.py:
Test_Config_ClientIPHeader_Configured: v1.38.0
Test_Config_ClientTagQueryString_Configured: missing_feature (endpoints return 404, but in theory should work)
Test_Config_ClientTagQueryString_Empty: missing_feature (incorrect default value)
Test_Config_HttpClientErrorStatuses_Default: missing_feature
Expand Down
1 change: 1 addition & 0 deletions manifests/nodejs.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -560,6 +560,7 @@ tests/:
Test_RemoteConfigurationUpdateSequenceLiveDebugging: *ref_5_16_0 #actual version unknown
Test_RemoteConfigurationUpdateSequenceLiveDebuggingNoCache: irrelevant (cache is implemented)
test_config_consistency.py:
Test_Config_ClientIPHeader_Configured: *ref_5_15_0
mabdinur marked this conversation as resolved.
Show resolved Hide resolved
mabdinur marked this conversation as resolved.
Show resolved Hide resolved
Test_Config_ClientTagQueryString_Configured: missing_feature (adding query string to http.url is not supported)
Test_Config_ClientTagQueryString_Empty: missing_feature (removes query strings by default)
Test_Config_HttpClientErrorStatuses_Default: missing_feature
Expand Down
1 change: 1 addition & 0 deletions manifests/php.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -316,6 +316,7 @@ tests/:
test_miscs.py:
Test_Miscs: missing_feature
test_config_consistency.py:
Test_Config_ClientIPHeader_Configured: v1.3,0
mabdinur marked this conversation as resolved.
Show resolved Hide resolved
Test_Config_ClientTagQueryString_Configured: missing_feature (supports dd_trace_http_url_query_param_allowed instead)
Test_Config_ClientTagQueryString_Empty: v1.2.0
Test_Config_HttpClientErrorStatuses_Default: missing_feature
Expand Down
1 change: 1 addition & 0 deletions manifests/python.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -758,6 +758,7 @@ tests/:
Test_RemoteConfigurationUpdateSequenceLiveDebugging: v2.8.0.dev
Test_RemoteConfigurationUpdateSequenceLiveDebuggingNoCache: missing_feature
test_config_consistency.py:
Test_Config_ClientIPHeader_Configured: v2.12.0
Test_Config_ClientTagQueryString_Configured: missing_feature (supports DD_HTPP_CLIENT_TAGS_QUERY_STRING instead)
Test_Config_ClientTagQueryString_Empty: v2.12.0
Test_Config_HttpClientErrorStatuses_Default: missing_feature
Expand Down
1 change: 1 addition & 0 deletions manifests/ruby.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -386,6 +386,7 @@ tests/:
test_miscs.py:
Test_Miscs: missing_feature
test_config_consistency.py:
Test_Config_ClientIPHeader_Configured: v2.3.0
Test_Config_ClientTagQueryString_Configured: missing_feature
Test_Config_ClientTagQueryString_Empty: missing_feature (removes query string by default)
Test_Config_HttpClientErrorStatuses_Default: missing_feature
Expand Down
57 changes: 35 additions & 22 deletions tests/test_config_consistency.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ def test_status_code_400(self):
interfaces.library.assert_trace_exists(self.r)
spans = [s for _, _, s in interfaces.library.get_spans(request=self.r, full_trace=True)]

client_span = _get_span(spans, resource_name="GET /status", tags={"span.kind": "client"})
client_span = _get_span_by_tags(spans, tags={"span.kind": "client"})

assert client_span.get("meta").get("http.status_code") == "400"
assert client_span.get("error") == 1
Expand All @@ -106,7 +106,7 @@ def test_status_code_500(self):
interfaces.library.assert_trace_exists(self.r)
spans = [s for _, _, s in interfaces.library.get_spans(request=self.r, full_trace=True)]

client_span = _get_span(spans, resource_name="GET /status", tags={"span.kind": "client"})
client_span = _get_span_by_tags(spans, tags={"span.kind": "client"})

assert client_span.get("meta").get("http.status_code") == "500"
assert client_span.get("error") == None or client_span.get("error") == 0
Expand All @@ -128,7 +128,7 @@ def test_status_code_200(self):
interfaces.library.assert_trace_exists(self.r)
spans = [s for _, _, s in interfaces.library.get_spans(request=self.r, full_trace=True)]

client_span = _get_span(spans, resource_name="GET /status", tags={"span.kind": "client"})
client_span = _get_span_by_tags(spans, tags={"span.kind": "client"})

assert client_span.get("meta").get("http.status_code") == "200"
assert client_span.get("error") == 1
Expand All @@ -144,7 +144,7 @@ def test_status_code_202(self):
interfaces.library.assert_trace_exists(self.r)
spans = [s for _, _, s in interfaces.library.get_spans(request=self.r, full_trace=True)]

client_span = _get_span(spans, resource_name="GET /status", tags={"span.kind": "client"})
client_span = _get_span_by_tags(spans, tags={"span.kind": "client"})

assert client_span.get("meta").get("http.status_code") == "202"
assert client_span.get("error") == 1
Expand Down Expand Up @@ -178,33 +178,46 @@ def test_query_string_redaction(self):
assert _get_span_by_tags(trace, expected_tags), f"Span with tags {expected_tags} not found in {trace}"


def _get_span(spans, resource_name, tags):
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@zacharycmontoya I replaced _get_span with _get_span_by_tags. I found that this approach works best with most libraries. Resource names are not set in a consistent way across client libraries. This is something we can work with IDM to address.

for s in spans:
match = True
if s["resource"] != resource_name:
continue

for tagKey in tags:
if tagKey in s["meta"]:
expectValue = tags[tagKey]
actualValue = s["meta"][tagKey]
if expectValue != actualValue:
continue

if match:
return s
return {}
@scenarios.tracing_config_nondefault
@features.tracing_configuration_consistency
class Test_Config_ClientIPHeader_Configured:
"""Verify headers containing ips are tagged when DD_TRACE_CLIENT_IP_ENABLED=true
and DD_TRACE_CLIENT_IP_HEADER=custom-ip-header"""

IP_HEADERS = {
"custom-ip-header": "5.6.7.8",
"x-forwarded-for": "98.73.45.0",
"x-real-ip": "98.73.145.1",
"true-client-ip": "98.73.45.2",
"x-client-ip": "98.73.45.3",
"x-forwarded": "98.73.45.4",
"forwarded-for": "98.73.45.5",
"x-cluster-client-ip": "98.73.45.6",
"fastly-client-ip": "98.73.45.7",
"cf-connecting-ip": "98.73.45.8",
"cf-connecting-ipv6": "2001:db8:3333:4444:5555:6666:7777:8888",
}

def setup_ip_headers_sent_in_one_client_requests(self):
self.req2 = weblog.get("/make_distant_call", params={"url": "http://weblog:7777"}, headers=self.IP_HEADERS)

def test_ip_headers_sent_in_one_client_requests(self):
# Ensures the header set in DD_TRACE_CLIENT_IP_HEADER takes precedence over all supported ip headers
trace = [span for _, _, span in interfaces.library.get_spans(self.req2, full_trace=True)]
expected_tags = {"http.client_ip": "5.6.7.8"}
assert _get_span_by_tags(trace, expected_tags), f"Span with tags {expected_tags} not found in {trace}"


def _get_span_by_tags(trace, tags):
for span in trace:
def _get_span_by_tags(spans, tags):
for span in spans:
# Avoids retrieving the client span by the operation/resource name, this value varies between languages
# Use the expected tags to identify the span
for k, v in tags.items():
if span["meta"].get(k) != v:
break
else:
return span
return {}


@scenarios.tracing_config_nondefault
Expand Down
4 changes: 4 additions & 0 deletions utils/_context/_scenarios/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -446,6 +446,10 @@ def all_endtoend_scenarios(test_object):
"TRACING_CONFIG_NONDEFAULT",
weblog_env={
"DD_TRACE_HTTP_SERVER_ERROR_STATUSES": "200-201,202",
"DD_TRACE_CLIENT_IP_ENABLED": "true",
"DD_TRACE_CLIENT_IP_HEADER": "custom-ip-header",
# disable ASM to test non asm client ip tagging
mabdinur marked this conversation as resolved.
Show resolved Hide resolved
"DD_APPSEC_ENABLED": "false",
"DD_TRACE_HTTP_CLIENT_ERROR_STATUSES": "200-201,202",
"DD_SERVICE": "service_test",
},
Expand Down
Loading