Implement new SSI configuration telemetry #3301

iamluc · 2025-06-17T15:57:46Z

Description

Implement https://docs.google.com/document/d/1wydXj2fW0V0jaWU6W9pIbipT_AXD15tmbTtOPXFBEy4/edit?tab=t.nun42691l9k2 for PHP

https://datadoghq.atlassian.net/browse/INPLAT-577

System-tests PR: DataDog/system-tests#4797

Reviewer checklist

Test coverage seems ok.
Appropriate labels assigned.

pr-commenter · 2025-06-17T16:27:02Z

Benchmarks [ tracer ]

Benchmark execution time: 2025-07-21 16:24:12

Comparing candidate commit c525ae0 in PR branch luc/ssi-config-telemetry with baseline commit 779da23 in branch master.

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

scenario:PDOBench/benchPDOBaseline-opcache

🟩 execution_time [-5.819µs; -5.458µs] or [-8.686%; -8.148%]

bwoebi · 2025-06-18T06:49:09Z

loader/dd_library_loader.c

+    }
+    char **ddtrace_ssi_forced_injection_enabled = (char **)DL_FETCH_SYMBOL(module->handle, "ddtrace_ssi_forced_injection_enabled");
+    if (ddtrace_ssi_forced_injection_enabled) {
+        *ddtrace_ssi_forced_injection_enabled = getenv("DD_INJECTION_FORCE");


Suggested change

*ddtrace_ssi_forced_injection_enabled = getenv("DD_INJECTION_FORCE");

*ddtrace_ssi_forced_injection_enabled = force_load ? "True" : "False";

At least according to the document it should be only one of those values, not the raw value?

I implemented it the same way as in Ruby: https://github.com/DataDog/dd-trace-rb/pull/4737/files#diff-106f688b0baf1218c65023cab312b14ccff2fbf78f07e09bb9c2dde05da93f95R145-R157

@mabdinur what do you think?

Well, at least it would not report it correctly when force_load is set via ini.

Good catch!

Updated as you suggested

Yup it should ideally be a Boolean but this behavior is not consistent across libraries. The .NET tracer reports raw configuration values to the instrumentation telemetry platform. Python reports the processed values. Other libraries do a mix of the two. This issue should be resolved by the config chaining initiative. Here we should go with the simplest implementation.

The ruby implementation is incomplete. I'll update it later today.

bwoebi · 2025-06-18T06:56:27Z

loader/dd_library_loader.c

+    }
+    char **ddtrace_ssi_injection_enabled = (char **)DL_FETCH_SYMBOL(module->handle, "ddtrace_ssi_injection_enabled");
+    if (ddtrace_ssi_injection_enabled) {
+        *ddtrace_ssi_injection_enabled = getenv("DD_INJECTION_ENABLED");


Stupid question, not related to this PR, but we don't make use of that variable at all currently, right?

You're correct. We don't need it.

Then this configuration feels misleading, I think we only should report what we actually use. We don't use that config, so we should not report it.

cc @mabdinur WDYT? Again, it is similar to what's done in Ruby I think

This is the same approach we also used in Java and .NET. It was used to simplify the implementation and testing. These fields could also be used in debug logs or by other products (I believe it's used by the profiler in Python).

I dont have a strong opinion here. As long as we capture the right telemetry names and values this change looks good to me.

codecov-commenter · 2025-07-03T12:27:30Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 61.80%. Comparing base (779da23) to head (c525ae0).
Report is 1 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3301      +/-   ##
==========================================
- Coverage   61.92%   61.80%   -0.12%     
==========================================
  Files         140      140              
  Lines       12356    12356              
  Branches     1616     1616              
==========================================
- Hits         7651     7637      -14     
- Misses       3995     4008      +13     
- Partials      710      711       +1

see 2 files with indirect coverage changes

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 779da23...c525ae0. Read the comment docs.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

bwoebi

All comments addressed, nice :-)

bwoebi reviewed Jun 18, 2025

View reviewed changes

iamluc mentioned this pull request Jun 18, 2025

Enable docker_ssi/test_instrumentation_source_ssi test for PHP DataDog/system-tests#4797

Merged

7 tasks

iamluc marked this pull request as ready for review June 18, 2025 14:51

iamluc requested a review from a team as a code owner June 18, 2025 14:51

iamluc added 2 commits June 20, 2025 16:44

Implement new SSI configuration telemetry

a40964e

Take into account force flag set with PHP ini config

dd19f89

iamluc force-pushed the luc/ssi-config-telemetry branch from 9c29c6c to 33719c5 Compare June 20, 2025 14:44

Don't store getenv pointer as it can become invalid

75d6075

iamluc force-pushed the luc/ssi-config-telemetry branch from 33719c5 to 75d6075 Compare June 20, 2025 14:45

iamluc added 2 commits June 26, 2025 15:15

Merge branch 'master' into luc/ssi-config-telemetry

a2cf5e3

Merge branch 'master' into luc/ssi-config-telemetry

9b4471b

mabdinur and others added 2 commits July 8, 2025 17:43

Merge branch 'master' into luc/ssi-config-telemetry

098f53e

Merge branch 'master' into luc/ssi-config-telemetry

02be80d

mabdinur requested a review from bwoebi July 21, 2025 15:53

Merge branch 'master' into luc/ssi-config-telemetry

c525ae0

mabdinur enabled auto-merge July 21, 2025 15:53

bwoebi approved these changes Jul 21, 2025

View reviewed changes

mabdinur merged commit 8a256df into master Jul 21, 2025
1814 of 1881 checks passed

mabdinur deleted the luc/ssi-config-telemetry branch July 21, 2025 16:38

github-actions bot added this to the 1.11.0 milestone Jul 21, 2025

	*ddtrace_ssi_forced_injection_enabled = getenv("DD_INJECTION_FORCE");
	*ddtrace_ssi_forced_injection_enabled = force_load ? "True" : "False";

Implement new SSI configuration telemetry #3301

Implement new SSI configuration telemetry #3301

Uh oh!

Conversation

iamluc commented Jun 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Reviewer checklist

Uh oh!

pr-commenter bot commented Jun 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Benchmarks [ tracer ]

scenario:PDOBench/benchPDOBaseline-opcache

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov-commenter commented Jul 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

bwoebi left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

iamluc commented Jun 17, 2025 •

edited

Loading

pr-commenter bot commented Jun 17, 2025 •

edited

Loading

codecov-commenter commented Jul 3, 2025 •

edited

Loading