-
Notifications
You must be signed in to change notification settings - Fork 166
feat(process_tags): add process_tags to tracing payloads #3566
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
Co-Authored-By: PROFeNoM <alexandre.choura@datadoghq.com>
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3566 +/- ##
=======================================
Coverage 62.01% 62.01%
=======================================
Files 140 140
Lines 13309 13309
Branches 1762 1762
=======================================
Hits 8253 8253
Misses 4268 4268
Partials 788 788 Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
ext/process_tags.c
Outdated
| process_tag_entry_t* bigger_list = realloc( | ||
| process_tags.tag_list, | ||
| process_tags.capacity * sizeof(process_tag_entry_t) | ||
| ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| process_tag_entry_t* bigger_list = realloc( | |
| process_tags.tag_list, | |
| process_tags.capacity * sizeof(process_tag_entry_t) | |
| ); | |
| process_tag_entry_t* bigger_list = perealloc( | |
| process_tags.tag_list, | |
| process_tags.capacity * sizeof(process_tag_entry_t), | |
| 1 | |
| ); |
Also at other places, use pemalloc(, 1), and pefree(, 1). (They ultimately call system malloc functions, but defer to PHPs handling for OOM.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Then you can remove the if (!bigger_list) completely.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
addressed in 127c555
|
|
||
| } | ||
|
|
||
| static void collect_process_tags(void) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is known and acknowledged, that for web requests, where multiple web applications are hosted on a single web server, or have multiple entrypoints, the values will always reflect the first request after process start (which may be distinct between different processes of the same webserver in multiprocessing mode - which is e.g. the default for FPM), given that they're collected in first rinit?
If yes, then it's fine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
addressed in 127c555
e6cb061 to
3a8038c
Compare
Description
This PR implements this RFC for the tracing product.
Add process_tags in APM payload. According to the RFC, the tag
_dd.tags.processis set in the first first span of each payload.The set tag are:
entrypoint.workdir. The working dir name, should be the last path segment.entrypoint.name. The entrypoint of the application. It is the the script name. Set to none if php is executed as an executable.entrypoint.type. Eitherscriptorexecutableentrypoint.basedir. The base dir is the directory where the called executable resides. Should be the last path segment.runtime.sapi. The name of the SAPI in which PHP is run.Reviewer checklist