Skip to content

Conversation

@martincostello
Copy link
Member

Changes

Add EnvironmentVariableScope helper class for managing the scope of changes to environment variables in tests to ensure the environment is consistently reset between tests.

For example, #3411 sets multiple variables but does not reset them so state could easily bleed into other tests. With these changes the test could be trivially refactored to do so without requiring the test class to implement IDisposable and manually revert the changes.

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)

Add `EnvironmentVariableScope` helper class for managing the scope of changes to environment variables in tests to ensure the environment is consistently reset between tests.
@github-actions github-actions bot added comp:instrumentation.awslambda Things related to OpenTelemetry.Instrumentation.AWSLambda comp:instrumentation.aspnet Things related to OpenTelemetry.Instrumentation.AspNet comp:instrumentation.entityframeworkcore Things related to OpenTelemetry.Instrumentation.EntityFrameworkCore comp:instrumentation.owin Things related to OpenTelemetry.Instrumentation.Owin comp:instrumentation.sqlclient Things related to OpenTelemetry.Instrumentation.SqlClient comp:instrumentation.stackexchangeredis Things related to OpenTelemetry.Instrumentation.StackExchangeRedis comp:resources.aws Things related to OpenTelemetry.Resources.AWS comp:resources.azure Things related to OpenTelemetry.Resources.Azure labels Nov 7, 2025
@codecov
Copy link

codecov bot commented Nov 7, 2025

Codecov Report

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

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3412      +/-   ##
==========================================
- Coverage   70.79%   70.78%   -0.01%     
==========================================
  Files         440      440              
  Lines       17265    17265              
==========================================
- Hits        12222    12221       -1     
- Misses       5043     5044       +1     
Flag Coverage Δ
unittests-Contrib.Shared.Tests 86.54% <ø> (ø)
unittests-Exporter.Geneva 53.75% <ø> (-0.02%) ⬇️
unittests-Exporter.InfluxDB 95.14% <ø> (ø)
unittests-Exporter.Instana 74.86% <ø> (ø)
unittests-Exporter.OneCollector 94.62% <ø> (ø)
unittests-Extensions 90.65% <ø> (-0.47%) ⬇️
unittests-Extensions.Enrichment 100.00% <ø> (ø)
unittests-Extensions.Enrichment.AspNetCore 86.27% <ø> (ø)
unittests-Extensions.Enrichment.Http 94.33% <ø> (ø)
unittests-Instrumentation.AWS 83.59% <ø> (-0.22%) ⬇️
unittests-Instrumentation.AspNet 77.92% <ø> (ø)
unittests-Instrumentation.AspNetCore 71.69% <ø> (ø)
unittests-Instrumentation.Cassandra 23.52% <ø> (ø)
unittests-Instrumentation.ConfluentKafka 14.10% <ø> (ø)
unittests-Instrumentation.ElasticsearchClient 80.12% <ø> (ø)
unittests-Instrumentation.EntityFrameworkCore 80.80% <ø> (ø)
unittests-Instrumentation.EventCounters 77.27% <ø> (ø)
unittests-Instrumentation.GrpcCore 91.42% <ø> (ø)
unittests-Instrumentation.GrpcNetClient 79.61% <ø> (ø)
unittests-Instrumentation.Hangfire 84.61% <ø> (ø)
unittests-Instrumentation.Http 74.22% <ø> (ø)
unittests-Instrumentation.Owin 88.62% <ø> (ø)
unittests-Instrumentation.Process 100.00% <ø> (ø)
unittests-Instrumentation.Quartz 78.76% <ø> (ø)
unittests-Instrumentation.Runtime 100.00% <ø> (ø)
unittests-Instrumentation.ServiceFabricRemoting 34.54% <ø> (ø)
unittests-Instrumentation.SqlClient 87.29% <ø> (ø)
unittests-Instrumentation.StackExchangeRedis 71.80% <ø> (ø)
unittests-Instrumentation.Wcf 79.44% <ø> (-0.14%) ⬇️
unittests-OpAmp.Client 71.55% <ø> (ø)
unittests-PersistentStorage 76.58% <ø> (+1.33%) ⬆️
unittests-Resources.AWS 74.42% <ø> (ø)
unittests-Resources.Azure 85.31% <ø> (ø)
unittests-Resources.Container 67.34% <ø> (ø)
unittests-Resources.Gcp 71.42% <ø> (ø)
unittests-Resources.Host 71.85% <ø> (ø)
unittests-Resources.OperatingSystem 76.98% <ø> (ø)
unittests-Resources.Process 100.00% <ø> (ø)
unittests-Resources.ProcessRuntime 79.59% <ø> (ø)
unittests-Sampler.AWS 93.23% <ø> (ø)

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

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Instead of `Span<T>`.
Add a `using` to ensure the environment variables are reset before another test runs, rather than outside the scope of the test when the instance is disposed.
@martincostello martincostello marked this pull request as ready for review November 7, 2025 11:53
@martincostello martincostello requested a review from a team as a code owner November 7, 2025 11:53
Copilot AI review requested due to automatic review settings November 7, 2025 11:53
Copy link
Contributor

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 introduces a shared utility class EnvironmentVariableScope to improve test isolation by managing environment variables with proper cleanup. The PR refactors multiple test files across the codebase to use this utility instead of manually managing environment variables.

  • Adds EnvironmentVariableScope utility class for automatic environment variable cleanup
  • Refactors test classes to use the new utility, removing manual IDisposable implementations and cleanup code
  • Improves test isolation by ensuring environment variables are always restored to their original state

