-
Notifications
You must be signed in to change notification settings - Fork 477
chore(tracing): removes tracer._configure from products and tests #12696
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
Conversation
|
|
f0ad5a4 to
a925600
Compare
a925600 to
64f1622
Compare
21474ae to
5a4256b
Compare
2c06702 to
0564439
Compare
BenchmarksBenchmark execution time: 2025-03-25 15:15:58 Comparing candidate commit 8772d56 in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 498 metrics, 2 unstable metrics. |
dade3a6 to
1322497
Compare
Yun-Kim
left a comment
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.
LGTM from team mlobs, thanks for the update!
Bootstrap import analysisComparison of import times between this PR and main. SummaryThe average import time in this PR is: 242 ± 3 ms. The average import time in main is: 245 ± 4 ms. The import time difference between this PR and main is: -3.4 ± 0.2 ms. Import time breakdownThe following import paths have grown:
|
wconti27
left a comment
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.
integration changes LGTM
ZStriker19
left a comment
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.
Just need to add back in those sampling tests (commented on above) with an envar and I think we're good!
598e747 to
5f96efb
Compare
5f96efb to
d9bbafb
Compare
Background
In ddtrace v3.0, most functionality previously handled by tracer.configure(...) was internalized. As a result, ddtrace >3.0 no longer provides a public API for configuring agent connections, custom samplers, UDS, or enabling tracing. To avoid refactoring all tests and products relying on tracer.configure(...), we introduced the private method tracer._configure.
This method has been used across ddtrace products (e.g., ASM, MLObs, and CiVis), integrations (e.g., Tornado, Vertex AI), and hundreds of tests. Now, it’s time to fully migrate away from this API. 🫡
Motivation
This refactor encourages the use of environment variables and/or remote configuration for setting up tracing features. Even though tracer._configure is internal, it can still be used to enable unsupported behavior. This PR aims to put an end to that. 🫡
Changes
Note: This PR also introduces Tracer._recreate(...), a private method used in ASM and CIVis tests to recreate the agent writer and span processors. This was added to limit the scope of changes in this PR. Ideally, tests requiring tracer reconfiguration/restart should be run in a subprocess. This is a temporary workaround (hopefully).
Follow up:
tracer.configure(...).tracer.configure(...)from all tests.Checklist
Reviewer Checklist