Merge remote-tracking branch 'origin/master' into florian/prof-15506-guard-internal-interrupt

Author: morrisonlevi

Cargo.toml

@@ -52,7 +52,10 @@ hyper-util = { version = "0.1.10", features = [
   "client",
   "client-legacy",
 ] }
+prost-build = { version = "0.14.1", default-features = false }
+protoc-bin-vendored = { version = "3.0.0", default-features = false }
 serde = { version = "1.0", default-features = false }
+serde_json = { version = "1.0", default-features = false, features = ["alloc"] }
 tokio = { version = "1.36", default-features = false }
 tracing = { version = "0.1", default-features = false }
 

appsec/cmake/ddtrace.cmake

@@ -92,6 +92,8 @@ file(GLOB_RECURSE FILES_DDTRACE
     CONFIGURE_DEPENDS
     "${CMAKE_SOURCE_DIR}/../ext/*.c"
     "${CMAKE_SOURCE_DIR}/../ext/**/*.c"
+    "${CMAKE_SOURCE_DIR}/../tracer/*.c"
+    "${CMAKE_SOURCE_DIR}/../tracer/**/*.c"
     "${CMAKE_SOURCE_DIR}/../zend_abstract_interface/*.c"
     "${CMAKE_SOURCE_DIR}/../zend_abstract_interface/**/*.c"
 )
@@ -101,30 +103,38 @@ list(APPEND FILES_DDTRACE
     "${CMAKE_SOURCE_DIR}/../components/log/log.c"
     "${CMAKE_SOURCE_DIR}/../components/sapi/sapi.c"
     "${CMAKE_SOURCE_DIR}/../components/string_view/string_view.c"
+    "${CMAKE_SOURCE_DIR}/../tracer/vendor/mpack/mpack.c"
+    "${CMAKE_SOURCE_DIR}/../tracer/vendor/mt19937/mt19937-64.c"
 )
 if (PhpConfig_VERNUM GREATER_EQUAL 80000)
