[OpenTelemetry] Remove vendored EnvironmentVariablesConfigurationProvider

[stevejgordon]

Fixes #7141
Contributes to #6380 as a pre-requisite simplification

Changes

• Remove vendored EnvironmentVariablesConfigurationProvider.
• Update dependencies to use Microsoft.Extensions.Configuration.EnvironmentVariables.
• Add basic E2E test to validate existing ENV VAR behaviour continues to function.

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)

[martincostello]

The change seems OK to me from a code perspective, but I defer to the other maintainers for background on why this was done in the first place that might cause issues (e.g. the additional dependencies for opentelemetry-dotnet-instrumentation).

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.98%. Comparing base (7ef0ebc) to head (bd44b7f).
⚠️ Report is 7 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #7146   +/-   ##
=======================================
  Coverage   89.98%   89.98%           
=======================================
  Files         277      277           
  Lines       14622    14642   +20     
=======================================
+ Hits        13157    13176   +19     
- Misses       1465     1466    +1     

Flag	Coverage Δ
unittests-Project-Experimental	89.88% <ø> (-0.05%)	⬇️
unittests-Project-Stable	89.91% <ø> (+0.06%)	⬆️
unittests-Solution	89.94% <ø> (-0.03%)	⬇️
unittests-UnstableCoreLibraries-Experimental	48.83% <ø> (+0.02%)	⬆️

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

[stevejgordon]

Yeah if the driver is dependency reduction, then we may need to instead consider if we want a sync mechanism for the vendored code intead. I know the instrumentation team were doing work to improve dependency issues, so this may now be suitable.

[Kielek]

@open-telemetry/dotnet-instrumentation-approvers perspective:
after changes in 1.15.0 this additional dependency should not be a problem any longer. Even for .NET versions. It will increase a bit the distribution size.

I would not block this change only for AutoInstrumentation.

[github-actions]

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.

[stevejgordon]

I can update the tests if we still want to consider this change? I guess we need a final position from the @open-telemetry/dotnet-maintainers on taking this dependency within the SDK.

[github-actions]

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]

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.

[martincostello]

We discussed this in the SIG just now. We'd like a 👍 from @rajkumar-rangaraj, but so far there's been no dissenting opinions on taking this change.

[alanwest]

I recognize the CI isn't passing at the moment, but giving my 👍 based on conversation during our meeting today.

We were originally hesitant to take a dependency on Microsoft.Extensions.Configuration.EnvironmentVariable package because it leads us further down the slippery slope of taking on even more Microsoft.Extensions.* dependencies. If we had it to do all over again, we probably would have avoided all dependencies for the core SDK on Microsoft.Extensions.* packages.

We should continue to take a conservative approach with respect to dependencies, but I don't see a big issue with adding this one. I'd like to hear @rajkumar-rangaraj's input.

[rajkumar-rangaraj]

We were originally hesitant to take a dependency on Microsoft.Extensions.Configuration.EnvironmentVariable package because it leads us further down the slippery slope of taking on even more Microsoft.Extensions.* dependencies. If we had it to do all over again, we probably would have avoided all dependencies for the core SDK on Microsoft.Extensions.* packages.

Thanks @alanwest for adding the context here. I'm fine with the current approach.

[github-actions]

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.

[stevejgordon]

I'll update the tests as recommended and rebase ready for a final review and merge