[Debugger] Preserve EMITTING diagnostics across probe change#8654
Conversation
There was a problem hiding this comment.
Pull request overview
This PR improves debugger probe diagnostic delivery by ensuring EMITTING statuses are not lost when probes are removed/updated before the next native status poll, and by tightening ownership/invalidating stale queued diagnostics via per-probe generations.
Changes:
- Reworked
DiagnosticsSinkto use a single lock + per-probe generations, preserving one-shot removedEMITTINGdiagnostics while preventing stale re-emission after probe replacement. - Updated
ProbeStatusPollerto immediately emit explicit (non-default) statuses on add/update, while keeping default/fetchable statuses on the native polling path. - Added focused unit tests covering preserved
EMITTINGbehavior across removal/replacement, queue pressure/recreation, and generation cleanup.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tracer/test/Datadog.Trace.Tests/Debugger/ProbeStatusSinkTests.cs | Adds coverage for removed/updated probe diagnostics (especially EMITTING), queue pressure, and poller immediate emission behavior. |
| tracer/src/Datadog.Trace/Debugger/Sink/DiagnosticsSink.cs | Reworks diagnostic storage/queueing with generation-based invalidation and one-shot EMITTING preservation. |
| tracer/src/Datadog.Trace/Debugger/ProbeStatuses/ProbeStatusPoller.cs | Emits explicit statuses immediately during add/update and resets diagnostics on updates/removals. |
| tracer/src/Datadog.Trace/Debugger/Sink/Models/TimedMessage.cs | Removes the standalone TimedMessage model (now internal to DiagnosticsSink). |
| tracer/missing-nullability-files.csv | Removes the deleted TimedMessage.cs entry. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Guarantee that a probe reaching EMITTING is reported at least once even when it is removed, replaced, or queued under pressure before diagnostics are uploaded. Rework DiagnosticsSink ownership around locked generation tracking and one-shot EMITTING removal, and emit explicit poller statuses immediately while keeping default statuses on the native polling path.
519ab8d to
f8ac8ba
Compare
|
@codex review |
|
To use Codex here, create a Codex account and connect to github. |
BenchmarksBenchmark execution time: 2026-05-18 14:24:43 Comparing candidate commit f8ac8ba in PR branch Some scenarios are present only in baseline or only in candidate runs. If you didn't create or remove some scenarios in your branch, this maybe a sign of crashed benchmarks 💥💥💥 Scenarios present only in baseline:
Found 1 performance improvements and 5 performance regressions! Performance is the same for 47 metrics, 19 unstable metrics, 89 known flaky benchmarks, 37 flaky benchmarks without significant changes.
|
## Summary of changes Adds some more owners to the nullability file ## Reason for change There's no reason this has to only be owned by apm-dotnet IMO ## Implementation details - Add more teams as owners of the nullability files csv - Add the full path ## Test coverage Meh ## Other details PRs like #8654 require an APM review even though they only touch Debugger code
Summary of changes
EMITTINGprobe diagnostics are emitted at least once when a probe is removed or updated before the next delayed status poll.ProbeStatusPollerinstead of waiting for the native polling cycle.DiagnosticsSinkqueue ownership so stale queued statuses are invalidated by generation, and one-shotEMITTINGstatuses survive probe removal.Reason for change
EMITTINGand then be removed or replaced before diagnostics were uploaded, causing the backend to miss the fact that instrumentation actually emitted.Implementation details
EMITTINGdiagnostics as one-shot messages: they are preserved until returned once, then both the diagnostic and generation entries are removed.EMITTINGmessages when needed, preferring to rebuild/drop stale queue entries rather than lose a not-yet-returnedEMITTINGstatus.EMITTINGdiagnostics.ProbeStatusPollernow emits known explicit statuses during add/update, but still schedules native polling for default/fetchable statuses.Test coverage
ProbeStatusSinkTestsfor preservedEMITTINGdiagnostics, stale status invalidation, queue pressure, queue recreation, default-status updates, and poller immediate emission.