-    list(REMOVE_ITEM FILES_DDTRACE "${CMAKE_SOURCE_DIR}/../ext/handlers_curl_php7.c"
+    list(REMOVE_ITEM FILES_DDTRACE "${CMAKE_SOURCE_DIR}/../tracer/handlers_curl_php7.c"
         "${CMAKE_SOURCE_DIR}/../zend_abstract_interface/interceptor/php7/interceptor.c"
         "${CMAKE_SOURCE_DIR}/../zend_abstract_interface/interceptor/php7/resolver.c"
         "${CMAKE_SOURCE_DIR}/../zend_abstract_interface/sandbox/php7/sandbox.c")
 else() # PHP 7
-    list(REMOVE_ITEM FILES_DDTRACE "${CMAKE_SOURCE_DIR}/../ext/handlers_curl.c"
-        "${CMAKE_SOURCE_DIR}/../ext/hook/uhook_attributes.c"
-        "${CMAKE_SOURCE_DIR}/../ext/hook/uhook_otel.c"
+    list(REMOVE_ITEM FILES_DDTRACE "${CMAKE_SOURCE_DIR}/../tracer/handlers_curl.c"
+        "${CMAKE_SOURCE_DIR}/../tracer/hook/uhook_attributes.c"
+        "${CMAKE_SOURCE_DIR}/../tracer/hook/uhook_otel.c"
         "${CMAKE_SOURCE_DIR}/../zend_abstract_interface/interceptor/php8/interceptor.c"
         "${CMAKE_SOURCE_DIR}/../zend_abstract_interface/interceptor/php8/resolver.c"
         "${CMAKE_SOURCE_DIR}/../zend_abstract_interface/interceptor/php8/resolver_pre-8_2.c"
         "${CMAKE_SOURCE_DIR}/../zend_abstract_interface/jit_utils/jit_blacklist.c"
         "${CMAKE_SOURCE_DIR}/../zend_abstract_interface/sandbox/php8/sandbox.c")
 endif()
+if (PhpConfig_VERNUM GREATER_EQUAL 70300)
+    list(REMOVE_ITEM FILES_DDTRACE "${CMAKE_SOURCE_DIR}/../ext/zend_hrtime.c")
+endif()
+if (PhpConfig_VERNUM LESS 80000 OR PhpConfig_VERNUM GREATER_EQUAL 80200)
+    list(REMOVE_ITEM FILES_DDTRACE "${CMAKE_SOURCE_DIR}/../ext/patch_zend_call_known_function.c")
+endif()
 if (PhpConfig_VERNUM LESS 80200)
-    list(REMOVE_ITEM FILES_DDTRACE "${CMAKE_SOURCE_DIR}/../ext/weakrefs.c")
     list(REMOVE_ITEM FILES_DDTRACE "${CMAKE_SOURCE_DIR}/../zend_abstract_interface/interceptor/php8/resolver.c")
 else() # PHP 8.2+
-    list(REMOVE_ITEM FILES_DDTRACE "${CMAKE_SOURCE_DIR}/../zend_abstract_interface/interceptor/php8/resolver_pre-8_2.c")
+    list(REMOVE_ITEM FILES_DDTRACE "${CMAKE_SOURCE_DIR}/../tracer/weakrefs.c"
+        "${CMAKE_SOURCE_DIR}/../zend_abstract_interface/interceptor/php8/resolver_pre-8_2.c")
 endif()
 if (PhpConfig_VERNUM LESS 80100)
-    list(REMOVE_ITEM FILES_DDTRACE "${CMAKE_SOURCE_DIR}/../ext/handlers_fiber.c")
+    list(REMOVE_ITEM FILES_DDTRACE "${CMAKE_SOURCE_DIR}/../tracer/handlers_fiber.c")
 endif()
 list(REMOVE_ITEM FILES_DDTRACE "${CMAKE_SOURCE_DIR}/../ext/crashtracking_windows.c")
 
@@ -162,16 +172,20 @@ endif()
 if(CURL_DEFINITIONS)
     target_compile_definitions(ddtrace PRIVATE ${CURL_DEFINITIONS})
 endif()
-target_compile_definitions(ddtrace PRIVATE ZEND_ENABLE_STATIC_TSRMLS_CACHE=1 COMPILE_DL_DDTRACE=1)
+target_compile_definitions(ddtrace PRIVATE ZEND_ENABLE_STATIC_TSRMLS_CACHE=1 COMPILE_DL_DDTRACE=1 DDTRACE=1)
 target_include_directories(ddtrace PRIVATE
     ${CURL_INCLUDE_DIRS}
     ${CMAKE_SOURCE_DIR}/..
     ${CMAKE_SOURCE_DIR}/../src/dogstatsd
     ${CMAKE_SOURCE_DIR}/../zend_abstract_interface
     ${CMAKE_SOURCE_DIR}/../ext
-    ${CMAKE_SOURCE_DIR}/../ext/vendor
-    ${CMAKE_SOURCE_DIR}/../ext/vendor/mt19937
+    ${CMAKE_SOURCE_DIR}/../tracer
+    ${CMAKE_SOURCE_DIR}/../tracer/integrations
+    ${CMAKE_SOURCE_DIR}/../tracer/vendor
+    ${CMAKE_SOURCE_DIR}/../tracer/vendor/mpack
+    ${CMAKE_SOURCE_DIR}/../tracer/vendor/mt19937
     ${CMAKE_BINARY_DIR}/gen_ddtrace
+    ${CMAKE_BINARY_DIR}/gen_ddtrace/ext
 )
 add_dependencies(ddtrace ddtrace_exports update_version_h)
 

appsec/tests/integration/build.gradle

@@ -343,6 +343,9 @@ def buildTracerTask = { String version, String variant, altBaseTag = null ->
             inputs: [
                     dirs: [
                             '../../../ext',
+                            '../../../tracer',
+                            '../../../components',
+                            '../../../components-rs',
                             '../../../zend_abstract_interface',
                             '../../../libdatadog',
                             '../../../datadog.sym',
@@ -375,8 +378,10 @@ def buildTracerCmakeTask = { String version, String variant, altBaseTag = null -
             inputs: [
                     dirs: [
                             '../../../ext',
-                            '../../../zend_abstract_interface',
+                            '../../../tracer',
+                            '../../../components',
                             '../../../components-rs',
+                            '../../../zend_abstract_interface',
                             '../../../libdatadog',
                             '../../../datadog.sym',
                             '../../cmake',
@@ -471,8 +476,10 @@ def buildTracerSsiCmakeTask = { String version, String variant ->
             inputs: [
                     dirs: [
                             '../../../ext',
-                            '../../../zend_abstract_interface',
+                            '../../../tracer',
+                            '../../../components',
                             '../../../components-rs',
+                            '../../../zend_abstract_interface',
                             '../../../libdatadog',
                             '../../../datadog.sym',
                             '../../cmake',

appsec/tests/integration/src/main/groovy/com/datadog/appsec/php/TelemetryHelpers.groovy

@@ -41,6 +41,33 @@ class TelemetryHelpers {
         payloads.collect { type.newInstance([it] as Object[]) }
     }
 
+    /**
+     * Synthetic service the in-process background sender reports its own metrics under
+     * (see dd_bgs_register_application() in tracer/tracer_telemetry.c). The counters describe
+     * the sender, not the application being traced, so they are deliberately not attributed to
+     * the request's service.
+     */
+    static final String BGS_SERVICE = 'background_sender-php-service'
+
+    /**
+     * Drains telemetry and returns the {@code generate-metrics} series submitted under the
+     * given service.
+     *
+     * <p>{@link #filterMessages} only tells the sidecar's own telemetry apart from everything
+     * else; this keys on the enclosing message's service name, which is what separates the
+     * background sender's synthetic application ({@link #BGS_SERVICE}) from the traced ones.
+     */
+    static List<Metric> drainMetricSeries(AppSecContainer container, String service, int timeoutInMs = 500) {
+        List<Metric> series = []
+        for (msg in container.drainTelemetry(timeoutInMs)) {
+            if (msg.application?.service_name != service) continue
+            for (GenerateMetrics metrics in filterMessages([msg], GenerateMetrics, false)) {
+                series.addAll(metrics.series)
+            }
+        }
+        series
+    }
+
     static class GenerateMetrics {
         static names = ['generate-metrics']
         List<Metric> series

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

@@ -0,0 +1,90 @@
+package com.datadog.appsec.php.integration
+
+import com.datadog.appsec.php.TelemetryHelpers
+import com.datadog.appsec.php.TelemetryHelpers.Metric
+import com.datadog.appsec.php.docker.AppSecContainer
+import com.datadog.appsec.php.docker.FailOnUnmatchedTraces
+import org.junit.jupiter.api.Test
+import org.junit.jupiter.api.condition.DisabledIf
+import org.testcontainers.containers.Container.ExecResult
+import org.testcontainers.junit.jupiter.Container
+import org.testcontainers.junit.jupiter.Testcontainers
+
+import static com.datadog.appsec.php.TelemetryHelpers.BGS_SERVICE
+import static com.datadog.appsec.php.integration.TestParams.getPhpVersion
+import static com.datadog.appsec.php.integration.TestParams.getVariant
+
+/**
+ * What the background sender does while its process goes away: submit the counters it has
+ * accumulated since the last flush from MSHUTDOWN (see ddtrace_mshutdown()), while the sidecar
+ * can still address the application.
+ *
+ * A single-request CLI process isolates the process-exit path: its trace is queued before
+ * telemetry finalize, and the sender is synchronously drained later in MSHUTDOWN. The FPM
+ * workers, by contrast, are killed abruptly at the end of a run and never reach MSHUTDOWN.
+ *
+ * No request is ever served in this container. Since the background sender's application is
+ * synthetic and shared by every process (see {@link TelemetryBackgroundSenderTests}), request
+ * traffic would make any {@code trace_api} point observed here unattributable — that is what
+ * keeps this apart from the request-path class, which has a container of its own.
+ */
+@Testcontainers
+@DisabledIf('isDisabled')
+class TelemetryBackgroundSenderShutdownTests {
+    static boolean disabled = phpVersion != '8.2'
+
+    @Container
+    @FailOnUnmatchedTraces
+    public static final AppSecContainer CONTAINER =
+            new AppSecContainer(
+                    workVolume: this.name,
+                    baseTag: 'apache2-fpm-php',
+                    phpVersion: phpVersion,
+                    phpVariant: variant,
+                    www: 'base',
+            )
+
+    private static final String FLUSH_PROBE_SERVICE = 'bgs_flush_probe'
+    private static final long METRICS_WAIT_TIMEOUT_MS = 30_000
+    private static final long METRICS_POLL_INTERVAL_MS = 500
+
+    /**
+     * The process it starts is the only one in the container that can have produced a trace_api
+     * point: the metrics carry no process identity, so anything already queued for the synthetic
+     * service would satisfy the assertion below.
+     *
+     * The sidecar buffers the points in the telemetry worker for this service/env and emits them
+     * on its next flush (DD_TELEMETRY_HEARTBEAT_INTERVAL, 10 s here), hence the generous wait.
+     */
+    @Test
+    void 'metrics accumulated during shutdown are submitted'() {
+        ExecResult res = CONTAINER.execInContainer('sh', '-c',
+                "DD_SERVICE=${FLUSH_PROBE_SERVICE} php -r 'usleep(300 * 1000);'; echo status=\$?".toString())
+        assert res.stdout.readLines().last() == 'status=0' : "${res.stdout}\n${res.stderr}"
+
+        // consume the trace this generated, or @FailOnUnmatchedTraces trips
+        assert CONTAINER.nextCapturedTrace() != null
+
+        List<Metric> series = []
+        long deadline = System.currentTimeMillis() + METRICS_WAIT_TIMEOUT_MS
+        while (!series.any { it.name == 'trace_api.requests' } &&
+                System.currentTimeMillis() < deadline) {
+            series.addAll(TelemetryHelpers.drainMetricSeries(CONTAINER, BGS_SERVICE, 0))
+            if (!series.any { it.name == 'trace_api.requests' }) {
+                long remaining = deadline - System.currentTimeMillis()
+                if (remaining > 0) {
+                    Thread.sleep(Math.min(METRICS_POLL_INTERVAL_MS, remaining))
+                }
+            }
+        }
+
+        Metric requests = series.find { it.name == 'trace_api.requests' }
+        assert requests != null : "no trace_api.requests for ${BGS_SERVICE}; got ${series*.name}"
+        assert requests.namespace == 'tracers'
+        assert requests.points[0][1] >= 1.0
+
+        Metric responses = series.find { it.name == 'trace_api.responses' }
+        assert responses != null : 'trace_api.responses not reported at shutdown'
+        assert 'status_code:2xx' in responses.tags
+    }
+}

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

@@ -0,0 +1,83 @@
+package com.datadog.appsec.php.integration
+
+import com.datadog.appsec.php.TelemetryHelpers
+import com.datadog.appsec.php.TelemetryHelpers.Metric
+import com.datadog.appsec.php.docker.AppSecContainer
+import com.datadog.appsec.php.docker.FailOnUnmatchedTraces
+import groovy.util.logging.Slf4j
+import org.junit.jupiter.api.Test
+import org.junit.jupiter.api.condition.DisabledIf
+import org.testcontainers.junit.jupiter.Container
+import org.testcontainers.junit.jupiter.Testcontainers
+
+import java.net.http.HttpResponse
+
+import static com.datadog.appsec.php.TelemetryHelpers.BGS_SERVICE
+import static com.datadog.appsec.php.integration.TestParams.getPhpVersion
+import static com.datadog.appsec.php.integration.TestParams.getVariant
+
+/**
+ * The in-process background sender (tracer/coms.c, enabled for every container through
+ * DD_TRACE_SIDECAR_TRACE_SENDER=0) counts the HTTP exchanges it has with the agent and
+ * reports them as the trace_api.requests / trace_api.responses telemetry metrics. Those
+ * counters live on a connection-wide queue id of their own, so the sidecar needs an
+ * application registered for that queue or it drops the payload with "No application
+ * found".
+ *
+ * That application is deliberately synthetic ({@link TelemetryHelpers#BGS_SERVICE} / env
+ * {@code none}): the counters describe the sender, not the traced application. It also means
+ * every process in the container reports them under the same service, and the sidecar merges
+ * same-service telemetry into a single worker, so the payloads carry nothing that ties them
+ * back to the process that produced them.
+ *
+ * This class covers the ordinary path: counters produced by request traffic and flushed by a
+ * later request. The paths that only run while a process is going away are in
+ * {@link TelemetryBackgroundSenderShutdownTests}, which needs a container where no request has
+ * ever been served — hence a separate class rather than an ordered method here.
+ *
+ * Nothing here is version-specific, so a single PHP version is enough, but both threading
+ * modes are covered.
+ */
+@Testcontainers
+@Slf4j
+@DisabledIf('isDisabled')
+class TelemetryBackgroundSenderTests {
+    static boolean disabled = phpVersion != '8.2'
+
+    @Container
+    @FailOnUnmatchedTraces
+    public static final AppSecContainer CONTAINER =
+            new AppSecContainer(
+                    workVolume: this.name,
+                    baseTag: 'apache2-fpm-php',
+                    phpVersion: phpVersion,
+                    phpVariant: variant,
+                    www: 'base',
+            )
+
+    @Test
+    void 'background sender trace_api metrics are reported'() {
+        List<Metric> series = []
+        for (int i = 0; i < 30 && !series.any { it.name == 'trace_api.requests' }; i++) {
+            // The counters are only produced once the sender thread has actually talked to the
+            // agent, and they are only flushed by a *later* request, so keep issuing them.
+            CONTAINER.traceFromRequest('/hello.php') { HttpResponse<InputStream> resp ->
+                assert resp.statusCode() == 200
+            }
+            series.addAll(TelemetryHelpers.drainMetricSeries(CONTAINER, BGS_SERVICE))
+        }
+
+        Metric requests = series.find { it.name == 'trace_api.requests' }
+        assert requests != null : "no trace_api.requests for ${BGS_SERVICE}; got ${series*.name}"
+        assert requests.namespace == 'tracers'
+        assert requests.type == 'count'
+        assert requests.points[0][1] >= 1.0
+
+        Metric responses = series.find { it.name == 'trace_api.responses' }
+        assert responses != null : 'trace_api.responses metric not received'
+        assert responses.namespace == 'tracers'
+        assert responses.type == 'count'
+        assert responses.points[0][1] >= 1.0
+        assert 'status_code:2xx' in responses.tags
+    }
+}