diff --git a/Cargo.toml b/Cargo.toml index 22279e8bab..ed73244658 100644 --- a/Cargo.toml +++ b/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 } diff --git a/appsec/cmake/ddtrace.cmake b/appsec/cmake/ddtrace.cmake index 8e0f3092cd..376115a068 100644 --- a/appsec/cmake/ddtrace.cmake +++ b/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) diff --git a/appsec/tests/integration/build.gradle b/appsec/tests/integration/build.gradle index ecc4d33c60..b86d21e7c7 100644 --- a/appsec/tests/integration/build.gradle +++ b/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', diff --git a/appsec/tests/integration/src/main/groovy/com/datadog/appsec/php/TelemetryHelpers.groovy b/appsec/tests/integration/src/main/groovy/com/datadog/appsec/php/TelemetryHelpers.groovy index d9709f6e74..ce3ac9debf 100644 --- a/appsec/tests/integration/src/main/groovy/com/datadog/appsec/php/TelemetryHelpers.groovy +++ b/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. + * + *

{@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 drainMetricSeries(AppSecContainer container, String service, int timeoutInMs = 500) { + List 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 series diff --git a/appsec/tests/integration/src/test/groovy/com/datadog/appsec/php/integration/TelemetryBackgroundSenderShutdownTests.groovy b/appsec/tests/integration/src/test/groovy/com/datadog/appsec/php/integration/TelemetryBackgroundSenderShutdownTests.groovy new file mode 100644 index 0000000000..829128b19e --- /dev/null +++ b/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 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 + } +} diff --git a/appsec/tests/integration/src/test/groovy/com/datadog/appsec/php/integration/TelemetryBackgroundSenderTests.groovy b/appsec/tests/integration/src/test/groovy/com/datadog/appsec/php/integration/TelemetryBackgroundSenderTests.groovy new file mode 100644 index 0000000000..c56283071d --- /dev/null +++ b/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 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 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 + } +} diff --git a/libdatadog b/libdatadog index 43156bbe53..f2010b616f 160000 --- a/libdatadog +++ b/libdatadog @@ -1 +1 @@ -Subproject commit 43156bbe53c026fdeeaeb3777cb9d4054507a250 +Subproject commit f2010b616feac15557a2b07e6ffd8c1899c69c63 diff --git a/tracer/ddtrace.c b/tracer/ddtrace.c index 60181d437f..2192b33406 100644 --- a/tracer/ddtrace.c +++ b/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(); diff --git a/tracer/tracer_telemetry.c b/tracer/tracer_telemetry.c index 097b755b7e..10a79c563e 100644 --- a/tracer/tracer_telemetry.c +++ b/tracer/tracer_telemetry.c @@ -19,7 +19,19 @@ ZEND_EXTERN_MODULE_GLOBALS(datadog); zend_long dd_composer_hook_id; -ddog_QueueId dd_bgs_queued_id; + +static ddog_QueueId dd_bgs_queue_id; + +// Whether the sidecar has been told about the BGS application. Process-wide, like the +// application itself: it is keyed by (session id, runtime id, queue id), all three of which are +// process-wide (ext/sidecar.c), so a single announcement over any thread's connection is visible +// to every other thread's flush. A reconnect clears it (ddtrace_telemetry_register_services) and +// the next finalize announces it again, over whichever connection gets there first. +// uint64_t rather than bool because the Windows atomics polyfill only has 64-bit intrinsics +// (components/atomic_win32_polyfill.h). +static _Atomic(uint64_t) dd_bgs_application_registered = 0; + +static void dd_bgs_register_application(void); const char *ddtrace_telemetry_redact_file(const char *file) { #ifdef _WIN32 @@ -109,6 +121,7 @@ void ddtrace_telemetry_finalize(void) { ddog_sidecar_telemetry_add_span_metric_point_buffer(buffer, DDOG_CHARSLICE_C("context_header_style.malformed"), DDTRACE_G(baggage_malformed_count), DDOG_CHARSLICE_C("header_style:baggage")); // Flush any accumulated BGS (background sender) metrics if enough time has passed. + dd_bgs_register_application(); ddtrace_telemetry_flush_bgs_metrics_if_due(DATADOG_GLOBALS_PTR()); } @@ -128,18 +141,41 @@ void ddtrace_telemetry_rshutdown(void) { } void ddtrace_telemetry_register_services(ddog_SidecarTransport **sidecar) { - if (!dd_bgs_queued_id) { - dd_bgs_queued_id = ddog_sidecar_queueId_generate(); - } - ddog_sidecar_telemetry_register_metric(sidecar, DDOG_CHARSLICE_C("trace_api.requests"), DDOG_METRIC_TYPE_COUNT, DDOG_METRIC_NAMESPACE_TRACERS); ddog_sidecar_telemetry_register_metric(sidecar, DDOG_CHARSLICE_C("trace_api.responses"), DDOG_METRIC_TYPE_COUNT, DDOG_METRIC_NAMESPACE_TRACERS); ddog_sidecar_telemetry_register_metric(sidecar, DDOG_CHARSLICE_C("trace_api.errors"), DDOG_METRIC_TYPE_COUNT, DDOG_METRIC_NAMESPACE_TRACERS); - // FIXME: it seems we must call "enqueue_actions" (even with an empty list of actions) for things to work properly - ddog_SidecarActionsBuffer *buffer = ddog_sidecar_telemetry_buffer_alloc(); - datadog_ffi_try("Failed flushing background sender telemetry buffer", - ddog_sidecar_telemetry_buffer_flush(sidecar, datadog_sidecar_instance_id, &dd_bgs_queued_id, buffer)); + // this function is in fact a sidecar post-connect callback, + // so the application has been forgotten + atomic_store(&dd_bgs_application_registered, 0); +} + +// must be called before dd_bgs_register_application() +void ddtrace_telemetry_bgs_init(void) { + dd_bgs_queue_id = ddog_sidecar_queueId_generate(); +} + +// Gives the BGS queue an application, without which the sidecar has no service/env to attribute +// its metrics to and drops them ("No application found"). The service/env pair is synthetic and +// fixed: these counters describe the sender, not the application being traced, and this is the +// pair they have always been reported under (up to 1.10.0), so existing queries keep working. +static void dd_bgs_register_application(void) { + if (!dd_bgs_queue_id || !DATADOG_G(sidecar) || atomic_load(&dd_bgs_application_registered)) { + return; + } + + // Only consider it registered if the sidecar took it. Flushing against an application that + // does not exist is not merely a no-op: the flush drains the counters with atomic_exchange() + // before handing them over, so every subsequent interval would zero them and throw them away. + atomic_store(&dd_bgs_application_registered, + datadog_ffi_try("Failed registering background sender application", + ddog_sidecar_set_universal_service_tags(&DATADOG_G(sidecar), datadog_sidecar_instance_id, &dd_bgs_queue_id, + DDOG_CHARSLICE_C("background_sender-php-service"), + DDOG_CHARSLICE_C("none"), + DDOG_CHARSLICE_C(""), // no app version, as before + &DATADOG_G(active_global_tags), + DDOG_DYNAMIC_INSTRUMENTATION_CONFIG_STATE_DISABLED, + UINT64_MAX))); } void ddtrace_telemetry_notify_integration(const char *name, size_t name_len) { @@ -219,6 +255,10 @@ void ddtrace_telemetry_flush_bgs_metrics_if_due(zend_datadog_globals *datadog_gl if (!datadog_globals->sidecar || !get_global_DD_INSTRUMENTATION_TELEMETRY_ENABLED()) { return; } + if (!atomic_load(&dd_bgs_application_registered)) { + // The sidecar would have nowhere to file these metrics; keep accumulating instead. + return; + } // Rate-limit: flush at most once per agent flush interval. uint64_t now_ns = ddtrace_nanoseconds_realtime(); @@ -267,13 +307,24 @@ void ddtrace_telemetry_flush_bgs_metrics_if_due(zend_datadog_globals *datadog_gl } datadog_ffi_try("Failed flushing background sender metrics", - ddog_sidecar_telemetry_buffer_flush(&datadog_globals->sidecar, datadog_sidecar_instance_id, &dd_bgs_queued_id, buffer)); + ddog_sidecar_telemetry_buffer_flush(&datadog_globals->sidecar, datadog_sidecar_instance_id, &dd_bgs_queue_id, buffer)); } -void ddtrace_telemetry_flush_bgs_metrics_final(zend_datadog_globals *datadog_globals) { - // Bypass the time gate so any remaining metrics are sent before the transport - // is dropped in GSHUTDOWN. Setting last_flush_ns to 0 makes the time check in - // _if_due always pass; the CAS inside still prevents a concurrent double-flush. +void ddtrace_telemetry_flush_bgs_metrics_final(void) { + if (!atomic_load(&dd_bgs_application_registered) || !DATADOG_G(sidecar) || !datadog_sidecar_instance_id) { + return; + } + + // Bypass the time gate so any remaining metrics are sent while there is still a connection to + // send them on. Setting last_flush_ns to 0 makes the time check in _if_due always pass; the + // CAS inside still prevents a concurrent double-flush. atomic_store(&bgs_metrics_last_flush_ns, 0); - ddtrace_telemetry_flush_bgs_metrics_if_due(datadog_globals); + ddtrace_telemetry_flush_bgs_metrics_if_due(DATADOG_GLOBALS_PTR()); + + if (!atomic_exchange(&dd_bgs_application_registered, 0)) { + return; + } + + datadog_ffi_try("Failed removing background sender application", + ddog_sidecar_application_remove(&DATADOG_G(sidecar), datadog_sidecar_instance_id, &dd_bgs_queue_id)); } diff --git a/tracer/tracer_telemetry.h b/tracer/tracer_telemetry.h index 3ad266ad1f..95594d391e 100644 --- a/tracer/tracer_telemetry.h +++ b/tracer/tracer_telemetry.h @@ -12,14 +12,18 @@ void ddtrace_telemetry_notify_integration(const char *name, size_t name_len); void ddtrace_telemetry_notify_integration_version(const char *name, size_t name_len, const char *version, size_t version_len); void ddtrace_telemetry_inc_spans_created(ddtrace_span_data *span); +// Generates the queue id the BGS metrics are reported on. Call once per process, before any +// request thread runs, and only when the background sender is in use. +void ddtrace_telemetry_bgs_init(void); // Called by the background sender thread (coms.c) to accumulate metrics atomically. // Never touches the sidecar; the request thread flushes via ddtrace_telemetry_flush_bgs_metrics_if_due(). void ddtrace_telemetry_send_trace_api_metrics(trace_api_metrics metrics); // Called from datadog_telemetry_finalize() to flush accumulated BGS metrics through // the current thread's sidecar connection, at most once per flush interval. void ddtrace_telemetry_flush_bgs_metrics_if_due(zend_datadog_globals *datadog_globals); -// Force-flush accumulated BGS metrics regardless of the time gate. Call immediately -// before dropping the per-thread transport in GSHUTDOWN so no data is lost. -void ddtrace_telemetry_flush_bgs_metrics_final(zend_datadog_globals *datadog_globals); +// Force-flush accumulated BGS metrics regardless of the time gate and release the application +// they are reported under. Call once per process from MSHUTDOWN, before the sidecar's instance +// id is dropped. +void ddtrace_telemetry_flush_bgs_metrics_final(void); #endif // DD_TRACER_TELEMETRY_H