-
Notifications
You must be signed in to change notification settings - Fork 398
Telemetry: inject dependencies into telemetry app started event #5067
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
|
Thank you for updating Change log entry section 👏 Visited at: 2025-11-19 17:20:24 UTC |
BenchmarksBenchmark execution time: 2025-11-19 20:03:27 Comparing candidate commit adb655a in PR branch Found 1 performance improvements and 0 performance regressions! Performance is the same for 43 metrics, 2 unstable metrics. scenario:profiling - Allocations ()
|
Typing analysisNote: Ignored files are excluded from the next sections. Untyped methodsThis PR introduces 5 partially typed methods, and clears 4 partially typed methods. It decreases the percentage of typed methods from 54.67% to 54.64% (-0.03%). Partially typed methods (+5-4)❌ Introduced:If you believe a method or an attribute is rightfully untyped or partially typed, you can add |
Co-authored-by: Ivo Anjo <[email protected]>
|
✅ Tests 🎉 All green!❄️ No new flaky tests detected 🎯 Code Coverage 🔗 Commit SHA: 7b333e9 | Docs | Datadog PR Page | Was this helpful? Give us feedback! |
Strech
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.
Not sure, maybe it's hard to do a components part in the tests, but I'm not sure that I like the use of Datadog.send(:components) because there is no trace of what's inside.
But this is not a blocker.
|
Agree, all of the references to the component tree are not great. I haven't looked deeply into what it would take to eliminate them -my thinking is it might be tricky because telemetry is pretty much collecting the state of everything else in dd-trace-rb, thus either the test becomes extremely mocky or we have to construct a lot of variables in the tests. |
What does this PR do?
Injects component tree into telemetry AppStarted event constructor, instead of various methods of the event referencing various global variables.
Motivation:
Make telemetry code easier to read/reason about for the upcoming change to enable telemetry in child processes
Change log entry
None
Additional Notes:
The event does not hold a reference to the component tree - the tree is used only while constructor is running to build the various payloads that will be sent out later.
The unit tests now perform many references to
Datadog.send(:components)which looks odd (unit tests reference global state) but this was existing behavior - telemetry was reading global state including in unit tests.How to test the change?
Existing CI