Skip to content

Allow dumping shm data directly for inspection#3829

Open
bwoebi wants to merge 1 commit intomasterfrom
bob/shm_dump
Open

Allow dumping shm data directly for inspection#3829
bwoebi wants to merge 1 commit intomasterfrom
bob/shm_dump

Conversation

@bwoebi
Copy link
Copy Markdown
Collaborator

@bwoebi bwoebi commented Apr 24, 2026

This allows directly inspecting and analyzing the contents of data stored in SHM for debugging/testing purposes.

@bwoebi bwoebi requested review from a team as code owners April 24, 2026 16:08
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 196e70c2f3

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread ext/ddtrace.c Outdated
Comment on lines +3151 to +3159
if (rc_json && rc_json[0]) {
zend_string *json_str = zend_string_init(rc_json, strlen(rc_json), 0);
if (zai_json_decode_assoc_safe(return_value, ZSTR_VAL(json_str), (int)ZSTR_LEN(json_str), 128, false) != SUCCESS) {
array_init(return_value);
}
zend_string_release(json_str);
ddog_remote_config_loaded_configs_free(rc_json);
} else {
array_init(return_value);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Free remote-config JSON buffer even when payload is empty

When no remote configs are loaded, ddog_remote_config_get_loaded_configs() still returns an allocated empty C string (""), but this branch initializes an empty PHP array without calling ddog_remote_config_loaded_configs_free(). In polling/debug scenarios (including this test pattern), repeated calls leak one Rust-allocated buffer per call. Please free rc_json whenever it is non-null, regardless of whether rc_json[0] is set.

Useful? React with 👍 / 👎.

Comment thread ext/ddtrace.c Outdated
Comment on lines +3169 to +3177
if (info_json && info_json[0]) {
zend_string *json_str = zend_string_init(info_json, strlen(info_json), 0);
if (zai_json_decode_assoc_safe(return_value, ZSTR_VAL(json_str), (int)ZSTR_LEN(json_str), 128, false) != SUCCESS) {
array_init(return_value);
}
zend_string_release(json_str);
ddog_agent_info_json_free(info_json);
} else {
array_init(return_value);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Release agent-info JSON buffer on empty result path

The same ownership issue appears in the agent-info helper: ddog_agent_info_as_json() allocates and returns an empty C string when no info is available, but the else path here skips ddog_agent_info_json_free(). If callers invoke dd_trace_internal_fn('get_agent_info') before SHM data is populated, this leaks memory on each call.

Useful? React with 👍 / 👎.

@datadog-prod-us1-4
Copy link
Copy Markdown

datadog-prod-us1-4 Bot commented Apr 24, 2026

Tests

🎉 All green!

❄️ No new flaky tests detected
🧪 All tests passed

🎯 Code Coverage (details)
Patch Coverage: 100.00%
Overall Coverage: 60.68% (+0.00%)

This comment will be updated automatically if new data arrives.
🔗 Commit SHA: faa165b | Docs | Datadog PR Page | Give us feedback!

@pr-commenter
Copy link
Copy Markdown

pr-commenter Bot commented Apr 24, 2026

Benchmarks [ tracer ]

Benchmark execution time: 2026-04-24 18:24:36

Comparing candidate commit faa165b in PR branch bob/shm_dump with baseline commit b7eb412 in branch master.

Found 3 performance improvements and 0 performance regressions! Performance is the same for 191 metrics, 0 unstable metrics.

scenario:ComposerTelemetryBench/benchTelemetryParsing-opcache

  • 🟩 execution_time [-1.906µs; -0.894µs] or [-11.346%; -5.321%]

scenario:MessagePackSerializationBench/benchMessagePackSerialization

  • 🟩 execution_time [-13.454µs; -11.646µs] or [-11.500%; -9.955%]

scenario:MessagePackSerializationBench/benchMessagePackSerialization-opcache

  • 🟩 execution_time [-12.635µs; -11.685µs] or [-11.064%; -10.232%]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant