Reducing transitive dependency closure of System.Diagnostics.DiagnosticSource

@noahfalk

This is a question / suggestion / request about the current RC of System.Diagnostics.DiagnosticSource.

Auto-instrumentation scenarios require an in-proc agent / tracer to load DiagnosticSource dynamically while performing non-trivial logic to avoid potential versioning conflicts with assemblies referenced by the application being instrumented. The logic would be considerably simpler and more robust if the DiagnosticSource-assembly itself did not have any additional dependencies.

On the recent NetFx and Net Core versions the System.Diagnostics.DiagnosticSource-NuGet depends on System.Memory.(When targeting older Net Core and Net Standard flavors there are more NuGet dependencies.) In turn, System.Memory requires (depending on the targeted runtime version): System.Runtime.CompilerServices.Unsafe, System.Buffers and System.Numerics.Vectors. Older runtime flavors require more.

I am guessing that the dependencies are on actor types rather than on data exchange types on public APIs. If so, it should be possible to build the types that are referenced from System.Memory and System.Runtime.CompilerServices.Unsafe directly into the DiagnosticSource-assembly (copy code or, better, just reference the respective sources at Framework build time). That way DiagnosticSource-assembly would not contain any downstream dependencies.

This would considerably simplify scenarios where an auto-instrumentation agent needs to dynamically load DiagnosticSource (and all its dependencies) without causing versioning conflicts with the instrumented application.

@noahfalk and @davmason have context on the related discussion in the OTel SIG.

This should be a build-logic change only. Can this be considered for Net 5?

Can this be considered for Net 5?

Sorry it is too late : ( At the runtime and BCL layers of the stack we are already winding down for the release. We can consider this for .NET 6. Also I believe some of those types are special-cased JIT intrinsics - copying the source to another assembly may cause significant performance regressions or outright failures because the JIT no longer recognizes the special casing applies.

I am guessing that the dependencies are on actor types rather than on data exchange types on public APIs.

This does not appear to be the case:

    public readonly partial struct ActivitySpanId : System.IEquatable<System.Diagnostics.ActivitySpanId>
    {
        public static System.Diagnostics.ActivitySpanId CreateFromBytes(System.ReadOnlySpan<byte> idData);
        public static System.Diagnostics.ActivitySpanId CreateFromString(System.ReadOnlySpan<char> idData);
        public static System.Diagnostics.ActivitySpanId CreateFromUtf8String(System.ReadOnlySpan<byte> idData);
    }

System.ReadOnlySpan<T> lives in System.Memory.

#429 looks similar.

@KalleOlaviNiemitalo

Yes, @KalleOlaviNiemitalo , it does, indeed appear to be the same issue. Looks like it's not super new. Since then we have more scenarios that highlight the problem (auto-instrumentation), wished we realized it in time. :)

@jkotas

@jkotas , thank you for spotting this, it's a bummer. I really wished we could instead use an API shape that takes an array and an index. It's not as modern as a ReadOnlySpan, but the dependencies are a real challenge (details below).

Btw, the latest RC takes a direct additional dependency on System.Runtime.CompilerServices.Unsafe (it existed before transitively via System.Memory). Is it one of those types that have special-cased JIT intrinsics (see @noahfalk 's comment above), or could a copy of it be theoretically built into the DiagnosticSource Nuget? (This is for my own education.)

The challenge presented by these dependencies to auto-instrumentation is not only that an auto-instrumentation agent becomes more complex. It overall limits the scenarios it can cover:

If DiagnosticSource did not have dependencies, we can use a logic such as (simplified):

When the auto-instrumentation attaches, if DiagnosticSource is loaded, use the loaded version.
Otherwise load a relatively recent one as a "fall-back" strategy.
If an app was going to load DiagnisticSource later, it is still OK because of the stability of DiagnisticSource in respect to back-compatibility. It is generally OK for apps to use a newer version than they expected.

Consider now that an app did not have DiagnosticSource loaded, but it had an older version of System.Memory (or another dependency) loaded. If that version is older than required by our "fall-back" DiagnosticSource, we cannot load it. So applications that reference old versions of DiagnosticSource's dependencies without referencing a similarly old version of DiagnosticSource itself cannot be instrumented. (I'd love to find out that I am missing something, so please correct me if I am wrong. :) )

An even bigger issue (because it cannot be pre-preemptively detected) is that an app may want to load one of the DiagnosticSource dependencies after the tracer is attached. By then we have loaded the "fall-back", i.e. a recent DiagnosticSource version with respectively recent versions of its dependencies. Consider now that an app wanted to load an old version System.Memory (or another DiagnosticSource dependency). Instead, it will get a more recent version forced on it. If there is some backward incompatibility, there can be a very hard to debug issue or crash.

I will be researching workarounds and I would appreciate any thoughts you guys may have to share. Until then, I am somewhat concerned that these dependencies may put at risk the entire idea to have auto-instrumentation agents (incl. the OTel tracer) be Activity centric.

Thanks!

System.Runtime.CompilerServices.Unsafe (it existed before transitively via System.Memory). Is it one of those types that have special-cased JIT intrinsics

System.Runtime.CompilerServices.Unsafe does not have special-cased JIT intrinsics.

@noahfalk

@noahfalk - Could you please point to some information about

