Skip to content

Conversation

martincostello
Copy link
Member

@martincostello martincostello commented Jun 12, 2025

#5973 (comment)

Changes

Refactoring to match runtime packages to the major version of a target framework.

Unfortunately we can't do this complete for net8.0 without making breaking changes as we have exposed functionality that depends on new APIs added in the System.Diagnostics.DiagnosticSource package for 9.0.0.

tracer = new(new(key.Name, key.Version, key.Tags));

this.Activity!.AddLink(new ActivityLink(context, tags));

As a compromise for net8.0, 8.0.x packages are used for all dependencies except System.Diagnostics.DiagnosticSource which uses 9.0.0 to preserve these use cases.

The downside here is that users running net8.0 will technically be out of support for that package from May 2026, but that's still the case if we do nothing.

We could mitigate this by moving the special case for net8.0 to depend on the 10.0.0 package in November with a new release for .NET 10 (if that works) or users can self-mitigate by adding an explicit reference to the 10.0.0 version of that package to their own application without upgrading to .NET 10 immediately.

Moving forward to preserve the intention of this PR we would need to accept that new functionality would be gated to upgrading to the matching version of .NET. For example, a new feature added in .NET 10 would require your application to net10.0 to use it.

Alternatively we could take an approach where we only expose new APIs to older runtimes when the version is itself an LTS version.

In that case, a new API added in .NET 10 would be supported in all previous versions as we know the package exposing it is LTS, but a new API added in .NET 11 would only be available to net11.0 and later as that is STS.

Merge requirement checklist

  • CONTRIBUTING guidelines followed (license requirements, nullable enabled, static analysis, etc.)
  • Unit tests added/updated
  • Appropriate CHANGELOG.md files updated for non-trivial changes
  • Changes in public API reviewed (if applicable)

@github-actions github-actions bot added infra Infra work - CI/CD, code coverage, linters dependencies Pull requests that update a dependency file pkg:OpenTelemetry.Exporter.OpenTelemetryProtocol Issues related to OpenTelemetry.Exporter.OpenTelemetryProtocol NuGet package pkg:OpenTelemetry.Extensions.Hosting Issues related to OpenTelemetry.Extensions.Hosting NuGet package labels Jun 12, 2025
Copy link

codecov bot commented Jun 12, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.72%. Comparing base (fe5e6bc) to head (65fc0b6).
✅ All tests successful. No failed tests found.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #6327      +/-   ##
==========================================
+ Coverage   86.66%   86.72%   +0.05%     
==========================================
  Files         258      258              
  Lines       11878    11878              
==========================================
+ Hits        10294    10301       +7     
+ Misses       1584     1577       -7     
Flag Coverage Δ
unittests-Project-Experimental 86.56% <ø> (-0.04%) ⬇️
unittests-Project-Stable 86.63% <ø> (+0.10%) ⬆️
unittests-Solution 86.59% <ø> (+0.21%) ⬆️
unittests-UnstableCoreLibraries-Experimental 85.87% <ø> (+0.09%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.
see 4 files with indirect coverage changes

Comment on lines 55 to 58
However, for .NET 8 we need to use the .NET 9 package as the following APIs have been used to expose user-facing functionality:
- System.Diagnostics.Activity.ctor(string, string, IEnumerable<KeyValuePair<string, object>>)
- System.Diagnostics.Activity.AddException()
- System.Diagnostics.Activity.AddLink()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe this could be phrased to indicate that we need to (and can) use the latest version.

We, most likely, can use 10.0.x for all targets, like we can use 9.0.x.

For that matter, I'd use a property specific for this package's version.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Depends on the resolution to this comment:

Moving forward to preserve the intention of this PR we would need to accept that new functionality would be gated to upgrading to the matching version of .NET. For example, a new feature added in .NET 10 would require your application to net10.0 to use it.

Alternatively we could take an approach where we only expose new APIs to older runtimes when the version is itself an LTS version.

The PR as-is is working on the assumption new things need newer TFMs/versions, so the MSBuild as-written is correct as it would only have an exception for net8.0 and code would need to use #if NET10_0_OR_GREATER-like constructs as appropriate for future API additions.

Copy link
Member Author

@martincostello martincostello Jun 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've also written it with #6307 in mind, where if this was merged as-is it would just add these changes:

- <RuntimePackageVersions>9.0.0</RuntimePackageVersions>
+ <RuntimePackageVersions>10.0.0</RuntimePackageVersions>
+ <RuntimePackageVersions Condition="'$(TargetFramework)' == 'net10.0'">10.0.0</RuntimePackageVersions>

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should be prepared, but I'd reason about TFM specific stuff when we get there.

Polyfills are always an option. 😄

@eerhardt
Copy link
Contributor

Build / verify-aot-compat / run-verify-aot-compat (ubuntu-22.04, net8.0) (pull_request)Failing after 1m

Try using 8.0.2 of Microsoft.Extensions.Configuration.Binder. The 8.0.0 package has bugs in the source generator that were fixed in patched versions.

Copy link
Contributor

@pjanotti pjanotti left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will help with conflict versions of Microsoft.Extensions.* when using opentelemetry-dotnet-auto

Copy link
Contributor

This PR was marked stale due to lack of activity and will be closed in 7 days. Commenting or pushing will instruct the bot to automatically remove the label. This bot runs once per day.

@github-actions github-actions bot added the Stale Issues and pull requests which have been flagged for closing due to inactivity label Jul 30, 2025
@martincostello martincostello removed the Stale Issues and pull requests which have been flagged for closing due to inactivity label Jul 30, 2025
Copy link
Contributor

github-actions bot commented Aug 7, 2025

This PR was marked stale due to lack of activity and will be closed in 7 days. Commenting or pushing will instruct the bot to automatically remove the label. This bot runs once per day.

@github-actions github-actions bot added the Stale Issues and pull requests which have been flagged for closing due to inactivity label Aug 7, 2025
@github-actions github-actions bot removed the Stale Issues and pull requests which have been flagged for closing due to inactivity label Aug 8, 2025
@martincostello martincostello force-pushed the gh-5973 branch 2 times, most recently from 5f99c8d to 921303a Compare August 26, 2025 07:42
@Kielek
Copy link
Contributor

Kielek commented Aug 27, 2025

@martincostello, @rajkumar-rangaraj, I have a question about last discussion topics. As I understand, the goal is to keep
.NET8 - referencing all Extenstions/MS libraries from 9.* branch,
.NET9 - referencing all Extenstions/MS libraries from 9.* branch,
.NET10 - referencing all Extenstions/MS libraries from 10.* branch.
.NET11 - referencing all Extenstions/MS libraries from 11.* branch.
.NET12 - referencing all Extenstions/MS libraries from 12.* branch.

Based on https://dotnet.microsoft.com/en-us/platform/support/policy/dotnet-core. It works for most cases, but keep in mind, that we can have a security issues coverage in that case:
image

In general, it will be great to have it implemented. The only concern I see is the half year period for lack of security fixes for .NET8.

@martincostello
Copy link
Member Author

The only concern I see is the half year period for lack of security fixes for .NET8.

Yeah that's an unfortunate side-effect of us wanting to make this change in an LTS year.

I think the mitigations to that are:

  1. The user can update their application to reference secure versions if necessary, they don't have to come from us via a transient reference. Users shouldn't rely on us to be the sole vehicle to patch their dependencies. For example, a user can enable CPVM's transitive pinning to raise the dependency version.
  2. It's the last 6 months of the .NET 8 support lifetime after 2.5 years, and they'll have had 9 months to be able to update to .NET 8.
  3. If there's a security patch that's relevant to us after May 2026, we can make a one-off exception to update net8.0 to target the 10.0.0 version for the final 6 months of its support lifetime.

@martincostello
Copy link
Member Author

I'm just writing up the issue discussed in yesterday's SIG, and the security issue is less of a concern as the PR as it currently stands only has System.Diagnostics.DiagnosticSource@9.0.0 used in net8.0, but when we add .NET 10 support that will move to System.Diagnostics.DiagnosticSource@10.0.0 for all TFMs, so when .NET 9 drops out of support in May 2026, users on the latest of the OpenTelemetry packages will only be using 9.0.x packages if they are still targeting net9.0 itself.

Users on .NET 8 for the last 6 months of its release cycle will be using 8.0.x packages (which will still receive security patches) except for System.Diagnostics.DiagnosticSource which will be 10.0.x, but that will also still be in support for patches.

Based on that, the only users affected by the end-of-life of .NET 9 will be users affected by that regardless of whether they use OpenTelemetry or not.

@martincostello martincostello marked this pull request as ready for review August 28, 2025 07:03
@Copilot Copilot AI review requested due to automatic review settings August 28, 2025 07:03
@martincostello martincostello requested a review from a team as a code owner August 28, 2025 07:03
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR refactors the OpenTelemetry .NET project to match runtime package versions to the major version of the target framework, addressing user requests to align dependency versions with their chosen .NET version (LTS vs STS). The change ensures that applications targeting specific .NET versions receive matching package versions, while providing flexibility for users who want to upgrade dependencies independently.

Key changes:

  • Runtime packages now align to target framework major version (e.g., net8.0 uses 8.0.x packages)
  • System.Diagnostics.DiagnosticSource maintains latest version across all frameworks to preserve OpenTelemetry functionality
  • Special handling for net8.0 with Microsoft.Extensions.Configuration.Binder upgraded to 8.0.2

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
Directory.Packages.props Updates package versioning strategy to match runtime versions to target frameworks
src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OpenTelemetry.Exporter.OpenTelemetryProtocol.csproj Adds explicit Microsoft.Extensions.Configuration.Binder upgrade for net8.0
test/OpenTelemetry.Extensions.Hosting.Tests/OpenTelemetry.Extensions.Hosting.Tests.csproj Enables System.Text.Json package references for net8.0 testing
test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests.csproj Enables System.Text.Json package references for net8.0 testing
issue-summary.md Adds comprehensive documentation explaining the dependency versioning strategy change

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Copy link

linux-foundation-easycla bot commented Sep 11, 2025

CLA Signed

The committers listed above are authorized under a signed CLA.

@martincostello
Copy link
Member Author

Squashed commits as EasyCLA is still complaining about commits co-authored by Copilot.

Use .NET 8 packages with `net8.0`, except `System.Diagnostics.DiagnosticSource`.

Resolves open-telemetry#5973.
Use syntax that would avoid renovate trying to update these versions (see open-telemetry#6459).
- Use syntax for "pinning" for `RuntimePackageVersions`.
- Simplify syntax for `OTelLatestStableVer`.
The previous syntax is relied upon for automation.
@eerhardt
Copy link
Contributor

FYI - we announced https://devblogs.microsoft.com/dotnet/dotnet-sts-releases-supported-for-24-months/. I don't think that changes the direction in this PR. But it is related information.

Add changes missing from previous merge commit.
@github-actions github-actions bot removed the pkg:OpenTelemetry.Extensions.Hosting Issues related to OpenTelemetry.Extensions.Hosting NuGet package label Sep 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file infra Infra work - CI/CD, code coverage, linters keep-open Prevents issues and pull requests being closed as stale pkg:OpenTelemetry.Exporter.OpenTelemetryProtocol Issues related to OpenTelemetry.Exporter.OpenTelemetryProtocol NuGet package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants