feat(tracing): process tags about service name source [APMLP-1052]#16593
feat(tracing): process tags about service name source [APMLP-1052]#16593emmettbutler wants to merge 4 commits intomainfrom
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e6c92b4d54
ℹ️ 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".
| if not config.enabled: | ||
| return None, None | ||
|
|
||
| from ddtrace import config as ddtrace_config |
There was a problem hiding this comment.
Avoid importing
ddtrace.config during process-tags init
When DD_EXPERIMENTAL_PROPAGATE_PROCESS_TAGS_ENABLED=true, generate_process_tags() executes at module import time and hits from ddtrace import config; at that point ddtrace/__init__.py has imported telemetry (which imports process_tags) before it binds config, so this creates a circular import and can raise ImportError: cannot import name 'config' from partially initialized module 'ddtrace', preventing startup whenever process tags are enabled from env.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
I don't think this is true. The test I added proves it.
There was a problem hiding this comment.
So we have a problem because, tests are passing but codex is right, if you run
DD_EXPERIMENTAL_PROPAGATE_PROCESS_TAGS_ENABLED=True ddtrace-run python your_script.py, you indeed have a circular import (that is also why I created a config for process_tags).
This comment has been minimized.
This comment has been minimized.
Performance SLOsComparing candidate emmett.butler/svc-process-tags (e6c92b4) with baseline main (8c9f2b5) 📈 Performance Regressions (2 suites)📈 iastaspects - 118/118✅ add_aspectTime: ✅ 103.405µs (SLO: <130.000µs 📉 -20.5%) vs baseline: +0.3% Memory: ✅ 42.880MB (SLO: <46.000MB -6.8%) vs baseline: +4.1% ✅ add_inplace_aspectTime: ✅ 100.923µs (SLO: <130.000µs 📉 -22.4%) vs baseline: -2.8% Memory: ✅ 42.959MB (SLO: <46.000MB -6.6%) vs baseline: +4.4% ✅ add_inplace_noaspectTime: ✅ 28.520µs (SLO: <40.000µs 📉 -28.7%) vs baseline: +0.5% Memory: ✅ 42.900MB (SLO: <46.000MB -6.7%) vs baseline: +4.7% ✅ add_noaspectTime: ✅ 48.708µs (SLO: <70.000µs 📉 -30.4%) vs baseline: -0.9% Memory: ✅ 42.939MB (SLO: <46.000MB -6.7%) vs baseline: +4.5% ✅ bytearray_aspectTime: ✅ 248.658µs (SLO: <400.000µs 📉 -37.8%) vs baseline: -0.3% Memory: ✅ 42.939MB (SLO: <46.000MB -6.7%) vs baseline: +4.5% ✅ bytearray_extend_aspectTime: ✅ 634.669µs (SLO: <800.000µs 📉 -20.7%) vs baseline: -3.2% Memory: ✅ 42.880MB (SLO: <46.000MB -6.8%) vs baseline: +4.4% ✅ bytearray_extend_noaspectTime: ✅ 266.302µs (SLO: <400.000µs 📉 -33.4%) vs baseline: -1.4% Memory: ✅ 42.959MB (SLO: <46.000MB -6.6%) vs baseline: +4.3% ✅ bytearray_noaspectTime: ✅ 135.391µs (SLO: <300.000µs 📉 -54.9%) vs baseline: -4.0% Memory: ✅ 42.920MB (SLO: <46.000MB -6.7%) vs baseline: +4.5% ✅ bytes_aspectTime: ✅ 217.121µs (SLO: <300.000µs 📉 -27.6%) vs baseline: ~same Memory: ✅ 42.880MB (SLO: <46.000MB -6.8%) vs baseline: +4.3% ✅ bytes_noaspectTime: ✅ 132.609µs (SLO: <200.000µs 📉 -33.7%) vs baseline: -1.2% Memory: ✅ 42.920MB (SLO: <46.000MB -6.7%) vs baseline: +4.5% ✅ bytesio_aspectTime: ✅ 3.774ms (SLO: <5.000ms 📉 -24.5%) vs baseline: -0.6% Memory: ✅ 42.900MB (SLO: <46.000MB -6.7%) vs baseline: +4.4% ✅ bytesio_noaspectTime: ✅ 315.011µs (SLO: <420.000µs 📉 -25.0%) vs baseline: -0.8% Memory: ✅ 42.939MB (SLO: <46.000MB -6.7%) vs baseline: +4.1% ✅ capitalize_aspectTime: ✅ 88.809µs (SLO: <300.000µs 📉 -70.4%) vs baseline: +0.5% Memory: ✅ 42.959MB (SLO: <46.000MB -6.6%) vs baseline: +4.3% ✅ capitalize_noaspectTime: ✅ 250.817µs (SLO: <300.000µs 📉 -16.4%) vs baseline: -1.3% Memory: ✅ 42.998MB (SLO: <46.000MB -6.5%) vs baseline: +4.3% ✅ casefold_aspectTime: ✅ 92.584µs (SLO: <500.000µs 📉 -81.5%) vs baseline: +4.2% Memory: ✅ 42.861MB (SLO: <46.000MB -6.8%) vs baseline: +4.2% ✅ casefold_noaspectTime: ✅ 307.290µs (SLO: <500.000µs 📉 -38.5%) vs baseline: -1.6% Memory: ✅ 42.920MB (SLO: <46.000MB -6.7%) vs baseline: +4.3% ✅ decode_aspectTime: ✅ 86.422µs (SLO: <100.000µs 📉 -13.6%) vs baseline: -0.2% Memory: ✅ 42.880MB (SLO: <46.000MB -6.8%) vs baseline: +4.2% ✅ decode_noaspectTime: ✅ 152.152µs (SLO: <210.000µs 📉 -27.5%) vs baseline: -2.3% Memory: ✅ 42.880MB (SLO: <46.000MB -6.8%) vs baseline: +4.4% ✅ encode_aspectTime: ✅ 84.053µs (SLO: <200.000µs 📉 -58.0%) vs baseline: -0.3% Memory: ✅ 42.900MB (SLO: <46.000MB -6.7%) vs baseline: +4.5% ✅ encode_noaspectTime: ✅ 139.899µs (SLO: <200.000µs 📉 -30.1%) vs baseline: -1.7% Memory: ✅ 42.939MB (SLO: <46.000MB -6.7%) vs baseline: +4.4% ✅ format_aspectTime: ✅ 14.694ms (SLO: <19.200ms 📉 -23.5%) vs baseline: +0.3% Memory: ✅ 42.959MB (SLO: <46.000MB -6.6%) vs baseline: +4.2% ✅ format_map_aspectTime: ✅ 16.402ms (SLO: <21.500ms 📉 -23.7%) vs baseline: ~same Memory: ✅ 43.037MB (SLO: <46.000MB -6.4%) vs baseline: +4.4% ✅ format_map_noaspectTime: ✅ 374.133µs (SLO: <500.000µs 📉 -25.2%) vs baseline: +0.2% Memory: ✅ 42.998MB (SLO: <46.000MB -6.5%) vs baseline: +4.6% ✅ format_noaspectTime: ✅ 301.353µs (SLO: <500.000µs 📉 -39.7%) vs baseline: -1.0% Memory: ✅ 42.900MB (SLO: <46.000MB -6.7%) vs baseline: +4.4% ✅ index_aspectTime: ✅ 121.245µs (SLO: <300.000µs 📉 -59.6%) vs baseline: +1.2% Memory: ✅ 42.979MB (SLO: <46.000MB -6.6%) vs baseline: +4.5% ✅ index_noaspectTime: ✅ 40.114µs (SLO: <300.000µs 📉 -86.6%) vs baseline: ~same Memory: ✅ 42.939MB (SLO: <46.000MB -6.7%) vs baseline: +4.5% ✅ join_aspectTime: ✅ 211.275µs (SLO: <300.000µs 📉 -29.6%) vs baseline: -0.9% Memory: ✅ 42.900MB (SLO: <46.000MB -6.7%) vs baseline: +4.3% ✅ join_noaspectTime: ✅ 141.903µs (SLO: <300.000µs 📉 -52.7%) vs baseline: -1.0% Memory: ✅ 42.880MB (SLO: <46.000MB -6.8%) vs baseline: +4.3% ✅ ljust_aspectTime: ✅ 495.800µs (SLO: <700.000µs 📉 -29.2%) vs baseline: -2.1% Memory: ✅ 42.900MB (SLO: <46.000MB -6.7%) vs baseline: +4.4% ✅ ljust_noaspectTime: ✅ 260.315µs (SLO: <300.000µs 📉 -13.2%) vs baseline: -0.7% Memory: ✅ 42.939MB (SLO: <46.000MB -6.7%) vs baseline: +4.2% ✅ lower_aspectTime: ✅ 293.339µs (SLO: <500.000µs 📉 -41.3%) vs baseline: -2.3% Memory: ✅ 42.920MB (SLO: <46.000MB -6.7%) vs baseline: +4.3% ✅ lower_noaspectTime: ✅ 234.736µs (SLO: <300.000µs 📉 -21.8%) vs baseline: -1.4% Memory: ✅ 42.900MB (SLO: <46.000MB -6.7%) vs baseline: +4.3% ✅ lstrip_aspectTime: ✅ 0.268ms (SLO: <3.000ms 📉 -91.1%) vs baseline: -3.0% Memory: ✅ 42.939MB (SLO: <46.000MB -6.7%) vs baseline: +4.0% ✅ lstrip_noaspectTime: ✅ 0.212ms (SLO: <3.000ms 📉 -92.9%) vs baseline: 📈 +18.9% Memory: ✅ 42.959MB (SLO: <46.000MB -6.6%) vs baseline: +4.4% ✅ modulo_aspectTime: ✅ 14.472ms (SLO: <18.750ms 📉 -22.8%) vs baseline: +1.3% Memory: ✅ 43.077MB (SLO: <46.000MB -6.4%) vs baseline: +4.4% ✅ modulo_aspect_for_bytearray_bytearrayTime: ✅ 14.814ms (SLO: <19.350ms 📉 -23.4%) vs baseline: +0.4% Memory: ✅ 43.096MB (SLO: <46.000MB -6.3%) vs baseline: +4.4% ✅ modulo_aspect_for_bytesTime: ✅ 14.390ms (SLO: <18.900ms 📉 -23.9%) vs baseline: -0.3% Memory: ✅ 43.077MB (SLO: <46.000MB -6.4%) vs baseline: +4.7% ✅ modulo_aspect_for_bytes_bytearrayTime: ✅ 14.619ms (SLO: <19.150ms 📉 -23.7%) vs baseline: +0.2% Memory: ✅ 43.096MB (SLO: <46.000MB -6.3%) vs baseline: +4.6% ✅ modulo_noaspectTime: ✅ 0.358ms (SLO: <3.000ms 📉 -88.1%) vs baseline: -0.8% Memory: ✅ 42.959MB (SLO: <46.000MB -6.6%) vs baseline: +4.6% ✅ replace_aspectTime: ✅ 18.397ms (SLO: <24.000ms 📉 -23.3%) vs baseline: -0.2% Memory: ✅ 42.959MB (SLO: <46.000MB -6.6%) vs baseline: +4.2% ✅ replace_noaspectTime: ✅ 279.236µs (SLO: <300.000µs -6.9%) vs baseline: -1.1% Memory: ✅ 42.959MB (SLO: <46.000MB -6.6%) vs baseline: +4.3% ✅ repr_aspectTime: ✅ 313.237µs (SLO: <420.000µs 📉 -25.4%) vs baseline: -2.2% Memory: ✅ 42.900MB (SLO: <46.000MB -6.7%) vs baseline: +4.2% ✅ repr_noaspectTime: ✅ 46.666µs (SLO: <90.000µs 📉 -48.1%) vs baseline: -0.2% Memory: ✅ 42.979MB (SLO: <46.000MB -6.6%) vs baseline: +4.7% ✅ rstrip_aspectTime: ✅ 380.625µs (SLO: <500.000µs 📉 -23.9%) vs baseline: -1.4% Memory: ✅ 42.920MB (SLO: <46.000MB -6.7%) vs baseline: +4.5% ✅ rstrip_noaspectTime: ✅ 185.025µs (SLO: <300.000µs 📉 -38.3%) vs baseline: -0.3% Memory: ✅ 42.920MB (SLO: <46.000MB -6.7%) vs baseline: +4.6% ✅ slice_aspectTime: ✅ 185.254µs (SLO: <300.000µs 📉 -38.2%) vs baseline: +2.1% Memory: ✅ 42.900MB (SLO: <46.000MB -6.7%) vs baseline: +4.3% ✅ slice_noaspectTime: ✅ 54.092µs (SLO: <90.000µs 📉 -39.9%) vs baseline: +0.4% Memory: ✅ 42.900MB (SLO: <46.000MB -6.7%) vs baseline: +4.3% ✅ stringio_aspectTime: ✅ 3.820ms (SLO: <5.000ms 📉 -23.6%) vs baseline: -1.2% Memory: ✅ 42.939MB (SLO: <46.000MB -6.7%) vs baseline: +4.3% ✅ stringio_noaspectTime: ✅ 379.471µs (SLO: <500.000µs 📉 -24.1%) vs baseline: +6.2% Memory: ✅ 42.959MB (SLO: <46.000MB -6.6%) vs baseline: +4.9% ✅ strip_aspectTime: ✅ 268.249µs (SLO: <350.000µs 📉 -23.4%) vs baseline: -4.1% Memory: ✅ 42.880MB (SLO: <46.000MB -6.8%) vs baseline: +4.4% ✅ strip_noaspectTime: ✅ 176.389µs (SLO: <240.000µs 📉 -26.5%) vs baseline: -0.6% Memory: ✅ 42.959MB (SLO: <46.000MB -6.6%) vs baseline: +4.4% ✅ swapcase_aspectTime: ✅ 330.710µs (SLO: <500.000µs 📉 -33.9%) vs baseline: -3.1% Memory: ✅ 42.861MB (SLO: <46.000MB -6.8%) vs baseline: +4.2% ✅ swapcase_noaspectTime: ✅ 272.918µs (SLO: <400.000µs 📉 -31.8%) vs baseline: -1.4% Memory: ✅ 42.880MB (SLO: <46.000MB -6.8%) vs baseline: +4.4% ✅ title_aspectTime: ✅ 317.523µs (SLO: <500.000µs 📉 -36.5%) vs baseline: -2.8% Memory: ✅ 42.900MB (SLO: <46.000MB -6.7%) vs baseline: +4.2% ✅ title_noaspectTime: ✅ 258.750µs (SLO: <400.000µs 📉 -35.3%) vs baseline: -1.6% Memory: ✅ 42.900MB (SLO: <46.000MB -6.7%) vs baseline: +4.3% ✅ translate_aspectTime: ✅ 492.714µs (SLO: <700.000µs 📉 -29.6%) vs baseline: -1.7% Memory: ✅ 42.959MB (SLO: <46.000MB -6.6%) vs baseline: +4.3% ✅ translate_noaspectTime: ✅ 423.512µs (SLO: <500.000µs 📉 -15.3%) vs baseline: -1.7% Memory: ✅ 42.920MB (SLO: <46.000MB -6.7%) vs baseline: +4.4% ✅ upper_aspectTime: ✅ 294.714µs (SLO: <500.000µs 📉 -41.1%) vs baseline: -1.1% Memory: ✅ 42.979MB (SLO: <46.000MB -6.6%) vs baseline: +4.4% ✅ upper_noaspectTime: ✅ 237.256µs (SLO: <400.000µs 📉 -40.7%) vs baseline: -0.4% Memory: ✅ 42.939MB (SLO: <46.000MB -6.7%) vs baseline: +4.2% 📈 iastaspectsospath - 24/24✅ ospathbasename_aspectTime: ✅ 515.871µs (SLO: <700.000µs 📉 -26.3%) vs baseline: 📈 +18.4% Memory: ✅ 42.703MB (SLO: <46.000MB -7.2%) vs baseline: +4.6% ✅ ospathbasename_noaspectTime: ✅ 431.781µs (SLO: <700.000µs 📉 -38.3%) vs baseline: -1.6% Memory: ✅ 42.684MB (SLO: <46.000MB -7.2%) vs baseline: +4.8% ✅ ospathjoin_aspectTime: ✅ 625.295µs (SLO: <700.000µs 📉 -10.7%) vs baseline: +0.3% Memory: ✅ 42.625MB (SLO: <46.000MB -7.3%) vs baseline: +4.2% ✅ ospathjoin_noaspectTime: ✅ 635.203µs (SLO: <700.000µs -9.3%) vs baseline: +1.1% Memory: ✅ 42.585MB (SLO: <46.000MB -7.4%) vs baseline: +4.4% ✅ ospathnormcase_aspectTime: ✅ 348.471µs (SLO: <700.000µs 📉 -50.2%) vs baseline: -1.5% Memory: ✅ 42.625MB (SLO: <46.000MB -7.3%) vs baseline: +4.3% ✅ ospathnormcase_noaspectTime: ✅ 357.694µs (SLO: <700.000µs 📉 -48.9%) vs baseline: -1.5% Memory: ✅ 42.585MB (SLO: <46.000MB -7.4%) vs baseline: +4.2% ✅ ospathsplit_aspectTime: ✅ 492.567µs (SLO: <700.000µs 📉 -29.6%) vs baseline: -0.8% Memory: ✅ 42.585MB (SLO: <46.000MB -7.4%) vs baseline: +4.2% ✅ ospathsplit_noaspectTime: ✅ 501.503µs (SLO: <700.000µs 📉 -28.4%) vs baseline: -1.0% Memory: ✅ 42.664MB (SLO: <46.000MB -7.3%) vs baseline: +4.8% ✅ ospathsplitdrive_aspectTime: ✅ 374.235µs (SLO: <700.000µs 📉 -46.5%) vs baseline: -0.4% Memory: ✅ 42.703MB (SLO: <46.000MB -7.2%) vs baseline: +4.8% ✅ ospathsplitdrive_noaspectTime: ✅ 73.353µs (SLO: <700.000µs 📉 -89.5%) vs baseline: +0.3% Memory: ✅ 42.605MB (SLO: <46.000MB -7.4%) vs baseline: +4.6% ✅ ospathsplitext_aspectTime: ✅ 453.838µs (SLO: <700.000µs 📉 -35.2%) vs baseline: -2.0% Memory: ✅ 42.546MB (SLO: <46.000MB -7.5%) vs baseline: +4.1% ✅ ospathsplitext_noaspectTime: ✅ 464.603µs (SLO: <700.000µs 📉 -33.6%) vs baseline: -2.2% Memory: ✅ 42.546MB (SLO: <46.000MB -7.5%) vs baseline: +4.1%
|
Codeowners resolved as |
dubloom
left a comment
There was a problem hiding this comment.
There is indeed a circular import.
Otherwise, LGTM
| if not config.enabled: | ||
| return None, None | ||
|
|
||
| from ddtrace import config as ddtrace_config |
There was a problem hiding this comment.
So we have a problem because, tests are passing but codex is right, if you run
DD_EXPERIMENTAL_PROPAGATE_PROCESS_TAGS_ENABLED=True ddtrace-run python your_script.py, you indeed have a circular import (that is also why I created a config for process_tags).
This change implements the RFC referenced in AMPLP-1052 describing a process-level tagging structure indicating the source of the service name.