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

[repo] Bump Microsoft.Extensions.Options to 6.0.0+ #4054

Closed

Conversation

CodeBlanch
Copy link
Member

Relates to #4046

OpenTelemetry SDK has a dependency on Microsoft.Extensions.Options package. The current version specified in Common.props is 5.0.0 and above. The version 5.0.0 of Microsoft.Extensions.Options package is deprecated and no longer maintained: https://www.nuget.org/packages/Microsoft.Extensions.Options/5.0.0

Changes

  • Update package version of Microsoft.Extensions.Options to [6.0.0,)
  • Add Microsoft.Extensions.DependencyInjection reference at [6.0.0,) to SDK to resolve conflicts

@CodeBlanch CodeBlanch requested a review from a team January 3, 2023 21:47
<MicrosoftExtensionsLoggingConfigurationPkgVer>[3.1.0,)</MicrosoftExtensionsLoggingConfigurationPkgVer>
<MicrosoftExtensionsOptionsPkgVer>[5.0.0,)</MicrosoftExtensionsOptionsPkgVer>
<MicrosoftExtensionsLoggingConfigurationPkgVer>$(MicrosoftExtensionsLoggingPkgVer)</MicrosoftExtensionsLoggingConfigurationPkgVer>
<MicrosoftExtensionsOptionsPkgVer>[6.0.0,)</MicrosoftExtensionsOptionsPkgVer>
Copy link
Member

Choose a reason for hiding this comment

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

I recall there was a reason we needed 5+, but can you remind me why we couldn't have used 3.1.0?

Copy link
Member Author

Choose a reason for hiding this comment

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

We use this CreateInstance method (which was added in 5.0) to fire delegates when options are retrieved which is used for the retrieve-environment-variables-from-iconfiguration feature.

@codecov
Copy link

codecov bot commented Jan 3, 2023

Codecov Report

Merging #4054 (5866fa8) into main (00314ac) will increase coverage by 0.04%.
The diff coverage is n/a.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #4054      +/-   ##
==========================================
+ Coverage   85.41%   85.45%   +0.04%     
==========================================
  Files         289      289              
  Lines       11229    11229              
==========================================
+ Hits         9591     9596       +5     
+ Misses       1638     1633       -5     
Impacted Files Coverage Δ
...emetry.Api/Internal/OpenTelemetryApiEventSource.cs 76.47% <0.00%> (-5.89%) ⬇️
...p/Implementation/HttpInstrumentationEventSource.cs 72.00% <0.00%> (-4.00%) ⬇️
src/OpenTelemetry/Logs/Pool/LogRecordSharedPool.cs 100.00% <0.00%> (+21.05%) ⬆️

Copy link
Member

@alanwest alanwest left a comment

Choose a reason for hiding this comment

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

@CodeBlanch based on our SIG conversation, did we decide to punt on making this change, or do you think we should still proceed?

@CodeBlanch
Copy link
Member Author

@alanwest Discussed this with @tarekgh. The guidance we received is:

  • For minor releases, don't have to force upgrade the deprecated package reference.
  • For major releases, best practice is to upgrade the deprecated package references to the lowest supported LTS version.

I'm going to close this based on that.

@CodeBlanch CodeBlanch closed this Jan 6, 2023
@CodeBlanch CodeBlanch deleted the mse-options-version-bump branch January 6, 2023 17:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants