Skip to content

Commit 98a31cb

Browse files
cataphractcataphract
authored
Merge pull request #3556 from DataDog/glopes/enable-endpoint-appsec
Enable http.endpoint calculation when appsec is explicitly enabled
2 parents 1e38574 + f1ee6fb commit 98a31cb

5 files changed

Lines changed: 48 additions & 2 deletions

File tree

‎appsec/tests/extension/client_init_record_span_tags.phpt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,7 @@ Array
9292
[_dd.p.tid] => %s
9393
[_dd.runtime_family] => php
9494
[appsec.event] => true
95+
[http.endpoint] => /foo
9596
[http.method] => GET
9697
[http.request.headers.user-agent] => my user agent
9798
[http.response.headers.content-type] => text/html; charset=UTF-8

‎appsec/tests/extension/rinit_record_span_tags.phpt‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,7 @@ Array
8787
[_dd.p.tid] => %s
8888
[_dd.runtime_family] => php
8989
[appsec.event] => true
90+
[http.endpoint] => /foo
9091
[http.method] => GET
9192
[http.request.headers.user-agent] => my user agent
9293
[http.response.headers.content-type] => text/html; charset=UTF-8
@@ -107,4 +108,4 @@ Array
107108
[php.memory.peak_usage_bytes] => %f
108109
[process_id] => %d
109110
[rshutdown_metric] => 2.1
110-
)
111+
)

‎appsec/tests/integration/src/test/groovy/com/datadog/appsec/php/integration/Apache2FpmTests.groovy‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,44 @@ class Apache2FpmTests implements CommonTests, SamplingTestsInFpm {
9494
php-fpm -y /etc/php-fpm.conf -c /etc/php/php.ini''')
9595
}
9696

97+
@Test
98+
void 'resource renaming auto-enabled with appsec'() {
99+
// By default, DD_APPSEC_ENABLED=true is set but DD_TRACE_RESOURCE_RENAMING_ENABLED is not set.
100+
def req = container.buildReq('/hello.php').GET().build()
101+
def trace = container.traceFromRequest(req, ofString()) { HttpResponse<String> resp ->
102+
assert resp.body() == 'Hello world!'
103+
}
104+
105+
def span = trace.first()
106+
assert span.metrics."_dd.appsec.enabled" == 1.0d : "AppSec should be enabled"
107+
assert span.meta."http.endpoint" == '/hello.php' : "http.endpoint tag should be set when AppSec is enabled"
108+
}
109+
110+
@Test
111+
void 'resource renaming disabled when explicitly set to false'() {
112+
// When DD_TRACE_RESOURCE_RENAMING_ENABLED=false is explicitly set, resource renaming should be disabled
113+
// even when AppSec is enabled
114+
def res = container.execInContainer(
115+
'bash', '-c',
116+
'''kill -9 `pgrep php-fpm`;
117+
export DD_TRACE_RESOURCE_RENAMING_ENABLED=false;
118+
php-fpm -y /etc/php-fpm.conf -c /etc/php/php.ini''')
119+
assert res.exitCode == 0
120+
121+
try {
122+
def req = container.buildReq('/hello.php').GET().build()
123+
def trace = container.traceFromRequest(req, ofString()) { HttpResponse<String> resp ->
124+
assert resp.body() == 'Hello world!'
125+
}
126+
127+
def span = trace.first()
128+
assert span.metrics."_dd.appsec.enabled" == 1.0d : "AppSec should still be enabled"
129+
assert span.meta."http.endpoint" == null : "http.endpoint tag should NOT be set when resource renaming is explicitly disabled"
130+
} finally {
131+
resetFpm()
132+
}
133+
}
134+
97135
@Test
98136
void 'test sampling priority'() {
99137
// Set rate limit to 5 to ensure fewer than 15 events get sampling priority 2

‎ext/configuration.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,7 @@ enum ddtrace_sampling_rules_format {
151151
CONFIG(BOOL, DD_TRACE_SYMFONY_HTTP_ROUTE, "true") \
152152
CONFIG(BOOL, DD_TRACE_REMOVE_ROOT_SPAN_LARAVEL_QUEUE, "true") \
153153
CONFIG(BOOL, DD_TRACE_REMOVE_ROOT_SPAN_SYMFONY_MESSENGER, "true") \
154+
CONFIG(BOOL, DD_APPSEC_ENABLED, "false", .ini_change = zai_config_system_ini_change) \
154155
CONFIG(BOOL, DD_APPSEC_RASP_ENABLED , "true") \
155156
CONFIG(BOOL, DD_TRACE_REMOVE_AUTOINSTRUMENTATION_ORPHANS, "false") \
156157
CONFIG(SET, DD_TRACE_RESOURCE_URI_FRAGMENT_REGEX, "") \

‎ext/span.c‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -864,7 +864,12 @@ void ddtrace_close_span(ddtrace_span_data *span) {
864864
zend_execute_data *execute_data = EG(current_execute_data);
865865
ddtrace_maybe_add_code_origin_information(span, execute_data && EX(func) && !ZEND_USER_CODE(EX(func)->type));
866866

867-
if (get_DD_TRACE_RESOURCE_RENAMING_ENABLED()) {
867+
// Enable resource renaming if:
868+
// - DD_TRACE_RESOURCE_RENAMING_ENABLED is explicitly set to true, OR
869+
// - DD_APPSEC_ENABLED is true and DD_TRACE_RESOURCE_RENAMING_ENABLED is not explicitly set
870+
bool resource_renaming_at_default = zai_config_memoized_entries[DDTRACE_CONFIG_DD_TRACE_RESOURCE_RENAMING_ENABLED].name_index == ZAI_CONFIG_ORIGIN_DEFAULT;
871+
bool enable_resource_renaming = resource_renaming_at_default ? get_DD_APPSEC_ENABLED() : get_DD_TRACE_RESOURCE_RENAMING_ENABLED();
872+
if (enable_resource_renaming) {
868873
ddtrace_maybe_add_guessed_endpoint_tag(ROOTSPANDATA(&span->std));
869874
}
870875
}

0 commit comments

Comments
 (0)