Merge pull request #3556 from DataDog/glopes/enable-endpoint-appsec

Enable http.endpoint calculation when appsec is explicitly enabled

Author: cataphract

appsec/tests/extension/client_init_record_span_tags.phpt

@@ -92,6 +92,7 @@ Array
     [_dd.p.tid] => %s
     [_dd.runtime_family] => php
     [appsec.event] => true
+    [http.endpoint] => /foo
     [http.method] => GET
     [http.request.headers.user-agent] => my user agent
     [http.response.headers.content-type] => text/html; charset=UTF-8

appsec/tests/extension/rinit_record_span_tags.phpt

@@ -87,6 +87,7 @@ Array
     [_dd.p.tid] => %s
     [_dd.runtime_family] => php
     [appsec.event] => true
+    [http.endpoint] => /foo
     [http.method] => GET
     [http.request.headers.user-agent] => my user agent
     [http.response.headers.content-type] => text/html; charset=UTF-8
@@ -107,4 +108,4 @@ Array
     [php.memory.peak_usage_bytes] => %f
     [process_id] => %d
     [rshutdown_metric] => 2.1
-)
+)

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

@@ -94,6 +94,44 @@ class Apache2FpmTests implements CommonTests, SamplingTestsInFpm {
                php-fpm -y /etc/php-fpm.conf -c /etc/php/php.ini''')
     }
 
+    @Test
+    void 'resource renaming auto-enabled with appsec'() {
+        // By default, DD_APPSEC_ENABLED=true is set but DD_TRACE_RESOURCE_RENAMING_ENABLED is not set.
+        def req = container.buildReq('/hello.php').GET().build()
+        def trace = container.traceFromRequest(req, ofString()) { HttpResponse<String> resp ->
+            assert resp.body() == 'Hello world!'
+        }
+
+        def span = trace.first()
+        assert span.metrics."_dd.appsec.enabled" == 1.0d : "AppSec should be enabled"
+        assert span.meta."http.endpoint" == '/hello.php' : "http.endpoint tag should be set when AppSec is enabled"
+    }
+
+    @Test
+    void 'resource renaming disabled when explicitly set to false'() {
+        // When DD_TRACE_RESOURCE_RENAMING_ENABLED=false is explicitly set, resource renaming should be disabled
+        // even when AppSec is enabled
+        def res = container.execInContainer(
+                'bash', '-c',
+                '''kill -9 `pgrep php-fpm`;
+               export DD_TRACE_RESOURCE_RENAMING_ENABLED=false;
+               php-fpm -y /etc/php-fpm.conf -c /etc/php/php.ini''')
+        assert res.exitCode == 0
+
+        try {
+            def req = container.buildReq('/hello.php').GET().build()
+            def trace = container.traceFromRequest(req, ofString()) { HttpResponse<String> resp ->
+                assert resp.body() == 'Hello world!'
+            }
+
+            def span = trace.first()
+            assert span.metrics."_dd.appsec.enabled" == 1.0d : "AppSec should still be enabled"
+            assert span.meta."http.endpoint" == null : "http.endpoint tag should NOT be set when resource renaming is explicitly disabled"
+        } finally {
+            resetFpm()
+        }
+    }
+
     @Test
     void 'test sampling priority'() {
         // Set rate limit to 5 to ensure fewer than 15 events get sampling priority 2

ext/configuration.h

@@ -151,6 +151,7 @@ enum ddtrace_sampling_rules_format {
     CONFIG(BOOL, DD_TRACE_SYMFONY_HTTP_ROUTE, "true")                                                          \
     CONFIG(BOOL, DD_TRACE_REMOVE_ROOT_SPAN_LARAVEL_QUEUE, "true")                                              \
     CONFIG(BOOL, DD_TRACE_REMOVE_ROOT_SPAN_SYMFONY_MESSENGER, "true")                                          \
+    CONFIG(BOOL, DD_APPSEC_ENABLED, "false", .ini_change = zai_config_system_ini_change)                       \
     CONFIG(BOOL, DD_APPSEC_RASP_ENABLED , "true")                                                              \
     CONFIG(BOOL, DD_TRACE_REMOVE_AUTOINSTRUMENTATION_ORPHANS, "false")                                         \
     CONFIG(SET, DD_TRACE_RESOURCE_URI_FRAGMENT_REGEX, "")                                                      \

ext/span.c

@@ -864,7 +864,12 @@ void ddtrace_close_span(ddtrace_span_data *span) {
         zend_execute_data *execute_data = EG(current_execute_data);
         ddtrace_maybe_add_code_origin_information(span, execute_data && EX(func) && !ZEND_USER_CODE(EX(func)->type));
 
-        if (get_DD_TRACE_RESOURCE_RENAMING_ENABLED()) {
+        // Enable resource renaming if:
+        // - DD_TRACE_RESOURCE_RENAMING_ENABLED is explicitly set to true, OR
+        // - DD_APPSEC_ENABLED is true and DD_TRACE_RESOURCE_RENAMING_ENABLED is not explicitly set
+        bool resource_renaming_at_default = zai_config_memoized_entries[DDTRACE_CONFIG_DD_TRACE_RESOURCE_RENAMING_ENABLED].name_index == ZAI_CONFIG_ORIGIN_DEFAULT;
+        bool enable_resource_renaming = resource_renaming_at_default ? get_DD_APPSEC_ENABLED() : get_DD_TRACE_RESOURCE_RENAMING_ENABLED();
+        if (enable_resource_renaming) {
             ddtrace_maybe_add_guessed_endpoint_tag(ROOTSPANDATA(&span->std));
         }
     }