NUnit: emit test.final_status on terminal executions#8216
Merged
tonyredondo merged 10 commits intomasterfrom Feb 20, 2026
Merged
NUnit: emit test.final_status on terminal executions#8216tonyredondo merged 10 commits intomasterfrom
tonyredondo merged 10 commits intomasterfrom
Conversation
Set final_status for non-retry NUnit executions in ExecuteTest(), before FinishTest() closes the span, to avoid missing test.final_status in flaky CI snapshot runs.
Ensure NUnit writes test.final_status only on final retry executions, and stop writing it for non-retry and pre-execution skip paths to eliminate close-order race behavior and align with the feature contract. Update NUnit snapshot baselines to match the retry-only tag semantics.
…tatus Update additional NUnit EVP snapshot baselines to remove test.final_status on non-retry executions, matching the retry-only contract and the behavior now emitted by the NUnit integration paths.
Drop investigation-only runtime logging and temporary StyleCop suppressions from NUnit test optimization paths while keeping the retry-only final_status behavior and snapshot baselines unchanged.
Ensure CI environment tag checks always remove inspected keys from the target span, even when the generated reference span does not contain that tag, so leftover tags don't cause downstream assertion noise.
… tests Keep final_status retry-only for EFD, ATR, and ATF while setting final_status=skip for quarantined tests that do not retry, so quarantined outcomes remain queryable in CI Visibility.
…antics Set test.final_status on executions that will not be retried anymore (including pre-skip and disabled paths) while keeping final-retry behavior for retry features. Update NUnit snapshot baselines and EVP metadata cleanup assertions so integration verification remains stable.
BenchmarksBenchmark execution time: 2026-02-20 13:07:05 Comparing candidate commit 696c0ea in PR branch Found 4 performance improvements and 10 performance regressions! Performance is the same for 161 metrics, 17 unstable metrics. scenario:Benchmarks.Trace.AgentWriterBenchmark.WriteAndFlushEnrichedTraces net6.0
scenario:Benchmarks.Trace.Asm.AppSecEncoderBenchmark.EncodeLegacyArgs net6.0
scenario:Benchmarks.Trace.Asm.AppSecWafBenchmark.RunWafRealisticBenchmark netcoreapp3.1
scenario:Benchmarks.Trace.AspNetCoreBenchmark.SendRequest net6.0
scenario:Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark.WriteAndFlushEnrichedTraces net472
scenario:Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark.WriteAndFlushEnrichedTraces net6.0
scenario:Benchmarks.Trace.CharSliceBenchmark.OptimizedCharSlice net6.0
scenario:Benchmarks.Trace.ElasticsearchBenchmark.CallElasticsearchAsync netcoreapp3.1
scenario:Benchmarks.Trace.ILoggerBenchmark.EnrichedLog net6.0
scenario:Benchmarks.Trace.Log4netBenchmark.EnrichedLog netcoreapp3.1
scenario:Benchmarks.Trace.SingleSpanAspNetCoreBenchmark.SingleSpanAspNetCore netcoreapp3.1
scenario:Benchmarks.Trace.SpanBenchmark.StartFinishTwoScopes net6.0
|
Execution-Time Benchmarks Report ⏱️Execution-time results for samples comparing This PR (8216) 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 (8216) - mean (73ms) : 71, 75
master - mean (75ms) : 72, 77
section Bailout
This PR (8216) - mean (77ms) : 75, 79
master - mean (79ms) : 77, 81
section CallTarget+Inlining+NGEN
This PR (8216) - mean (1,065ms) : 1016, 1114
master - mean (1,079ms) : 1039, 1118
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 (8216) - mean (114ms) : 111, 116
master - mean (115ms) : 111, 120
section Bailout
This PR (8216) - mean (115ms) : 113, 118
master - mean (117ms) : 114, 120
section CallTarget+Inlining+NGEN
This PR (8216) - mean (760ms) : 701, 818
master - mean (763ms) : 706, 820
FakeDbCommand (.NET 6)gantt
title Execution time (ms) FakeDbCommand (.NET 6)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8216) - mean (101ms) : 98, 104
master - mean (102ms) : 99, 105
section Bailout
This PR (8216) - mean (102ms) : 99, 105
master - mean (104ms) : 101, 107
section CallTarget+Inlining+NGEN
This PR (8216) - mean (749ms) : 698, 801
master - mean (759ms) : 700, 819
FakeDbCommand (.NET 8)gantt
title Execution time (ms) FakeDbCommand (.NET 8)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8216) - mean (99ms) : 96, 102
master - mean (101ms) : 98, 103
section Bailout
This PR (8216) - mean (100ms) : 98, 102
master - mean (102ms) : 99, 105
section CallTarget+Inlining+NGEN
This PR (8216) - mean (657ms) : 644, 670
master - mean (666ms) : 654, 679
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 (8216) - mean (194ms) : 189, 199
master - mean (197ms) : 180, 215
section Bailout
This PR (8216) - mean (197ms) : 194, 200
master - mean (199ms) : 195, 202
section CallTarget+Inlining+NGEN
This PR (8216) - mean (1,187ms) : 1072, 1301
master - mean (1,144ms) : 1087, 1202
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 (8216) - mean (284ms) : 279, 289
master - mean (280ms) : 272, 287
section Bailout
This PR (8216) - mean (284ms) : 279, 289
master - mean (279ms) : 275, 283
section CallTarget+Inlining+NGEN
This PR (8216) - mean (945ms) : 908, 982
master - mean (944ms) : 909, 980
HttpMessageHandler (.NET 6)gantt
title Execution time (ms) HttpMessageHandler (.NET 6)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8216) - mean (270ms) : 265, 275
master - mean (271ms) : 266, 276
section Bailout
This PR (8216) - mean (271ms) : 267, 275
master - mean (271ms) : 267, 275
section CallTarget+Inlining+NGEN
This PR (8216) - mean (933ms) : 899, 967
master - mean (931ms) : 909, 954
HttpMessageHandler (.NET 8)gantt
title Execution time (ms) HttpMessageHandler (.NET 8)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8216) - mean (270ms) : 265, 275
master - mean (271ms) : 266, 276
section Bailout
This PR (8216) - mean (270ms) : 267, 274
master - mean (270ms) : 266, 275
section CallTarget+Inlining+NGEN
This PR (8216) - mean (831ms) : 810, 851
master - mean (828ms) : 809, 846
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
andrewlock
approved these changes
Feb 20, 2026
Member
andrewlock
left a comment
There was a problem hiding this comment.
I take your word for it in general 😅
...c/Datadog.Trace/ClrProfiler/AutoInstrumentation/Testing/NUnit/TestOptimizationTestCommand.cs
Show resolved
Hide resolved
| !tools/dumps/start_debian-arm64.sh | ||
|
|
||
| # test optimization temp/run folder | ||
| .dd/ No newline at end of file |
Member
There was a problem hiding this comment.
ooooh, so that's where that was coming from 😅
I'm kinda surprised this isn't a problem for customers tbh 🤔
Clarify that the non-retry branch only sets final_status when no retry will run, while final retry handling is performed in the isFinalExecution path below.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary of changes
test.final_statuswhen that execution is the final one for the test.test.final_status=skipfor pre-execution skips and disabled tests.test.final_statuson terminal retry executions.Reason for change
People use
test.final_statusto answer simple questions like “which tests ultimately failed?”NUnit still had a few paths where that tag could be missing or inconsistent (especially around skip/disabled and non-retry paths). This PR makes the behavior consistent with the intended rule: set the tag when no more retries will run.
Implementation details
tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/Testing/NUnit/NUnitIntegration.csfinal_status=skipbefore closing the test span.tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/Testing/NUnit/TestOptimizationTestCommand.csfinal_status=skipfor disabled tests.final_statusfrom the current execution outcome.tracer/test/Datadog.Trace.ClrProfiler.IntegrationTests/CI/TestingFrameworkEvpTest.cstracer/test/Datadog.Trace.ClrProfiler.IntegrationTests/CI/TestingFrameworkTest.csAssert.Single()noise.Test coverage
NUnitTests.SubmitTracesNUnitEvpTests.SubmitTracesNUnitEvpTests.EarlyFlakeDetectionNUnitEvpTests.QuarantinedTestsNUnitEvpTests.DisabledTestsNUnitEvpTests.AttemptToFixTestsOther details