Reviewed Changes

Copilot reviewed 21 out of 21 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
test/Shared/EnvironmentVariableScope.cs New shared utility class for managing scoped environment variables with automatic cleanup
test/OpenTelemetry.Resources.Azure.Tests/*.cs Refactored to use EnvironmentVariableScope, removed manual IDisposable implementation
test/OpenTelemetry.Resources.AWS.Tests/*.cs Refactored to use EnvironmentVariableScope, removed manual cleanup methods
test/OpenTelemetry.Instrumentation.StackExchangeRedis.Tests/*.cs Simplified SemanticConventionScope to use EnvironmentVariableScope
test/OpenTelemetry.Instrumentation.SqlClient.Tests/*.cs Refactored to use EnvironmentVariableScope for test setup and semantic convention scoping
test/OpenTelemetry.Instrumentation.Owin.Tests/*.cs Replaced try-finally blocks with EnvironmentVariableScope
test/OpenTelemetry.Instrumentation.EntityFrameworkCore.Tests/*.cs Simplified semantic convention scope management
test/OpenTelemetry.Instrumentation.AspNet.Tests/*.cs Replaced try-finally blocks with EnvironmentVariableScope
test/OpenTelemetry.Instrumentation.AWSLambda.Tests/*.cs Refactored constructor to use EnvironmentVariableScope for multiple environment variables
test/OpenTelemetry.Contrib.Shared.Tests/*.cs Refactored to use EnvironmentVariableScope, removed manual IDisposable implementation
Multiple .csproj files Added references to the shared EnvironmentVariableScope.cs file
opentelemetry-dotnet-contrib.sln Added EnvironmentVariableScope.cs to solution and reordered docker-compose entries

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Remove unused variables.
Only reset the environment once.
@martincostello martincostello added this pull request to the merge queue Nov 12, 2025
@Kielek Kielek removed this pull request from the merge queue due to a manual request Nov 12, 2025
@martincostello martincostello added this pull request to the merge queue Nov 12, 2025
Merged via the queue into open-telemetry:main with commit b9b5e2e Nov 12, 2025
320 checks passed
@martincostello martincostello deleted the env-var-test-helper branch November 12, 2025 08:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp:instrumentation.aspnet Things related to OpenTelemetry.Instrumentation.AspNet comp:instrumentation.awslambda Things related to OpenTelemetry.Instrumentation.AWSLambda comp:instrumentation.entityframeworkcore Things related to OpenTelemetry.Instrumentation.EntityFrameworkCore comp:instrumentation.owin Things related to OpenTelemetry.Instrumentation.Owin comp:instrumentation.sqlclient Things related to OpenTelemetry.Instrumentation.SqlClient comp:instrumentation.stackexchangeredis Things related to OpenTelemetry.Instrumentation.StackExchangeRedis comp:resources.aws Things related to OpenTelemetry.Resources.AWS comp:resources.azure Things related to OpenTelemetry.Resources.Azure

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants