feat(prof): add source code integration#3418
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3418 +/- ##
==========================================
- Coverage 61.90% 61.74% -0.17%
==========================================
Files 141 141
Lines 12481 12481
Branches 1630 1630
==========================================
- Hits 7726 7706 -20
- Misses 4033 4054 +21
+ Partials 722 721 -1 see 3 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
Benchmarks [ profiler ]Benchmark execution time: 2025-09-22 07:14:14 Comparing candidate commit 55850d4 in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 31 metrics, 5 unstable metrics. |
a0b954a to
b642c03
Compare
There was a problem hiding this comment.
This doesn't integrate with the config module, so users cannot use INIs to configure this. That also means our datadog-setup.php config mode won't work with it either.
If we agree it should be done, then add these two variants to the ConfigId enum:
GitCommitSha,
GitRepositoryUrl,Add some functions like:
/// # Safety
/// This function must only be called after config has been initialized in
/// rinit, and before it is uninitialized in mshutdown.
pub(crate) unsafe fn git_commit_sha() -> Option<String> {
get_str(Env)
}
/// # Safety
/// This function must only be called after config has been initialized in
/// rinit, and before it is uninitialized in mshutdown.
pub(crate) unsafe fn git_repository_url() -> Option<String> {
get_str(Env)
}Add it to the ENTRIES array:
zai_config_entry {
id: transmute::<ConfigId, u16>(GitCommitSha),
name: Env.env_var_name(),
type_: ZAI_CONFIG_TYPE_STRING,
default_encoded_value: ZaiStr::new(),
aliases: ptr::null_mut(),
aliases_count: 0,
ini_change: None,
parser: Some(parse_utf8_string),
displayer: None,
env_config_fallback: None,
},
zai_config_entry {
id: transmute::<ConfigId, u16>(GitRepositoryUrl),
name: Env.env_var_name(),
type_: ZAI_CONFIG_TYPE_STRING,
default_encoded_value: ZaiStr::new(),
aliases: ptr::null_mut(),
aliases_count: 0,
ini_change: None,
parser: Some(parse_utf8_string),
displayer: None,
env_config_fallback: None,
},Add it to the env_var_name match:
GitCommitSha => b"DD_GIT_COMMIT_SHA\0",
GitRepositoryUrl => c"DD_GIT_REPOSITORY_URL\0",And add it to the test cases for names:
(b"DD_GIT_COMMIT_SHA\0", "datadog.git_commit_sha"),
(b"DD_GIT_REPOSITORY_URL\0", "datadog.git_repository_url"),
morrisonlevi
left a comment
There was a problem hiding this comment.
Also, I think maybe it should be moved to the part where service, env, and version are handled. Theoretically you could change files and reconfigure, yeah? At least the tracer didn't register this as a system INI. What do you think?
|
I did the changes towards this, especially that it makes the feature symmetric with the tracer |
Description
This adds source code integration feature to the PHP profiler via the environment variables
DD_GIT_COMMIT_SHAandDD_GIT_REPOSITORY_URLor the equivalent INI settingsdatadog.git_commit_shaanddatadog.git_repository_url. You can already use this feature via theDD_TAGSenvironment variable.Reviewer checklist
PROF-12551