Also I believe some of those types are special-cased JIT intrinsics - copying the source to another assembly may cause significant performance regressions or outright failures because the JIT no longer recognizes the special casing applies.

As discussed in the OTel meeting, vendoring DiagnosticSource into a Tracer may be a fallback strategy when it is not on the default assembly loading path, and cannot be loaded dynamically from there. I'd like to understand what risks are associated with this.

Thank you!

I'm hoping we can stick to a simple rule of thumb, "don't transitively replicate any of DiagnosticSource.dll's dependencies." For any given dependency assembly we can either eliminate the requirement for it or accept that it will be needed for the scenario to work.

More info about intrinsics: https://github.com/dotnet/runtime/blob/master/docs/design/coreclr/botr/vectors-and-intrinsics.md

@jkotas

@jkotas , thank you for spotting this, it's a bummer. I really wished we could instead use an API shape that takes an array and an index. It's not as modern as a ReadOnlySpan, but the dependencies are a real challenge (details below).

Btw, the latest RC takes a direct additional dependency on System.Runtime.CompilerServices.Unsafe (it existed before transitively via System.Memory). Is it one of those types that have special-cased JIT intrinsics (see @noahfalk 's comment above), or could a copy of it be theoretically built into the DiagnosticSource Nuget? (This is for my own education.)

The challenge presented by these dependencies to auto-instrumentation is not only that an auto-instrumentation agent becomes more complex. It overall limits the scenarios it can cover:

If DiagnosticSource did not have dependencies, we can use a logic such as (simplified):

When the auto-instrumentation attaches, if DiagnosticSource is loaded, use the loaded version.

Otherwise load a relatively recent one as a "fall-back" strategy.

If an app was going to load DiagnisticSource later, it is still OK because of the stability of DiagnisticSource in respect to back-compatibility. It is generally OK for apps to use a newer version than they expected.

Consider now that an app did not have DiagnosticSource loaded, but it had an older version of System.Memory (or another dependency) loaded. If that version is older than required by our "fall-back" DiagnosticSource, we cannot load it. So applications that reference old versions of DiagnosticSource's dependencies without referencing a similarly old version of DiagnosticSource itself cannot be instrumented. (I'd love to find out that I am missing something, so please correct me if I am wrong. :) )

An even bigger issue (because it cannot be pre-preemptively detected) is that an app may want to load one of the DiagnosticSource dependencies after the tracer is attached. By then we have loaded the "fall-back", i.e. a recent DiagnosticSource version with respectively recent versions of its dependencies. Consider now that an app wanted to load an old version System.Memory (or another DiagnosticSource dependency). Instead, it will get a more recent version forced on it. If there is some backward incompatibility, there can be a very hard to debug issue or crash.

I will be researching workarounds and I would appreciate any thoughts you guys may have to share. Until then, I am somewhat concerned that these dependencies may put at risk the entire idea to have auto-instrumentation agents (incl. the OTel tracer) be Activity centric.

Thanks!

Backward incompatibility is a headache. You know how to decide whether the version of a package is backward compatible or the API has changed

Here are the packages I see still references S.R.CS.U in the dotnet-7 feeds so far:

System.Diagnostics.DiagnosticSource: removing it would reduce it to 0 package dependencies 🎉.
System.Collections.Immutable: removing it would reduce it to 0 package dependencies 🎉.

Needless packages in dotnet/efcore (there may be other repos that reference the package versions of what is in Microsoft.NETCore.App that needs changed:

System.Diagnostics.DiagnosticSource
System.Collections.Immutable

Other work that needs done (I think): Migrate all common package references Microsoft.Extensions.* (in the org and outside of it) to an new default installed runtime & ref pack and name it Microsoft.NETCore.Extensions.App and have dotnet/aspnetcore reference that instead of bundling them inside it's framework (so other types of apps can reference the Extensions FrameworkReference implicitly from the sdk with UseNetCoreExtensions=true in csproj/Directory.Build.props). This would greatly simplify infrastructure for everyone as well as enable RID based crossgen2 deployments of the Microsoft.Extensions.* projects.

Due to lack of recent activity, this issue has been marked as a candidate for backlog cleanup. It will be closed if no further activity occurs within 14 more days. Any new comment (by anyone, not necessarily the author) will undo this process.

This process is part of our issue cleanup automation.

Due to lack of recent activity, this issue has been marked as a candidate for backlog cleanup. It will be closed if no further activity occurs within 14 more days. Any new comment (by anyone, not necessarily the author) will undo this process.

This process is part of our issue cleanup automation.

Navigation Menu

Repository navigation

Reducing transitive dependency closure of System.Diagnostics.DiagnosticSource #42244

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Reducing transitive dependency closure of System.Diagnostics.DiagnosticSource #42244

Description

Activity

noahfalk commented on Sep 15, 2020

jkotas commented on Sep 15, 2020

KalleOlaviNiemitalo commented on Sep 15, 2020

macrogreg commented on Sep 15, 2020

macrogreg commented on Sep 15, 2020

jkotas commented on Sep 15, 2020

macrogreg commented on Sep 22, 2020

noahfalk commented on Sep 23, 2020

Smartisa commented on Mar 5, 2022

AraHaan commented on Mar 20, 2022

dotnet-policy-service commented on Oct 19, 2026

dotnet-policy-service commented on Oct 19, 2026

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions