Update libdatadog; bgs telemetry fix

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
+    }
+}

tracer/ddtrace.c

@@ -181,6 +181,7 @@ void ddtrace_activate_once(void) {
                 // Set the default to 5000 so that BGS does not flush too often. The sidecar can flush more often, but the BGS is per process. Keep it higher to avoid too much load on the agent.
                 zai_config_change_default_ini(DATADOG_CONFIG_DD_TRACE_AGENT_FLUSH_INTERVAL, (zai_str) ZAI_STR_FROM_CSTR("5000"));
             }
+            ddtrace_telemetry_bgs_init();
             ddtrace_coms_minit(get_global_DD_TRACE_AGENT_STACK_INITIAL_SIZE(),
                                get_global_DD_TRACE_AGENT_MAX_PAYLOAD_SIZE(),
                                get_global_DD_TRACE_AGENT_STACK_BACKLOG());
@@ -241,10 +242,6 @@ void ddtrace_gshutdown(zend_datadog_globals *datadog_globals) {
     if (datadog_globals->ddtrace.agent_config_reader) {
         ddog_agent_remote_config_reader_drop(datadog_globals->ddtrace.agent_config_reader);
     }
-    if (datadog_globals->sidecar) {
-        // Drain any accumulated background-sender metrics before the transport goes away.
-        ddtrace_telemetry_flush_bgs_metrics_final(datadog_globals);
-    }
 }
 
 
@@ -400,6 +397,8 @@ void ddtrace_mshutdown() {
         ddog_sidecar_flush(&DATADOG_G(sidecar), (ddog_SidecarFlushOptions){.traces_and_stats = true, .telemetry = true});
     }
 
+    ddtrace_telemetry_flush_bgs_metrics_final();
+
     ddtrace_engine_hooks_mshutdown();
     ddtrace_shutdown_proxy_info_map();