[Dynamic Instrumentation] clarify unbound line probe diagnostics#8415
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request improves debugger logging clarity by distinguishing between two types of unbound line probe failures: cases where a loaded, symbolicated assembly exists but with a mismatched source file path, versus cases where the assembly is genuinely missing or symbols are unavailable. This change allows support engineers and developers to quickly diagnose the root cause rather than being misled toward incorrect root causes.
Changes:
- Split the ambiguous
AssemblyNotLoadedOrSourceFileMismatchenum value into two specific reasons:AssemblyNotLoadedOrSymbolsUnavailableandLoadedAssemblySourceFileMismatch - Implemented logic in
LineProbeResolver.TryResolveLineProbeto differentiate between these two cases based on whetherSameFileNameMatchCount > 0 - Added two new message builder methods with distinct, descriptive messages for each failure reason
- Updated test
MismatchedPathReturnsUnboundWithFileNameDiagnosticsto expect the new reason and verify the mismatch-specific message - Added new test
UnknownFileReturnsUnboundWithGenericMessageto cover the fallback case where no matching files are found
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| tracer/src/Datadog.Trace/Debugger/Models/LiveProbeResolveStatus.cs | Updated enum to rename and split the unbound reason values for clarity |
| tracer/src/Datadog.Trace/Debugger/LineProbeResolver.cs | Implemented conditional logic to assign the appropriate reason based on diagnostics, and added message builder methods |
| tracer/test/Datadog.Trace.Debugger.IntegrationTests/LineProbeResolverTest.cs | Updated existing test and added new test to cover both cases |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Execution-Time Benchmarks Report ⏱️Execution-time results for samples comparing This PR (8415) and master. ✅ No regressions detected - check the details below Full Metrics ComparisonFakeDbCommand
HttpMessageHandler
Comparison explanationExecution-time benchmarks measure the whole time it takes to execute a program, and are intended to measure the one-off costs. Cases where the execution time results for the PR are worse than latest master results are highlighted in **red**. The following thresholds were used for comparing the execution times:
Note that these results are based on a single point-in-time result for each branch. For full results, see the dashboard. Graphs show the p99 interval based on the mean and StdDev of the test run, as well as the mean value of the run (shown as a diamond below the graph). Duration chartsFakeDbCommand (.NET Framework 4.8)gantt
title Execution time (ms) FakeDbCommand (.NET Framework 4.8)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8415) - mean (72ms) : 70, 75
master - mean (72ms) : 70, 75
section Bailout
This PR (8415) - mean (76ms) : 74, 78
master - mean (77ms) : 75, 78
section CallTarget+Inlining+NGEN
This PR (8415) - mean (1,070ms) : 1033, 1106
master - mean (1,066ms) : 1027, 1106
FakeDbCommand (.NET Core 3.1)gantt
title Execution time (ms) FakeDbCommand (.NET Core 3.1)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8415) - mean (112ms) : 109, 116
master - mean (113ms) : 110, 116
section Bailout
This PR (8415) - mean (114ms) : 112, 116
master - mean (114ms) : 111, 117
section CallTarget+Inlining+NGEN
This PR (8415) - mean (779ms) : 759, 799
master - mean (778ms) : 755, 801
FakeDbCommand (.NET 6)gantt
title Execution time (ms) FakeDbCommand (.NET 6)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8415) - mean (100ms) : 97, 103
master - mean (99ms) : 96, 102
section Bailout
This PR (8415) - mean (100ms) : 97, 103
master - mean (100ms) : 97, 102
section CallTarget+Inlining+NGEN
This PR (8415) - mean (936ms) : 898, 974
master - mean (933ms) : 900, 966
FakeDbCommand (.NET 8)gantt
title Execution time (ms) FakeDbCommand (.NET 8)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8415) - mean (98ms) : 94, 102
master - mean (99ms) : 95, 102
section Bailout
This PR (8415) - mean (99ms) : 97, 102
master - mean (99ms) : 97, 101
section CallTarget+Inlining+NGEN
This PR (8415) - mean (816ms) : 777, 855
master - mean (814ms) : 778, 850
HttpMessageHandler (.NET Framework 4.8)gantt
title Execution time (ms) HttpMessageHandler (.NET Framework 4.8)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8415) - mean (190ms) : 185, 196
master - mean (190ms) : 186, 193
section Bailout
This PR (8415) - mean (193ms) : 191, 194
master - mean (193ms) : 191, 195
section CallTarget+Inlining+NGEN
This PR (8415) - mean (1,135ms) : 1093, 1178
master - mean (1,146ms) : 1072, 1220
HttpMessageHandler (.NET Core 3.1)gantt
title Execution time (ms) HttpMessageHandler (.NET Core 3.1)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8415) - mean (272ms) : 269, 276
master - mean (272ms) : 268, 276
section Bailout
This PR (8415) - mean (272ms) : 269, 276
master - mean (272ms) : 270, 275
section CallTarget+Inlining+NGEN
This PR (8415) - mean (920ms) : 902, 937
master - mean (919ms) : 892, 946
HttpMessageHandler (.NET 6)gantt
title Execution time (ms) HttpMessageHandler (.NET 6)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8415) - mean (266ms) : 261, 271
master - mean (266ms) : 262, 271
section Bailout
This PR (8415) - mean (266ms) : 264, 268
master - mean (267ms) : 264, 269
section CallTarget+Inlining+NGEN
This PR (8415) - mean (1,130ms) : 1086, 1173
master - mean (1,133ms) : 1093, 1173
HttpMessageHandler (.NET 8)gantt
title Execution time (ms) HttpMessageHandler (.NET 8)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8415) - mean (265ms) : 260, 269
master - mean (265ms) : 262, 269
section Bailout
This PR (8415) - mean (264ms) : 262, 267
master - mean (265ms) : 262, 268
section CallTarget+Inlining+NGEN
This PR (8415) - mean (1,015ms) : 979, 1050
master - mean (1,016ms) : 966, 1066
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
BenchmarksBenchmark execution time: 2026-04-15 12:39:46 Comparing candidate commit f8af3a7 in PR branch Found 0 performance improvements and 1 performance regressions! Performance is the same for 26 metrics, 0 unstable metrics, 87 known flaky benchmarks.
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: eb35cb59f2
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Distinguish between a truly unloaded assembly and a loaded assembly whose PDB document path does not match the probe source path so debugger logs do not push investigations toward the wrong root cause. Made-with: Cursor
Return a dedicated resolve reason when a loaded, symbolicated assembly has a same-file match but the probe source path does not match the PDB document path, while keeping a separate fallback for unloaded or unsymbolicated cases. Made-with: Cursor
Avoid implying a probe file is definitively absent from all loaded assemblies when symbols are unavailable, and update the resolver test to assert the intended guidance without depending on the exact phrasing.
eb35cb5 to
0f23d50
Compare
Treat same-file-name matches as diagnostic hints instead of proof of a source-path mismatch, so unresolved probes keep the generic unbound reason unless we have stronger evidence.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary of changes
LineProbeResolverso unresolved probes no longer imply the target assembly is simply unloaded when same-file-name diagnostics are available.AssemblyNotLoadedOrSymbolsUnavailable.SameFileNameMatchCountandSameFileNameExamplesas supporting diagnostics, and add a conditional message hint when loaded symbolicated assemblies share the same file name.LineProbeResolverTestto cover both the same-file-name hint path and the generic no-match fallback.Reason for change
Implementation details
LiveProbeResolveStatus.Unboundand returnLineProbeResolveReason.AssemblyNotLoadedOrSymbolsUnavailablefor unresolved line probes.AssemblySearchDiagnostics.SameFileNameMatchCount > 0, so the additional context remains diagnostic rather than authoritative.Test coverage
dotnet test "tracer/test/Datadog.Trace.Debugger.IntegrationTests/Datadog.Trace.Debugger.IntegrationTests.csproj" --filter "FullyQualifiedName~LineProbeResolverTest"Other details