Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Core] Upgrading dependency on System.Diagnostics.DiagnosticSource #37418

Merged
merged 8 commits into from
Jul 7, 2023

Conversation

m-redding
Copy link
Member

@m-redding m-redding commented Jul 5, 2023

Summary of changes

These changes upgrade the global version of System.Diagnostics.DiagnosticSource from 4.6.0 to 6.0.1. It introduces multi-targeting for this package in Azure.Core to keep 4.6.0 for netcoreapp2.1.

This PR also makes some necessary but unrelated changes to MessageClientDiagnostics due to build failures.

Context

  • We are working towards making our repo Native AOT compatible: see this issue for more info
  • As part of this work, we need to upgrade our dependency on System.Diagnostics.DiagnosticSource from 4.6.0 to at least 5.0.0 so that we can access the new types added in version 5.0.0 directly rather than using reflection. There is more information in this issue.
  • System.Diagnostics.DiagnosticSource 5.0.0 is deprecated, so we should only take a dependency on 6.0.0+
  • Our target for netcoreapp2.1 prevents 6.0.1 from building because of a transitive dependency. The discussed solution is to multi-target with 4.6.0 and use preprocessor directives.

Functions considerations:

  • V4 supports System.Diagnostics.DiagnosticSource 6.0.x, so if we upgrade to 6.0.1 we won't have any issues. Upgrading to 7.0.2 would break functions integration so we can't do that.

PowerShell considerations

  • The latest version of PowerShell 7 takes a dependency on System.Diagnostics.DiagnosticSource 7.0.2, so we shouldn't have any issues with upgrading to 6.0.1 with new versions of PowerShell

Other considerations

  • Due to the modulated nature of our repository, we cannot make the changes needed to support the AOT work in one release. We need to release with the upgraded dependencies and then add in the new types.
    • This is because the new types are being used in Azure.Core shared source. This leads to a strange issue where any SDK using a package reference on Azure.Core will have only loaded the old System.Diagnostics.DiagnosticSource 4.6.0 from the Azure.Core 1.33 package. The targets file here is adding the source code that is changed without pulling in the upgraded dependencies, hence why all sorts of builds will break if we introduce this change in one go

Contributing to the Azure SDK

Please see our CONTRIBUTING.md if you are not familiar with contributing to this repository or have questions.

For specific information about pull request etiquette and best practices, see this section.

@azure-sdk
Copy link
Collaborator

API change check

API changes are not detected in this pull request.

@m-redding m-redding changed the title [Core] Testing Upgrading Dependency [Core] Upgrading System.Diagnostics.DiagnosticSource Jul 6, 2023
@m-redding m-redding changed the title [Core] Upgrading System.Diagnostics.DiagnosticSource [Core] Upgrading dependency on System.Diagnostics.DiagnosticSource Jul 6, 2023
@m-redding m-redding marked this pull request as ready for review July 6, 2023 23:25
@m-redding m-redding requested review from jsquire and heaths July 6, 2023 23:25
sdk/core/Azure.Core/src/Azure.Core.csproj Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants