Expand OpenTelemetry support: conventions, resource, env-driven setup, MSTest spans

[Evangelink]

Why

The OpenTelemetry extension worked, but it was minimal: traces and metrics only, custom (non-conventional) attribute names, no span status so failing tests produced green spans, eight untagged instruments with no units or buckets, no resource attributes, no OTEL_* support, no trace context propagation, and no instrumentation at all inside MSTest or the other extensions. In practice a trace showed one span per test case and nothing that explained a slow or failing run.

This makes the extension useful out of the box and extends the instrumentation down into the MSTest engine.

What changed

Conventions and signal quality. Where an OpenTelemetry semantic convention exists it is now used verbatim (test.case.name, test.case.result.status, test.suite.name, code.function.name, code.file.path, code.line.number, code.stacktrace, error.type), failures set an Error span status and record a proper exception event, and durations are recorded in seconds with explicit histogram buckets tuned for test latencies. The pre-existing attribute and instrument names are still emitted by default so current dashboards keep working; TESTINGPLATFORM_OTEL_EMIT_LEGACY_ATTRIBUTES=0 drops them.

Zero-code enablement. AddOpenTelemetryProviderFromEnvironment() configures instrumentation, resource and an OTLP exporter purely from the standard OTEL_* variables, and installs nothing when no exporter is configured, so it is free to leave in Program.cs. AddTestingPlatformResource() describes where a run happened, including CI provider, pipeline run, branch and commit detected from GitHub Actions, Azure Pipelines, GitLab CI and Jenkins.

Correlation. A run now nests under the TRACEPARENT published by whoever started the process, so test spans join their CI step's trace instead of starting an orphan one. tracestate is carried across too.

MSTest. A dependency-free instrumentation seam in the engine (also usable under VSTest, and a single static field read when off) produces spans for AssemblyInitialize/Cleanup, ClassInitialize/Cleanup, TestInitialize/Cleanup and the test method itself. This is what tells "the test is slow" apart from "the assembly fixture is slow".

Retry. Per-attempt spans and a test.case.retry.count counter.

Worth a careful look

MSTest spans are deliberately non-ambient. MSTest captures the ExecutionContext inside AssemblyInitialize/ClassInitialize and replays it for every subsequent test. Activity.Current is an async-local, so an ambient fixture span was captured into that context and parented the rest of the run, including any span the user's own code created, to an already-stopped fixture span. It also changed what test code observes from Activity.Current. The spans are therefore timed and exported without ever becoming current, and are parented explicitly. The trade-off is a flatter shape: MSTest spans are siblings under the test-framework span rather than nested. Restoring nesting safely is follow-up work.

Package dependency. The turnkey API pulls in OpenTelemetry.Exporter.OpenTelemetryProtocol. That is what makes "configure by environment variable" possible, but splitting it into a separate exporters package is a reasonable alternative if the closure size matters.

Platform extensions vs. conventions. Every emitted name was checked against the upstream registry (semantic conventions 1.43.0). There are no upstream test.* metrics and no test-case span conventions, and cicd.provider.name does not exist. Those names are documented as Microsoft.Testing.Platform extensions rather than presented as standard, and deliberately sit in the namespace where an upstream definition would land.

One local mitigation, one separate bug. OpenTelemetryResultHandler's bookkeeping is guarded so telemetry cannot throw during shutdown. The underlying platform behaviour, that a cancelled run never drains or disables the message bus so any IDataConsumer can be disposed while still consuming, predates this work and affects TRX, coverage and the report extensions equally. It is tracked separately in #10357 and should get its own PR.

Testing

Five review rounds produced 20 findings, all fixed. Around 20 tests were added, including live ActivityListener tests that cover the ambient-context behaviour mocks cannot see, and tests pinning the MSTest seam as inert when tracing is off. Build is clean and 7,116 tests pass across the platform, extensions, adapter and framework suites; the new concurrency test was run repeatedly to confirm it is not flaky.

[Copilot]

Pull request overview

Expands OpenTelemetry across MTP, MSTest, and retry execution with richer signals, environment configuration, resources, and trace correlation.

Changes:

• Adds semantic attributes, metrics, resources, context propagation, and OTLP setup.
• Instruments MSTest lifecycle operations and retry attempts.
• Adds tests, documentation, dependencies, and API tracking.

Show a summary per file

File	Description
test/UnitTests/MSTestAdapter.PlatformServices.UnitTests/MSTestInstrumentationTests.cs	Tests the MSTest tracing seam.
test/UnitTests/Microsoft.Testing.Platform.UnitTests/Telemetry/OpenTelemetryResultHandlerTests.cs	Expands result telemetry tests.
test/UnitTests/Microsoft.Testing.Platform.UnitTests/Telemetry/EnvironmentTraceContextTests.cs	Tests environment trace context parsing.
test/UnitTests/Microsoft.Testing.Platform.UnitTests/Services/TestApplicationResultTests.cs	Updates telemetry service mocks.
test/UnitTests/Microsoft.Testing.Extensions.UnitTests/OpenTelemetryPlatformServiceTests.cs	Tests live activity behavior.
test/UnitTests/Microsoft.Testing.Extensions.UnitTests/Microsoft.Testing.Extensions.UnitTests.csproj	References the OpenTelemetry extension.
src/Platform/Microsoft.Testing.Platform/Telemetry/TestingPlatformSemanticConventions.cs	Centralizes telemetry names and values.
src/Platform/Microsoft.Testing.Platform/Telemetry/PlatformOpenTelemetryOptions.cs	Adds environment-driven telemetry options.
src/Platform/Microsoft.Testing.Platform/Telemetry/PlatformActivityStatusCode.cs	Defines activity status abstraction.
src/Platform/Microsoft.Testing.Platform/Telemetry/PlatformActivityKind.cs	Defines activity kind abstraction.
src/Platform/Microsoft.Testing.Platform/Telemetry/OpenTelemetryResultHandler.cs	Adds richer spans and metrics.
src/Platform/Microsoft.Testing.Platform/Telemetry/Meters.cs	Extends metric abstractions.
src/Platform/Microsoft.Testing.Platform/Telemetry/IPlatformOpenTelemetryService.cs	Expands telemetry service capabilities.
src/Platform/Microsoft.Testing.Platform/Telemetry/IPlatformActivity.cs	Adds status, events, and identifiers.
src/Platform/Microsoft.Testing.Platform/Telemetry/EnvironmentTraceContext.cs	Reads W3C context from environment.
src/Platform/Microsoft.Testing.Platform/Services/TestApplicationResult.cs	Reports run-level telemetry.
src/Platform/Microsoft.Testing.Platform/Requests/TestHostTestFrameworkInvoker.cs	Adds conventional framework/session tags.
src/Platform/Microsoft.Testing.Platform/InternalAPI/InternalAPI.Unshipped.txt	Tracks new platform internals.
src/Platform/Microsoft.Testing.Platform/Hosts/TestHostBuilder.CommonServices.cs	Parents runs to environment context.
src/Platform/Microsoft.Testing.Platform/Hosts/CommonTestHost.cs	Records root run verdicts.
src/Platform/Microsoft.Testing.Platform/Helpers/ExtensionHelper.cs	Adds conventional extension tags.
src/Platform/Microsoft.Testing.Platform/Helpers/EnvironmentVariableConstants.cs	Defines telemetry environment variables.
src/Platform/Microsoft.Testing.Extensions.Retry/RetryOrchestrator.cs	Instruments retry attempts.
src/Platform/Microsoft.Testing.Extensions.OpenTelemetry/UpDownCounterWrapper.cs	Implements UpDownCounter support.
src/Platform/Microsoft.Testing.Extensions.OpenTelemetry/TestingPlatformResourceDetector.cs	Detects process and CI resources.
src/Platform/Microsoft.Testing.Extensions.OpenTelemetry/PublicAPI/PublicAPI.Unshipped.txt	Tracks new public extension APIs.
src/Platform/Microsoft.Testing.Extensions.OpenTelemetry/PACKAGE.md	Documents signals and configuration.
src/Platform/Microsoft.Testing.Extensions.OpenTelemetry/OpenTelemetryProviderExtensions.cs	Adds resource and environment setup APIs.
src/Platform/Microsoft.Testing.Extensions.OpenTelemetry/OpenTelemetryPlatformService.cs	Implements expanded telemetry abstractions.
src/Platform/Microsoft.Testing.Extensions.OpenTelemetry/OpenTelemetryEnvironmentVariables.cs	Defines standard OTEL variables.
src/Platform/Microsoft.Testing.Extensions.OpenTelemetry/Microsoft.Testing.Extensions.OpenTelemetry.csproj	Adds the OTLP exporter dependency.
src/Platform/Microsoft.Testing.Extensions.OpenTelemetry/MeasurementTags.cs	Converts metric tag collections.
src/Platform/Microsoft.Testing.Extensions.OpenTelemetry/InternalAPI/InternalAPI.Unshipped.txt	Tracks extension internals.
src/Platform/Microsoft.Testing.Extensions.OpenTelemetry/HistogramWrapper.cs	Adds tagged histogram recording.
src/Platform/Microsoft.Testing.Extensions.OpenTelemetry/CounterWrapper.cs	Adds tagged counter recording.
src/Platform/Microsoft.Testing.Extensions.OpenTelemetry/ActivityWrapper.cs	Implements status and exception events.
src/Adapter/MSTestAdapter.PlatformServices/Telemetry/MSTestInstrumentation.cs	Introduces the MSTest tracing seam.
src/Adapter/MSTestAdapter.PlatformServices/Execution/TestMethodRunner.cs	Instruments test-method execution.
src/Adapter/MSTestAdapter.PlatformServices/Execution/TestMethodInfo.Lifecycle.cs	Instruments test fixtures.
src/Adapter/MSTestAdapter.PlatformServices/Execution/TestClassInfo.Initializer.cs	Instruments class initialization.
src/Adapter/MSTestAdapter.PlatformServices/Execution/TestClassInfo.Cleanup.cs	Instruments class cleanup.
src/Adapter/MSTestAdapter.PlatformServices/Execution/TestAssemblyInfo.cs	Instruments assembly fixtures.
src/Adapter/MSTest.TestAdapter/TestingPlatformAdapter/MSTestTestFramework.cs	Enables MSTest instrumentation.
src/Adapter/MSTest.TestAdapter/TestingPlatformAdapter/MSTestPlatformActivity.cs	Bridges MSTest spans to MTP.
src/Adapter/MSTest.TestAdapter/InternalAPI/InternalAPI.Unshipped.txt	Tracks the new bridge internals.
Directory.Packages.props	Pins the OTLP exporter version.

Review details

Suppressed comments (3)

src/Adapter/MSTestAdapter.PlatformServices/Execution/TestAssemblyInfo.cs:321

• If RunWithTimeoutAndCancellationAsync throws, this catch stores the failure only after the using scope has already disposed the assembly-cleanup span, so that span is exported without Error status or an exception event. Keep the activity in scope for the catch and call RecordException(ex) before disposal.

        catch (Exception ex)
        {
            AssemblyCleanupException = ex;
        }

src/Adapter/MSTestAdapter.PlatformServices/Execution/TestMethodRunner.cs:137

• Inconclusive is not treated as skipped here, so an all-inconclusive invocation is tagged pass; the adapter normally converts inconclusive results to skipped (or failed when configured). Include this outcome and account for MapInconclusiveToFailed so the MSTest span agrees with the published test node.

            if (testResult.Outcome is not (UnitTestOutcome.Ignored or UnitTestOutcome.NotRunnable))
            {
                allSkipped = false;

src/Adapter/MSTestAdapter.PlatformServices/Execution/TestMethodRunner.cs:143

• Aborted, Unknown, and NotFound are omitted from the failure set, so invocations with those outcomes fall through to pass even though the adapter publishes them as failures/errors. Include every non-success failure outcome in this branch.

            if (testResult.Outcome is UnitTestOutcome.Failed or UnitTestOutcome.Error or UnitTestOutcome.Timeout)
            {
                anyFailed = true;
            }

• Files reviewed: 46/46 changed files
• Comments generated: 14
• Review effort level: Balanced

[Copilot]

Review details

Suppressed comments (5)

src/Platform/Microsoft.Testing.Platform/InternalAPI/InternalAPI.Unshipped.txt:448

• The shipped four-parameter StartActivity signature is still listed in InternalAPI.Shipped.txt, but this file only declares the new six-parameter signature. PublicApiAnalyzers will report RS0017 under warnings-as-errors unless the old signature is explicitly marked removed.

Microsoft.Testing.Platform.Telemetry.IPlatformOpenTelemetryService.StartActivity(string! name = "", System.Collections.Generic.IEnumerable<System.Collections.Generic.KeyValuePair<string!, object?>>? tags = null, string? parentId = null, System.DateTimeOffset startTime = default(System.DateTimeOffset), Microsoft.Testing.Platform.Telemetry.PlatformActivityKind kind = Microsoft.Testing.Platform.Telemetry.PlatformActivityKind.Internal, bool setAsCurrent = true) -> Microsoft.Testing.Platform.Telemetry.IPlatformActivity?

src/Platform/Microsoft.Testing.Extensions.OpenTelemetry/InternalAPI/InternalAPI.Unshipped.txt:31

• Both the one-parameter ActivityWrapper constructor and the four-parameter OpenTelemetryPlatformService.StartActivity remain in this project's InternalAPI.Shipped.txt. Their changed CLR signatures require *REMOVED* declarations; otherwise RS0017 fails the warnings-as-errors build.

Microsoft.Testing.Extensions.OpenTelemetry.ActivityWrapper.ActivityWrapper(System.Diagnostics.Activity! activity, bool isAmbient = true) -> void
Microsoft.Testing.Extensions.OpenTelemetry.OpenTelemetryPlatformService.HasCurrentActivity.get -> bool
Microsoft.Testing.Extensions.OpenTelemetry.OpenTelemetryPlatformService.StartActivity(string! name = "", System.Collections.Generic.IEnumerable<System.Collections.Generic.KeyValuePair<string!, object?>>? tags = null, string? parentId = null, System.DateTimeOffset startTime = default(System.DateTimeOffset), Microsoft.Testing.Platform.Telemetry.PlatformActivityKind kind = Microsoft.Testing.Platform.Telemetry.PlatformActivityKind.Internal, bool setAsCurrent = true) -> Microsoft.Testing.Platform.Telemetry.IPlatformActivity?

src/Platform/Microsoft.Testing.Platform/Helpers/EnvironmentVariableConstants.cs:50

• This source file is linked into Retry, TrxReport, HangDump, and HotReload, whose existing constants are tracked in each project's InternalAPI.Unshipped.txt. None of those four files declares these seven new constants, so RS0016 will fail those builds. Add the corresponding entries to every tracking consumer (the linked-project references are at Retry.csproj:26, TrxReport.csproj:27, HangDump.csproj:44, and HotReload.csproj:13).

    // OpenTelemetry
    // W3C trace context of the process that started this test run, so the run nests under it.
    public const string TRACEPARENT = nameof(TRACEPARENT);
    public const string TRACESTATE = nameof(TRACESTATE);
    public const string TESTINGPLATFORM_TRACEPARENT = nameof(TESTINGPLATFORM_TRACEPARENT);

src/Platform/Microsoft.Testing.Extensions.OpenTelemetry/OpenTelemetryPlatformService.cs:37

• RootTraceState is applied only after ActivitySource.StartActivity has already run its sampler. A sampler therefore cannot see the propagated tracestate, and if it drops the root activity this block is never reached at all. This breaks vendor sampling decisions carried in tracestate. Parse the traceparent and tracestate into an ActivityContext and pass that context to the StartActivity overload so sampling receives both values.

        // Activity only derives tracestate from an in-process parent reference, which an explicit parent id string
        // does not provide. Stamp it on so the caller's vendor sampling state survives down to the test spans.
        if (RootTraceState is not null && activity.TraceStateString is null)
        {
            activity.TraceStateString = RootTraceState;

src/Adapter/MSTest.TestAdapter/TestingPlatformAdapter/MSTestPlatformActivity.cs:21

• MSTest failures bypass TESTINGPLATFORM_OTEL_ATTRIBUTE_VALUE_LENGTH_LIMIT here. ActivityWrapper.RecordException writes the full exception message and exception.ToString() into event attributes, while only OpenTelemetryResultHandler applies PlatformOpenTelemetryOptions.Truncate. Large assertion messages or fixture exceptions can therefore produce unbounded payloads despite the configured limit. Pass the resolved limit into this bridge and truncate the MSTest exception event/status consistently.

    public void RecordException(Exception exception)
        => activity.RecordException(exception);

• Files reviewed: 48/48 changed files
• Comments generated: 1
• Review effort level: Balanced

[github-actions]

🤖 Automated content by GitHub Copilot. Generated by the Grade Tests on PR (on open / sync) workflow. · sonnet46 110.8 AIC · ⌖ 5.43 AIC · ⊞ 11.8K · ◷

Comments that could not be inline-anchored

test/UnitTests/Microsoft.Testing.Platform.UnitTests/Telemetry/OpenTelemetryResultHandlerTests.cs:385

🧪 Grade B (80–89) — Asserts counter increments but does not verify both activities are independently tracked in FIFO order.

Assert that both activities are tracked by completing each and checking their tags are set separately.

        Assert.AreEqual(2, _startedCounter.Value);
        // Both activities must be independently completable without throwing.
        _handler.NotifyPassed(testNode, PassedTestNodeStateProperty.CachedInstance);
        _handler.NotifyPassed(testNo…

</details>

<details><summary>test/UnitTests/Microsoft.Testing.Platform.UnitTests/Telemetry/OpenTelemetryResultHandlerTests.cs:141</summary>

🧪 **Grade B (80–89)** — Only verifies no exception; a mutation returning a non-null dummy activity would pass silently.

After NotifyPassed, assert that no SetTag or Dispose was called on any mock activity.

```suggestion
        // Should not throw when completing the test (no activity tracked).
        _handler.NotifyPassed(testNode, PassedTestNodeStateProperty.CachedInstance);

        // Verify no activity methods were invoked (null-activity path is inert).
        Assert.AreEqual(1, _comp…

</details>

<details><summary>test/UnitTests/Microsoft.Testing.Platform.UnitTests/Telemetry/OpenTelemetryResultHandlerTests.cs:694</summary>

🧪 **Grade B (80–89)** — Purely a &quot;does not throw&quot; smoke test; the concurrent state transitions are not asserted on.

After publisher completes, assert `_activeTestCases.Value` equals 0 to confirm all in-flight tests resolved.

```suggestion
        _handler.Dispose();
        publisher.GetAwaiter().GetResult();

        // All in-flight test nodes were either completed or orphaned; gauge must be back at zero.
        Assert.AreEqual(0, _activeTestCases.Value);

test/UnitTests/Microsoft.Testing.Platform.UnitTests/Telemetry/EnvironmentTraceContextTests.cs:89

🧪 Grade B (80–89) — Single trivial null-return assertion; no positive case is paired and the test does not verify which environment variable was consulted.

Add a mock verification to confirm the method accessed the expected variable before returning null.

        Assert.IsNull(EnvironmentTraceContext.TryGetTraceState(environment.Object));
        environment.Verify(e => e.GetEnvironmentVariable("TRACESTATE"), Times.Once);

[Evangelink]

Thanks, the mutation notes were useful. I took the two that closed real assertion gaps:

• Dispose_WhileResultsAreStillArriving now asserts test.case.active settles at zero. That is exactly the invariant the locking fix exists to preserve, so a smoke test was too weak there.
• NotifyInProgress_WhenStartActivityReturnsNull now asserts the result is still counted, not just that nothing throws.

I left the other two. TryGetTraceState_ReturnsNullWhenUnset already has a positive partner in TryGetTraceState_ReturnsTrimmedValue, and the duplicate-uid FIFO ordering is already covered by NotifyExecutionCompleted_ClosesOnlyOldestActivityWithoutOutcome and Dispose_DisposesOrphanedActivities, so restating it would be duplication rather than coverage.

[Copilot]

Review details

Suppressed comments (16)

src/Adapter/MSTestAdapter.PlatformServices/Execution/TestClassInfo.Initializer.cs:292

• An ordinary class-initialize exception is rethrown by RunWithTimeoutAndCancellationAsync (see FixtureMethodRunner.cs:37 and :145-148), so execution skips the RecordException block and disposes this new span with Unset status. Catch, record, and rethrow around the await so thrown fixture failures are exported as failed spans as well as timeout/cancellation results.

        TestFailedException? result = await FixtureMethodRunner.RunWithTimeoutAndCancellationAsync(

src/Adapter/MSTestAdapter.PlatformServices/Execution/TestMethodInfo.Lifecycle.cs:271

• A normal exception from TestInitialize is rethrown by the fixture runner, so this await exits before RecordException and the new initialize span remains green. Catch, record, and rethrow so both returned timeout/cancellation failures and thrown user-code failures mark the span.

        TestFailedException? result = await InvokeFixtureMethodAsync(

src/Adapter/MSTestAdapter.PlatformServices/Execution/TestAssemblyInfo.cs:300

• Unlike assembly initialization, assembly cleanup has no catch inside the activity scope. When the fixture body throws, the outer catch at line 318 records the adapter failure only after this activity has already been disposed, so the cleanup span remains green. Record and rethrow inside this scope before the outer catch converts the result.

            AssemblyCleanupException = await FixtureMethodRunner.RunWithTimeoutAndCancellationAsync(

src/Adapter/MSTestAdapter.PlatformServices/Execution/TestClassInfo.Cleanup.cs:208

• Thrown class-cleanup failures bypass the code after this await because RunWithTimeoutAndCancellationAsync rethrows ordinary fixture exceptions. The using then closes the span without an exception event or Error status; record the exception in a catch before rethrowing it to the existing caller.

        TestFailedException? result = await FixtureMethodRunner.RunWithTimeoutAndCancellationAsync(

src/Adapter/MSTestAdapter.PlatformServices/Execution/TestMethodInfo.Lifecycle.cs:314

• A thrown TestCleanup exception skips the post-await RecordException block, leaving the cleanup span with Unset status even though the adapter reports a cleanup failure. Record and rethrow thrown exceptions inside the activity lifetime.

        TestFailedException? result = await InvokeFixtureMethodAsync(

src/Platform/Microsoft.Testing.Platform/Hosts/TestHostBuilder.CommonServices.cs:138

• TRACESTATE is stamped even when TRACEPARENT is absent or invalid, so a newly created root trace can inherit vendor state that belongs to no parent context. W3C tracestate is only valid alongside a valid traceparent; resolve the parent once and only carry tracestate when that parent was accepted.

                platformOTelService.RootTraceState = EnvironmentTraceContext.TryGetTraceState(systemEnvironment);
                context.BuilderActivity = platformOTelService.StartActivity(
                    TestingPlatformSemanticConventions.Activities.TestHostBuilder,
                    parentId: EnvironmentTraceContext.TryGetParentId(systemEnvironment),
                    startTime: buildBuilderStart);

src/Platform/Microsoft.Testing.Extensions.OpenTelemetry/TestingPlatformResourceDetector.cs:116

• GITHUB_REPOSITORY contains owner/repository, but the OpenTelemetry definition of vcs.repository.name explicitly excludes the organization/group (for example, it expects semantic-conventions, not open-telemetry/semantic-conventions). Split off the owner before emitting this attribute, and optionally emit it separately as vcs.owner.name.

                ("vcs.repository.name", "GITHUB_REPOSITORY")))

src/Platform/Microsoft.Testing.Extensions.Retry/RetryOrchestrator.cs:168

• The attempt span is never assigned a status from attemptResult.ExitCode, including the ExitedBeforeConnect early return. Failed retry attempts therefore still appear as green/Unset spans, contrary to the PR's failure-status goal. Set Error/Ok after the attempt completes and mark the early-connect failure before returning.

            using IPlatformActivity? attemptActivity = otelService?.StartActivity(
                "RetryAttempt",
                tags:
                [
                    new(TestingPlatformSemanticConventions.Attributes.TestCaseRetryAttempt, attemptCount),
                    new("test.retry.max_attempts", userMaxRetryCount + 1),
                    new("test.retry.scheduled_count", lastListOfFailedId?.Length ?? 0),
                ]);

src/Adapter/MSTest.TestAdapter/TestingPlatformAdapter/MSTestPlatformActivity.cs:21

• This bridge sends full exception messages and exception.ToString() through ActivityWrapper.RecordException, bypassing TESTINGPLATFORM_OTEL_ATTRIBUTE_VALUE_LENGTH_LIMIT. As a result, the newly added MSTest fixture/method spans can still export arbitrarily large or sensitive stack traces even though the package documents that limit. Pass the resolved limit into this path or provide a truncating record operation.

    public void RecordException(Exception exception)
        => activity.RecordException(exception);

src/Platform/Microsoft.Testing.Extensions.OpenTelemetry/OpenTelemetryProviderExtensions.cs:209

• The standard OpenTelemetry boolean grammar is case-insensitive, but this recognizes only three hand-picked casings. For example, OTEL_SDK_DISABLED=TrUe incorrectly enables and exports telemetry. Use an ordinal case-insensitive comparison (while retaining 1 as the supported alias).

    private static bool IsTrue(string? value)
        => value is "1" or "true" or "True" or "TRUE";

src/Adapter/MSTest.TestAdapter/TestingPlatformAdapter/MSTestPlatformActivity.cs:46

• This enables the static MSTest factory whenever the telemetry service exists, not when tracing is enabled. A metrics-only AddOpenTelemetryProviderFromEnvironment registration still creates the service but has no ActivitySource listener, so every test and fixture now allocates its tag array only for StartActivity to return null. Expose listener availability or make tag creation lazy so the advertised inert tracing path also holds for metrics-only configurations.

        if (serviceProvider.GetService(typeof(IPlatformOpenTelemetryService)) is not IPlatformOpenTelemetryService otelService)
        {
            MSTestInstrumentation.SetActivityFactory(null);
            return;
        }

        MSTestInstrumentation.SetActivityFactory((name, tags)
            // setAsCurrent: false is required, not an optimization - see the remarks on MSTestInstrumentation.
            // Because the spans are not ambient they cannot pick a parent up from the ambient context either, so
            // they are parented explicitly to the same test-framework span the platform's test-case spans use,
            // which keeps everything in one trace.
            => otelService.StartActivity(name, tags, parentId: otelService.TestFrameworkActivity?.Id, setAsCurrent: false) is { } platformActivity

src/Platform/Microsoft.Testing.Platform/Telemetry/PlatformOpenTelemetryOptions.cs:36

• The documented global attribute limit is not actually enforced for all string attributes. For example, GetTestInitialInfo emits display names, paths, and metadata values unchanged, and SetResultDetails emits metadata values and step descriptions unchanged. Those values are user-controlled and can exceed this limit, so the package's TESTINGPLATFORM_OTEL_ATTRIBUTE_VALUE_LENGTH_LIMIT contract remains incomplete; apply truncation centrally to every string tag value or narrow the documented contract.
  src/Platform/Microsoft.Testing.Extensions.OpenTelemetry/OpenTelemetryProviderExtensions.cs:129
• There are no tests for this new public environment-driven API. Its signal-specific branches (none, otlp, comma-separated exporters, endpoint-only opt-in, SDK disablement, and delegates) determine whether providers and exporters are installed, and a prior bug already occurred in the no-configuration branch. Add unit tests that isolate the relevant environment variables and verify registration/configuration for each branch.

    public static void AddOpenTelemetryProviderFromEnvironment(this ITestApplicationBuilder builder, Action<TracerProviderBuilder>? configureTracing = null, Action<MeterProviderBuilder>? configureMetrics = null)

src/Platform/Microsoft.Testing.Extensions.OpenTelemetry/TestingPlatformResourceDetector.cs:107

• The new CI/resource detector has no tests, despite provider-specific mapping, precedence, and credential stripping being security-sensitive. Add isolated environment-variable tests for each provider, repository-name normalization, URL user-info removal, and the no-CI path so future mapping changes cannot leak credentials or silently emit non-conventional values.

    private static IEnumerable<KeyValuePair<string, object>> GetCiAttributes()
    {
        if (IsTrue(Environment.GetEnvironmentVariable("GITHUB_ACTIONS")))

src/Platform/Microsoft.Testing.Extensions.OpenTelemetry/OpenTelemetryPlatformService.cs:37

• RootTraceState is applied only after ActivitySource.StartActivity returns, but OpenTelemetry sampling runs inside StartActivity. A sampler therefore never sees the inherited tracestate and may drop a trace that the caller's vendor state intended to retain; ActivityStarted listeners also observe it as null. Parse the parent ID and tracestate into an ActivityContext and use the parent-context overload so tracestate participates in sampling as well as export.

        // Activity only derives tracestate from an in-process parent reference, which an explicit parent id string
        // does not provide. Stamp it on so the caller's vendor sampling state survives down to the test spans.
        if (RootTraceState is not null && activity.TraceStateString is null)
        {
            activity.TraceStateString = RootTraceState;

src/Platform/Microsoft.Testing.Extensions.Retry/RetryOrchestrator.cs:127

• No tests exercise the new retry counter or per-attempt spans. Add coverage for first-attempt exclusion, scheduled-count values on later attempts, success/failure span status, and interrupted/early-exit attempts; otherwise the newly added retry telemetry can regress while the existing retry behavior tests still pass.

        IPlatformOpenTelemetryService? otelService = _serviceProvider.GetPlatformOTelService();
        ICounter<int>? retryCounter = otelService?.CreateCounter<int>(
            TestingPlatformSemanticConventions.Metrics.TestRetryCount,
            TestingPlatformSemanticConventions.Units.Count,
            "Number of test cases scheduled for a retry attempt.");

• Files reviewed: 48/48 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[Copilot]

Review details

Suppressed comments (11)

src/Adapter/MSTestAdapter.PlatformServices/Execution/TestMethodInfo.Lifecycle.cs:314

• A regular TestCleanup failure is rethrown by RunWithTimeoutAndCancellationAsync, so execution skips this assignment's following RecordException block and disposes an Unset span. Catch, record, and rethrow around this await as is done for returned timeout/cancellation failures.

        TestFailedException? result = await InvokeFixtureMethodAsync(

src/Adapter/MSTestAdapter.PlatformServices/Execution/TestMethodInfo.Lifecycle.cs:271

• Ordinary TestInitialize exceptions never reach this post-await check: FixtureMethodRunner.RunWithTimeoutAndCancellationAsync rethrows non-cancellation exceptions. The using then closes the fixture span with an Unset status, so precisely the failing fixture this instrumentation is meant to expose still appears green. Record the exception before rethrowing it.

This issue also appears on line 314 of the same file.

        TestFailedException? result = await InvokeFixtureMethodAsync(

src/Adapter/MSTestAdapter.PlatformServices/Execution/TestClassInfo.Initializer.cs:292

• RunWithTimeoutAndCancellationAsync rethrows an exception thrown by the class initializer, so the code below that records a returned failure is bypassed. The enclosing class-initialize logic converts that exception to a failed result only after this using has disposed the span, leaving the fixture span Unset. Record and rethrow here.

        TestFailedException? result = await FixtureMethodRunner.RunWithTimeoutAndCancellationAsync(

src/Adapter/MSTestAdapter.PlatformServices/Execution/TestClassInfo.Cleanup.cs:208

• A class-cleanup method that throws takes the exception path out of RunWithTimeoutAndCancellationAsync; the later result check is therefore never reached, and the activity is disposed as successful/Unset before the outer cleanup code converts the exception. Catch, record, and rethrow around this await.

        TestFailedException? result = await FixtureMethodRunner.RunWithTimeoutAndCancellationAsync(

src/Adapter/MSTestAdapter.PlatformServices/Execution/TestAssemblyInfo.cs:300

• Only timeout/cancellation failures returned by the helper are recorded here. If AssemblyCleanup itself throws, the using activity is disposed while unwinding and the outer catch at line 318 stores the failure after the span is already closed, so that span remains Unset. Keep the activity alive through the catch and call RecordException there before disposing it.

            AssemblyCleanupException = await FixtureMethodRunner.RunWithTimeoutAndCancellationAsync(

src/Adapter/MSTest.TestAdapter/TestingPlatformAdapter/MSTestPlatformActivity.cs:41

• A metrics-only setup (OTEL_METRICS_EXPORTER=otlp, OTEL_TRACES_EXPORTER=none) still registers IPlatformOpenTelemetryService, so this enables the global MSTest factory even though no activity listener exists. Every test and fixture then allocates tag arrays and calls StartActivity only to receive null, contradicting the intended single-field-read disabled path. Gate factory installation on tracing/listener enablement rather than service presence alone.

        MSTestInstrumentation.SetActivityFactory((name, tags)

src/Platform/Microsoft.Testing.Platform/Requests/TestHostTestFrameworkInvoker.cs:123

• These conventional framework tags are only added on the base path. HotReloadTestHostTestFrameworkInvoker.ExecuteRequestAsync overrides this method and creates its own TestFramework activity with only testFramework.ToOTelTags(), so hot-reload runs omit test.framework.name and test.framework.version. Share this tag construction with the override or update that override as part of this change.
  src/Platform/Microsoft.Testing.Platform/Telemetry/OpenTelemetryResultHandler.cs:552
• IPlatformActivity.IsRecording was added specifically to avoid building large payloads when a listener requested propagation only, but this path checks only for a non-null activity. It still truncates/copies stdout and stderr even though SetTag will not record them. Include !activity.IsRecording in this guard.
  src/Platform/Microsoft.Testing.Platform/Telemetry/OpenTelemetryResultHandler.cs:503
• The documented attribute-value limit is bypassed for metadata values here (and in the initial metadata tags), so a large trait can still export an unbounded string despite TESTINGPLATFORM_OTEL_ATTRIBUTE_VALUE_LENGTH_LIMIT. Apply _options.Truncate(metadataProperty.Value) to both the semantic and legacy tags, and apply the same policy centrally to the other string attributes if the option is intended to cover every attribute as documented.
  src/Adapter/MSTestAdapter.PlatformServices/Execution/TestMethodRunner.cs:223
• UnitTestOutcome.NotFound falls into the skipped default here, but UnitTestOutcomeHelper.ToTestOutcome maps it to TestOutcome.NotFound, and the MTP converter publishes that as ErrorTestNodeStateProperty. The inner MSTest span therefore reports skipped while the outer test-case span reports error. Map NotFound to the failing status (and add a regression test) so the two spans agree.

            _ => SkippedOutcomeName,

src/Platform/Microsoft.Testing.Extensions.OpenTelemetry/TestingPlatformResourceDetector.cs:190

• The new CI resource detector and credential-stripping path have no tests: repository-wide test search finds no coverage for TestingPlatformResourceDetector, CI_REPOSITORY_URL, or RemoveUrlUserInfo. Add tests for each provider mapping and URL forms with user info, query/fragment, and no scheme; otherwise a future mapping change can reintroduce credential export unnoticed.

    private static string RemoveUrlUserInfo(string value)

• Files reviewed: 48/48 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[github-actions]

🧪 Test quality grade — PR #10358

No new or modified test methods were identified in the changed regions of this PR. Nothing to grade.

_{Re-run with /grade-tests.}

🤖 Automated content by GitHub Copilot. Generated by the Grade Tests on PR (on open / sync) workflow. · sonnet46 39.4 AIC · ⌖ 4.02 AIC · ⊞ 11.7K · [◷]( · ◷)

[Copilot]

Review details

Suppressed comments (8)

src/Adapter/MSTestAdapter.PlatformServices/Execution/TestMethodInfo.Lifecycle.cs:275

• When the test-initialize method throws normally, InvokeFixtureMethodAsync propagates the exception; only timeout/cancellation failures are returned in result. The using then closes this span without recording the thrown failure, so a failing initializer is exported as green. Record the exception and rethrow around this await, as well as handling the returned failure below.

This issue also appears on line 314 of the same file.

        TestFailedException? result = await InvokeFixtureMethodAsync(
            methodInfo,
            classInstance,
            arguments: null,
            timeout,

src/Adapter/MSTestAdapter.PlatformServices/Execution/TestClassInfo.Initializer.cs:296

• RunWithTimeoutAndCancellationAsync rethrows ordinary class-initialize exceptions; it returns a TestFailedException only for timeout/cancellation paths. Consequently the activity is disposed during unwinding before result can be recorded, and a throwing class initializer produces a green span. Record and rethrow exceptions around this await.

        TestFailedException? result = await FixtureMethodRunner.RunWithTimeoutAndCancellationAsync(
            () => methodInfo.InvokeAsFixtureMethodAsync(
                testContext,
                ec => ExecutionContext = ec),
            testContext.CancellationTokenSource,

src/Adapter/MSTestAdapter.PlatformServices/Execution/TestAssemblyInfo.cs:304

• A normally thrown assembly-cleanup exception jumps to the outer catch after this using has already disposed the activity; only returned timeout/cancellation failures are recorded below. Record the exception inside the activity scope and rethrow it to the existing outer catch so throwing assembly cleanup methods do not produce green spans.

            AssemblyCleanupException = await FixtureMethodRunner.RunWithTimeoutAndCancellationAsync(
                 () => AssemblyCleanupMethod.InvokeAsFixtureMethodAsync(
                     testContext,
                     ec => ExecutionContext = ec),
                 testContext.CancellationTokenSource,

src/Adapter/MSTestAdapter.PlatformServices/Execution/TestMethodInfo.Lifecycle.cs:318

• A normally thrown test-cleanup exception propagates from InvokeFixtureMethodAsync, bypassing the result check and disposing this span with no failure status. Catch only to record the exception on the activity, then rethrow so the existing cleanup error handling remains unchanged.

        TestFailedException? result = await InvokeFixtureMethodAsync(
            methodInfo,
            classInstance,
            arguments: null,
            timeout,

src/Adapter/MSTestAdapter.PlatformServices/Execution/TestClassInfo.Cleanup.cs:212

• Ordinary class-cleanup exceptions are thrown from this helper rather than returned. That skips the result check and disposes the new activity as successful while the caller reports a cleanup failure. Record the thrown exception on the activity before rethrowing it.

        TestFailedException? result = await FixtureMethodRunner.RunWithTimeoutAndCancellationAsync(
            () => methodInfo.InvokeAsFixtureMethodAsync(
                testContext,
                ec => ExecutionContext = ec),
            testContext.CancellationTokenSource,

src/Platform/Microsoft.Testing.Extensions.OpenTelemetry/OpenTelemetryPlatformService.cs:52

• RootTraceState is stamped only after ActivitySource.StartActivity has already run the sampler. For an explicit parent-id string, the sampler therefore receives a parent context without tracestate; a vendor sampler can reject the span, in which case Stamp is never reached at all. Parse a W3C parent id together with RootTraceState into an ActivityContext and use the context overload before sampling (while retaining a fallback for non-W3C parent ids).

        if (RootTraceState is not null && activity.TraceStateString is null)
        {
            activity.TraceStateString = RootTraceState;

src/Platform/Microsoft.Testing.Extensions.OpenTelemetry/OpenTelemetryProviderExtensions.cs:193

• A standard signal-specific endpoint does not opt this API in. With only OTEL_EXPORTER_OTLP_TRACES_ENDPOINT or OTEL_EXPORTER_OTLP_METRICS_ENDPOINT set and the exporter selector unset, this checks only the shared endpoint, both provider flags can remain false, and the method returns without exporting. Include the corresponding signal endpoint when deciding whether that signal has an OTLP configuration.

        // Mirror the behavior of the OpenTelemetry auto-instrumentation: an endpoint alone is enough to opt in.
        if (OpenTelemetryEnvironmentVariables.IsNullOrWhiteSpace(configured))
        {
            return !OpenTelemetryEnvironmentVariables.IsNullOrWhiteSpace(Environment.GetEnvironmentVariable(OpenTelemetryEnvironmentVariables.ExporterOtlpEndpoint));
        }

src/Platform/Microsoft.Testing.Platform/Hosts/CommonTestHost.cs:145

• ServerTestHost is also not a run-verdict owner at this point: test execution happens in ServerTestHost.RequestExecution, while this finally runs only when the server message loop exits. Reporting here makes test.run.duration cover server startup/idle time and emits one shutdown record instead of one record per run request (with cumulative shared result state). Exclude the server host here and report the per-request TestApplicationResult when each run request completes.

            if (OwnsRunVerdict
                && ServiceProvider.GetService<ITestApplicationProcessExitCode>() is TestApplicationResult testApplicationResult)
            {
                testApplicationResult.ReportRunTelemetry(activity, exitCode);
            }

• Files reviewed: 47/47 changed files
• Comments generated: 3
• Review effort level: Balanced