-
Notifications
You must be signed in to change notification settings - Fork 439
refactor: resolve config-telemetry circular reference #12820
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
|
Bootstrap import analysisComparison of import times between this PR and main. SummaryThe average import time in this PR is: 230 ± 3 ms. The average import time in main is: 231 ± 3 ms. The import time difference between this PR and main is: -1.5 ± 0.1 ms. Import time breakdownThe following import paths have appeared:
|
We refactor the interaction between telemetry and configuration to resolve the circular import issue between these two components.
f975fc1
to
6f64dbb
Compare
BenchmarksBenchmark execution time: 2025-03-26 10:18:11 Comparing candidate commit b86bfaf in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 498 metrics, 2 unstable metrics. |
3c6feb4
to
6d53d20
Compare
6d53d20
to
4f8f464
Compare
Verified offline that with this change there are no import cycles that involve |
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.
This LGTM, do we have R0401 set up to warn somewhere to prevent future cyclic imports?
We don't have a lint check for that unfortunately 🙁 . It looks like ruff still does not support R0401. However, we do have some scripts to check for that, but we're not in a position to include them in CI |
This PR should resolve the system test failures: https://github.com/DataDog/system-tests/pull/4360/files |
f382747
to
3f32a57
Compare
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.
Crashtracking and profiling stuff looks good 👍
We refactor the interaction between telemetry and configuration to resolve the circular import issue between these two components.
We also extend the product plugin interface to export an optional
config
property. When present, and of typeDDConfig
, its configuration items are automatically reported via telemetry.Checklist
Reviewer Checklist