Skip to content

Please reconsider the design surrounding OpenTelemetry in MTP #11505

Description

Summary

Microsoft.Testing.Extensions.OpenTelemetry is inherently incompatible with Aspire ServiceDefaults project due to the completely custom "mirror" interfaces defined by MTP around Microsoft.Extensions.Hosting.

We can't cleanly reuse our shared OpenTelemetry instrumentation setup across test projects, and will temporarily be relying upon XUnit.DependencyInjection's traces instead.

Please reconsider the design of MTP's Open Telemetry support and make it compatible with standard projects.

Background and Motivation

Here is the setup.

We have a large 200+ project monorepo, with a handful of "high level" testing projects (API test and UI tests mostly) using MTP. Those projects are relying today on Xunit.DependencyInjection to register services such as HttpClients, Azure AppConfiguration for storing test-specific configs per environment, etc.

We recently added a .ServiceDefaults project to centralize some of our common services and our OTEL setup logic, which was duplicated across a dozen or so API/worker/CLI projects in the solution.

As part of such unification, we also decided to take a look at the OpenTelemetry support in MTP as we were interested in getting test run telemetry over to Datadog for long-term analysis, monitoring, alerts, etc, and for the improved debugging experience as well while running such tests locally and publishing telemetry to the Aspire dashboard.

Since these high level test projects really operate like any other runnable project today, we decided to also use service defaults in them: we added it to the Xunit.DependencyInjection startup class (since MTP's own startup abstraction is completely incompatible with Microsoft.Extensions.*-based logic that .ServiceDefaults relies on.

This means our OTEL setup for MTP projects is in Startup, not in the Project.cs file.

We tried to consume Microsoft.Testing.Extensions.OpenTelemetry so we could have test-based spans, attributes, etc on top of all the other OTEL instrumentation we already have (runtime, process, SQL, EFCore, HTTP, WCF, ...).

My instinct, having used many OTEL instrumentation libraries before, was that we could just add the needed AddTestingPlatformInstrumentation (for metrics and traces) and AddTestingPlatformResource (for resource attributes) into our existing OTEL setup and it would just work.

I was mistaken.

It looks like without calling AddOpenTelemetryProvider on MTP's builder, no Activity is ever created because the service responsible for creating the activities is never registered.

This is the first glaring mistake here IMHO:

Activity, ActivitySource and Meter stuff is NOT OpenTelemetry

These are OTEL-compatible, but they are not OTEL itself. Requiring a OpenTelemetry package to trigger Activity creation is a massive architectural mistake. The Activity code should always exist in the underlying implementation, and should only be observed by the OTEL instrumentation.

You can talk to the rest of the .NET team on this. See how Activity instrumentation works on AspNetCore, on SqlClient, on EFCore, on HttpClient... they are all built-in now.

Activity classes are OTEL-agnostic. They existed before OTEL was even a thing.

I added a call to .AddOpenTelemetryProvider() on our Program.cs, which leads me to problem 2:

MTP's implementation creates its own TraceProvider and MeterProvider...

Which is completely insane to me. It reinvents parts of OpenTelemetry.Extensions.Hosting which already handles all of this: it has its own hosted service which tracks the initialization and lifetime of TraceProvider and MeterProvider. We rely on those today with the Xunit.DependencyInjection abstraction which uses standard HostApplicationBuilder. It works perfectly... but MTP ignores that and does its own completely custom thing again.

Result: the spans generated by MTP instrumentation are orphaned from all other spans. I'm assuming because we are dealing with 2 distinct TraceProvider instances (one from MTP and another from OpenTelemetry.Extensions.Hosting).

I tried to look at the MTP codebase to see if I could spot exactly why that was happening but you guys came up with an absolutely insane approach of replicating the entire Microsoft.Extensions.Hosting stack and replicating the entire System.Diagnostics stack with a bunch of wrapper implementations for instruments, for activities, for everything...

This is so incredibly frustrating...

I knew all of this would eventually happen. I tried to warn you guys to do things using existing standards instead of reinventing the entire wheel from scratch:

But here we are. Now we just can't use MTP's native OTEL implementation because it is setup in a completely incompatible way that would force us to duplicate a ton of code, code that already works perfectly using Microsoft.Extensions.Hosting abstractions across dozens of projects, that would need to be completely customized to fit MTP's crazyness.

Instead, we added your resources and metrics and used Xunit.DependencyInjection's trace source to get per-test trace spans. An absolute mess of a workaround that is completely unnecessary.

And of course, in the process of just duplicating everything from existing working libraries, you also decided it was a good idea to just partially replicate some existing OTEL resource detectors inside of your AddTestingPlatformResource detector... duplicating tons of process, host, os, service metadata... now we'll need to be very careful about you overriding standard OTEL attributes like service.name, service.version that we have already set through other means, with your test resource, which should have no concern with any of those things. If people wanted host, process, os, etc resources, they could just consume OpenTelemetry.Resources.OperatingSystem, OpenTelemetry.Resources.Host, OpenTelemetry.Resources.Process... but no, you thought it was a great idea to just replicate them inline in a hodgepodge resource detector and add some test attributes in there.

At least we can add your crazy resource detector before we call AddServiceDefaults() (a non-standard thing to do but here we are...) just so that your crazy logic for some of those things are overridden by the proper existing resource detectors.

Proposed Feature

  1. Make MTP raise Activity and Meter calls natively instead of requiring an "OpenTelemetry" library for that to happen. Follow all other .NET libraries here please. This would honestly solve most problems here.
  2. Use/promote existing ResourceDetector packages (Host, OperatingSystem, Process, etc) instead of reinventing all that logic inside of your "test" detector. Keep the test detector for test-related attributes only (this should be obvious but I have no idea why you guys decided to cram so much unrelated stuff in there)! Create a separate OpenTelemetry.Resources.SourceControl package if needed to put your vcs attributes in that can be reused in other applications as well and is not test-specific.
  3. Stop trying to "take ownership" of TraceProvider and MeterProvider MTP-specific instances and use the existing OpenTelemetry.Extensions.Hosting APIs for that purpose. Every single other application is built on top of that.

Basically: make a MTP project the same as any other project.

Test projects are the only project type that has a completely custom hosting model, yet it now shares its execution with all other project types:

  • MTP projects are executable, like any CLI/Worker project
  • MTP projects now honor launchsettings.json, like any CLI/Worker project

Alternative Designs

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

area/mtp-observabilityMTP OpenTelemetry, telemetry, and logging extensions.

Projects

No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions