Skip to content

Commit 96e9428

Browse files
bwoebicataphractclaude
authored
Batch endpoint collection & remove Wordpress Endpoint collection (#3764)
* Batch endpoint collection Signed-off-by: Bob Weinand <[email protected]> * Avoid duplicate Symfony endpoint collections & don't collect every page on wordpress Batch ack sending too Signed-off-by: Bob Weinand <[email protected]> * Match SHM cache Integration type with sidecar writer The sidecar's write_shm_file() serializes buffered_integrations as HashSet<Integration>, but the PHP-side reader in components-rs deserialized it as HashSet<String>. This mismatch has existed since the SHM cache was introduced in 91222ad ("feat: reduce telemetry sent", #3316), but was latent: config_sent was set true before any integrations appeared in SHM, and integration dedup (the only other consumer) was silently broken with the sidecar deduplicating server-side as a fallback. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * Use a proper struct for Telemetry SHM data Signed-off-by: Bob Weinand <[email protected]> * Update Makefile Signed-off-by: Bob Weinand <[email protected]> --------- Signed-off-by: Bob Weinand <[email protected]> Co-authored-by: Gustavo André dos Santos Lopes <[email protected]> Co-authored-by: Claude Opus 4.6 (1M context) <[email protected]>
1 parent 7fa4e0b commit 96e9428

19 files changed

Lines changed: 227 additions & 383 deletions

File tree

Cargo.lock

Lines changed: 30 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Makefile

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ RUN_TESTS_CMD := DD_SERVICE= DD_ENV= REPORT_EXIT_STATUS=1 TEST_PHP_SRCDIR=$(PROJ
4848

4949
C_FILES = $(shell find components components-rs ext src/dogstatsd zend_abstract_interface -name '*.c' -o -name '*.h' | awk '{ printf "$(BUILD_DIR)/%s\n", $$1 }' )
5050
TEST_FILES = $(shell find tests/ext -name '*.php*' -o -name '*.inc' -o -name '*.json' -o -name '*.yaml' -o -name 'CONFLICTS' | awk '{ printf "$(BUILD_DIR)/%s\n", $$1 }' )
51-
RUST_FILES = $(BUILD_DIR)/Cargo.toml $(BUILD_DIR)/Cargo.lock $(shell find components-rs -name '*.c' -o -name '*.rs' -o -name 'Cargo.toml' | awk '{ printf "$(BUILD_DIR)/%s\n", $$1 }' ) $(shell find libdatadog/{build-common,datadog-ffe,datadog-ipc,datadog-ipc-macros,datadog-live-debugger,datadog-live-debugger-ffi,datadog-remote-config,datadog-sidecar,datadog-sidecar-ffi,datadog-sidecar-macros,libdd-alloc,libdd-common,libdd-common-ffi,libdd-crashtracker,libdd-crashtracker-ffi,libdd-data-pipeline,libdd-ddsketch,libdd-dogstatsd-client,libdd-library-config,libdd-library-config-ffi,libdd-log,libdd-libunwind-sys,libdd-telemetry,libdd-telemetry-ffi,libdd-tinybytes,libdd-trace-*,spawn_worker,tools/{cc_utils,sidecar_mockgen},libdd-trace-*,Cargo.toml} \( -type l -o -type f \) \( -path "*/src*" -o -path "*/examples*" -o -path "*/libunwind*" -o -path "*Cargo.toml" -o -path "*/build.rs" -o -path "*/tests/dataservice.rs" -o -path "*/tests/service_functional.rs" \) -not -path "*/datadog-ipc/build.rs" -not -path "*/datadog-sidecar-ffi/build.rs")
51+
RUST_FILES = $(BUILD_DIR)/Cargo.toml $(BUILD_DIR)/Cargo.lock $(shell find components-rs -name '*.c' -o -name '*.rs' -o -name 'Cargo.toml' | awk '{ printf "$(BUILD_DIR)/%s\n", $$1 }' ) $(shell find libdatadog/{build-common,datadog-ffe,datadog-ipc,datadog-ipc-macros,datadog-live-debugger,datadog-live-debugger-ffi,datadog-remote-config,datadog-sidecar,datadog-sidecar-ffi,datadog-sidecar-macros,libdd-alloc,libdd-capabilities,libdd-capabilities-impl,libdd-common,libdd-common-ffi,libdd-crashtracker,libdd-crashtracker-ffi,libdd-data-pipeline,libdd-ddsketch,libdd-dogstatsd-client,libdd-library-config,libdd-library-config-ffi,libdd-log,libdd-libunwind-sys,libdd-telemetry,libdd-telemetry-ffi,libdd-tinybytes,libdd-trace-*,spawn_worker,tools/{cc_utils,sidecar_mockgen},libdd-trace-*,Cargo.toml} \( -type l -o -type f \) \( -path "*/src*" -o -path "*/examples*" -o -path "*/libunwind*" -o -path "*Cargo.toml" -o -path "*/build.rs" -o -path "*/tests/dataservice.rs" -o -path "*/tests/service_functional.rs" \) -not -path "*/datadog-ipc/build.rs" -not -path "*/datadog-sidecar-ffi/build.rs")
5252
ALL_OBJECT_FILES = $(C_FILES) $(RUST_FILES) $(BUILD_DIR)/Makefile
5353
TEST_OPCACHE_FILES = $(shell find tests/opcache -name '*.php*' -o -name '.gitkeep' | awk '{ printf "$(BUILD_DIR)/%s\n", $$1 }' )
5454
TEST_STUB_FILES = $(shell find tests/ext -type d -name 'stubs' -exec find '{}' -type f \; | awk '{ printf "$(BUILD_DIR)/%s\n", $$1 }' )

appsec/tests/integration/src/test/www/base/public/endpoint_collection.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
echo "are_endpoints_collected before: " . (\DDTrace\are_endpoints_collected() ? 'true' : 'false') . "\n";
44

55
\DDTrace\add_endpoint('/test_random_endpoint', 'http.request', 'GET /test_random_endpoint', 'GET');
6+
\DDTrace\flush_endpoints();
67

78
sleep(1);
89

components-rs/common.h

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -313,6 +313,19 @@ typedef enum ddog_Log {
313313
DDOG_LOG_HOOK_TRACE = (5 | (4 << 4)),
314314
} ddog_Log;
315315

316+
typedef enum ddog_Method {
317+
DDOG_METHOD_GET = 0,
318+
DDOG_METHOD_POST = 1,
319+
DDOG_METHOD_PUT = 2,
320+
DDOG_METHOD_DELETE = 3,
321+
DDOG_METHOD_PATCH = 4,
322+
DDOG_METHOD_HEAD = 5,
323+
DDOG_METHOD_OPTIONS = 6,
324+
DDOG_METHOD_TRACE = 7,
325+
DDOG_METHOD_CONNECT = 8,
326+
DDOG_METHOD_OTHER = 9,
327+
} ddog_Method;
328+
316329
typedef enum ddog_MetricKind {
317330
DDOG_METRIC_KIND_COUNT,
318331
DDOG_METRIC_KIND_GAUGE,
@@ -1007,19 +1020,6 @@ typedef enum ddog_DynamicInstrumentationConfigState {
10071020
DDOG_DYNAMIC_INSTRUMENTATION_CONFIG_STATE_NOT_SET,
10081021
} ddog_DynamicInstrumentationConfigState;
10091022

1010-
typedef enum ddog_Method {
1011-
DDOG_METHOD_GET = 0,
1012-
DDOG_METHOD_POST = 1,
1013-
DDOG_METHOD_PUT = 2,
1014-
DDOG_METHOD_DELETE = 3,
1015-
DDOG_METHOD_PATCH = 4,
1016-
DDOG_METHOD_HEAD = 5,
1017-
DDOG_METHOD_OPTIONS = 6,
1018-
DDOG_METHOD_TRACE = 7,
1019-
DDOG_METHOD_CONNECT = 8,
1020-
DDOG_METHOD_OTHER = 9,
1021-
} ddog_Method;
1022-
10231023
typedef struct ddog_AgentInfoReader ddog_AgentInfoReader;
10241024

10251025
typedef struct ddog_AgentRemoteConfigReader ddog_AgentRemoteConfigReader;

components-rs/ddtrace.h

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,15 @@ void ddog_sidecar_telemetry_addDependency_buffer(struct ddog_SidecarActionsBuffe
153153
ddog_CharSlice dependency_name,
154154
ddog_CharSlice dependency_version);
155155

156+
/**
157+
* Enqueues an endpoint into a telemetry actions buffer (to be sent via ddog_sidecar_telemetry_buffer_flush).
158+
*/
159+
void ddog_sidecar_telemetry_addEndpoint_buffer(struct ddog_SidecarActionsBuffer *buffer,
160+
enum ddog_Method method,
161+
ddog_CharSlice path,
162+
ddog_CharSlice operation_name,
163+
ddog_CharSlice resource_name);
164+
156165
void ddog_sidecar_telemetry_enqueueConfig_buffer(struct ddog_SidecarActionsBuffer *buffer,
157166
ddog_CharSlice config_key,
158167
ddog_CharSlice config_value,

components-rs/telemetry.rs

Lines changed: 29 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,10 @@
11
use crate::log::Log;
2-
use datadog_sidecar::service::telemetry::path_for_telemetry;
2+
use datadog_sidecar::service::telemetry::{path_for_telemetry, TelemetryCachedClientShmData};
33

44
use hashbrown::{Equivalent, HashMap};
5-
use std::collections::HashSet;
65
use std::ffi::CString;
76
use std::path::PathBuf;
8-
use std::time::{Duration, SystemTime};
7+
use std::time::Duration;
98

109
use datadog_ipc::platform::NamedShmHandle;
1110
use datadog_sidecar::one_way_shared_memory::{open_named_shm, OneWayShmReader};
@@ -126,6 +125,24 @@ pub unsafe extern "C" fn ddog_sidecar_telemetry_addDependency_buffer(
126125
buffer.buffer.push(SidecarAction::Telemetry(action));
127126
}
128127

128+
/// Enqueues an endpoint into a telemetry actions buffer (to be sent via ddog_sidecar_telemetry_buffer_flush).
129+
#[no_mangle]
130+
pub unsafe extern "C" fn ddog_sidecar_telemetry_addEndpoint_buffer(
131+
buffer: &mut SidecarActionsBuffer,
132+
method: data::Method,
133+
path: CharSlice,
134+
operation_name: CharSlice,
135+
resource_name: CharSlice,
136+
) {
137+
let action = TelemetryActions::AddEndpoint(data::Endpoint {
138+
method: Some(method),
139+
path: Some(path.to_utf8_lossy().into_owned()),
140+
operation_name: operation_name.to_utf8_lossy().into_owned(),
141+
resource_name: resource_name.to_utf8_lossy().into_owned(),
142+
});
143+
buffer.buffer.push(SidecarAction::Telemetry(action));
144+
}
145+
129146
#[no_mangle]
130147
pub unsafe extern "C" fn ddog_sidecar_telemetry_enqueueConfig_buffer(
131148
buffer: &mut SidecarActionsBuffer,
@@ -237,10 +254,7 @@ pub unsafe extern "C" fn ddog_sidecar_telemetry_add_integration_log_buffer(
237254
}
238255

239256
pub struct ShmCache {
240-
pub config_sent: bool,
241-
pub integrations: HashSet<String>,
242-
pub composer_paths: HashSet<PathBuf>,
243-
pub last_endpoints_push: SystemTime,
257+
pub shared: TelemetryCachedClientShmData,
244258
pub reader: OneWayShmReader<NamedShmHandle, CString>,
245259
}
246260

@@ -269,7 +283,7 @@ pub unsafe extern "C" fn ddog_sidecar_telemetry_config_sent(
269283
service: CharSlice,
270284
env: CharSlice,
271285
) -> bool {
272-
ddog_sidecar_telemetry_cache_get_or_update(cache, service, env).config_sent
286+
ddog_sidecar_telemetry_cache_get_or_update(cache, service, env).shared.config_sent
273287
}
274288

275289
unsafe fn ddog_sidecar_telemetry_cache_get_or_update<'a>(
@@ -287,21 +301,13 @@ unsafe fn ddog_sidecar_telemetry_cache_get_or_update<'a>(
287301
if changed {
288302
buf = newbuf;
289303
} else {
290-
cache.config_sent = false;
291-
cache.integrations.clear();
292-
cache.composer_paths.clear();
293-
cache.last_endpoints_push = SystemTime::UNIX_EPOCH;
304+
cache.shared = TelemetryCachedClientShmData::default();
294305
return;
295306
}
296307
}
297308

298-
if let Ok((config_sent, integrations, composer_paths, last_endpoints_push)) =
299-
bincode::deserialize::<(bool, HashSet<String>, HashSet<PathBuf>, SystemTime)>(buf)
300-
{
301-
cache.config_sent = config_sent;
302-
cache.integrations = integrations;
303-
cache.composer_paths = composer_paths;
304-
cache.last_endpoints_push = last_endpoints_push;
309+
if let Ok(shared) = bincode::deserialize::<TelemetryCachedClientShmData>(buf) {
310+
cache.shared = shared;
305311
}
306312
}
307313
}
@@ -319,10 +325,7 @@ unsafe fn ddog_sidecar_telemetry_cache_get_or_update<'a>(
319325
let reader = OneWayShmReader::<NamedShmHandle, _>::new(open_named_shm(&shm_path).ok(), shm_path);
320326
let cached_entry = cache.entry(ShmCacheKey(service_str.into(), env_str.into())).insert(ShmCache {
321327
reader,
322-
config_sent: false,
323-
integrations: HashSet::new(),
324-
composer_paths: HashSet::new(),
325-
last_endpoints_push: SystemTime::UNIX_EPOCH,
328+
shared: TelemetryCachedClientShmData::default(),
326329
}).into_mut();
327330

328331
refresh_cache(cached_entry);
@@ -347,10 +350,10 @@ pub unsafe extern "C" fn ddog_sidecar_telemetry_filter_flush(
347350
.into_iter()
348351
.filter(|action| match action {
349352
SidecarAction::Telemetry(TelemetryActions::AddIntegration(integration)) => {
350-
!cache_entry.integrations.contains(&integration.name)
353+
!cache_entry.shared.integrations.contains(integration)
351354
}
352355
SidecarAction::PhpComposerTelemetryFile(path) => {
353-
!cache_entry.composer_paths.contains(path)
356+
!cache_entry.shared.composer_paths.contains(path)
354357
}
355358
_ => true,
356359
})
@@ -374,6 +377,5 @@ pub unsafe extern "C" fn ddog_sidecar_telemetry_are_endpoints_collected(
374377
env: CharSlice,
375378
) -> bool {
376379
let cache_entry = ddog_sidecar_telemetry_cache_get_or_update(cache, service, env);
377-
let result = cache_entry.last_endpoints_push.elapsed().map_or(false, |d| d < Duration::from_secs(1800)); // 30 minutes
378-
result
380+
cache_entry.shared.last_endpoints_push.elapsed().map_or(false, |d| d < Duration::from_secs(1800)) // 30 minutes
379381
}

ext/ddtrace.c

Lines changed: 25 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2811,6 +2811,10 @@ PHP_FUNCTION(DDTrace_dogstatsd_set) {
28112811
PHP_FUNCTION(DDTrace_are_endpoints_collected) {
28122812
UNUSED(execute_data);
28132813

2814+
if (!ddtrace_sidecar || !ddtrace_sidecar_instance_id || !DDTRACE_G(sidecar_queue_id)) {
2815+
RETURN_TRUE; // Skip overhead if unnecessary
2816+
}
2817+
28142818
if (!DDTRACE_G(last_service_name) || !DDTRACE_G(last_env_name)) {
28152819
RETURN_FALSE;
28162820
}
@@ -2872,25 +2876,35 @@ PHP_FUNCTION(DDTrace_add_endpoint) {
28722876
ddog_CharSlice operation_name_slice = dd_zend_string_to_CharSlice(operation_name);
28732877
ddog_CharSlice resource_name_slice = dd_zend_string_to_CharSlice(resource_name);
28742878

2875-
ddog_MaybeError result = ddog_sidecar_telemetry_addEndpoint(
2876-
&ddtrace_sidecar, ddtrace_sidecar_instance_id, &DDTRACE_G(sidecar_queue_id), method_enum, path_slice, operation_name_slice,
2877-
resource_name_slice);
2878-
28792879
LOG_LINE(DEBUG,
28802880
"Adding endpoint: %.*s (%zu) - operation_name: %.*s (%zu) - resource_name: %.*s (%zu) - method: %.*s (%zu)",
28812881
(int)path_slice.len, (char *)path_slice.ptr, path_slice.len,
28822882
(int)operation_name_slice.len, (char *)operation_name_slice.ptr, operation_name_slice.len,
28832883
(int)resource_name_slice.len, (char *)resource_name_slice.ptr, resource_name_slice.len,
28842884
(int)method->len, ZSTR_VAL(method), (size_t)method->len);
28852885

2886-
if (result.tag == DDOG_OPTION_ERROR_SOME_ERROR) {
2887-
ddog_CharSlice message = ddog_Error_message(&result.some);
2888-
LOG_LINE(ERROR, "Error submitting endpoint to sidecar: %.*s", (int)message.len, (char *)message.ptr);
2889-
ZVAL_FALSE(return_value);
2890-
} else {
2891-
ZVAL_TRUE(return_value);
2886+
ddog_sidecar_telemetry_addEndpoint_buffer(ddtrace_telemetry_buffer(), method_enum, path_slice, operation_name_slice, resource_name_slice);
2887+
2888+
RETURN_TRUE;
2889+
}
2890+
2891+
PHP_FUNCTION(DDTrace_flush_endpoints) {
2892+
UNUSED(execute_data);
2893+
UNUSED(return_value);
2894+
2895+
if (!ddtrace_sidecar || !ddtrace_sidecar_instance_id || !DDTRACE_G(sidecar_queue_id) || !DDTRACE_G(telemetry_buffer)) {
2896+
return;
2897+
}
2898+
2899+
if (!DDTRACE_G(last_service_name) || !DDTRACE_G(last_env_name)) {
2900+
return;
28922901
}
2893-
ddog_MaybeError_drop(result);
2902+
2903+
ddog_CharSlice service_name = dd_zend_string_to_CharSlice(DDTRACE_G(last_service_name));
2904+
ddog_CharSlice env_name = dd_zend_string_to_CharSlice(DDTRACE_G(last_env_name));
2905+
2906+
ddtrace_ffi_try("Failed flushing endpoint telemetry buffer",
2907+
ddog_sidecar_telemetry_filter_flush(&ddtrace_sidecar, ddtrace_sidecar_instance_id, &DDTRACE_G(sidecar_queue_id), ddtrace_telemetry_buffer(), ddtrace_telemetry_cache(), service_name, env_name));
28942908
}
28952909

28962910
PHP_FUNCTION(dd_trace_send_traces_via_thread) {

ext/ddtrace.stub.php

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -839,6 +839,12 @@ function are_endpoints_collected(): bool {}
839839
* @param string $method The method of the endpoint
840840
*/
841841
function add_endpoint(string $path, string $operation_name, string $resource_name, string $method): bool {}
842+
843+
/**
844+
* Flush all buffered endpoints to the sidecar immediately.
845+
* Call this once after batching all add_endpoint() calls.
846+
*/
847+
function flush_endpoints(): void {}
842848
}
843849

844850
namespace DDTrace\System {

0 commit comments

Comments
 (0)