Skip to content

[Sampler.AWS] Fix flaky tests#4297

Merged
Kielek merged 2 commits intoopen-telemetry:mainfrom
martincostello:fix-flaky-tests
Apr 30, 2026
Merged

[Sampler.AWS] Fix flaky tests#4297
Kielek merged 2 commits intoopen-telemetry:mainfrom
martincostello:fix-flaky-tests

Conversation

@martincostello
Copy link
Copy Markdown
Member

Changes

  • Fix flaky tests by refactoring the handling of the background timer.
  • Avoid allocations when creating a client ID (spotted while reviewing the above changes).
  • Remove some code duplication in the tests.
  • Avoid private reflection.
  • Increase SpinWait timeout to avoid a different flaky test.

The parts around removing async void were suggested by Copilot when I asked it to investigate the issue.

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)

- Fix flaky tests by refactoring the handling of the background timer.
- Avoid allocations when creating a client ID.
- Remove some code duplication in the tests.
- Avoid private reflection.
- Increase `SpinWait` timeout.
@github-actions github-actions Bot added comp:sampler.aws Things related to OpenTelemetry.Samplers.AWS comp:instrumentation.aspnetcore Things related to OpenTelemetry.Instrumentation.AspNetCore labels Apr 24, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 24, 2026

Codecov Report

❌ Patch coverage is 87.30159% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 75.12%. Comparing base (a8d688f) to head (af1b6bd).
⚠️ Report is 42 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
.../OpenTelemetry.Sampler.AWS/AWSXRayRemoteSampler.cs 87.30% 8 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #4297      +/-   ##
==========================================
- Coverage   75.28%   75.12%   -0.17%     
==========================================
  Files         467      467              
  Lines       18449    18493      +44     
==========================================
+ Hits        13889    13892       +3     
- Misses       4560     4601      +41     
Flag Coverage Δ
unittests-Contrib.Shared.Tests 89.38% <ø> (ø)
unittests-Exporter.Geneva 54.70% <ø> (-0.44%) ⬇️
unittests-Exporter.InfluxDB 95.81% <ø> (ø)
unittests-Exporter.Instana 74.86% <ø> (ø)
unittests-Exporter.OneCollector 94.61% <ø> (ø)
unittests-Extensions 90.78% <ø> (ø)
unittests-Extensions.Enrichment 100.00% <ø> (ø)
unittests-Extensions.Enrichment.AspNetCore 86.27% <ø> (ø)
unittests-Extensions.Enrichment.Http 94.33% <ø> (ø)
unittests-Instrumentation.AWS 83.50% <ø> (ø)
unittests-Instrumentation.AspNet 76.61% <ø> (ø)
unittests-Instrumentation.AspNetCore 70.44% <ø> (ø)
unittests-Instrumentation.Cassandra 92.85% <ø> (ø)
unittests-Instrumentation.ConfluentKafka 78.52% <ø> (ø)
unittests-Instrumentation.ElasticsearchClient 80.60% <ø> (ø)
unittests-Instrumentation.EntityFrameworkCore 81.39% <ø> (ø)
unittests-Instrumentation.EventCounters 77.27% <ø> (ø)
unittests-Instrumentation.GrpcCore 91.27% <ø> (ø)
unittests-Instrumentation.GrpcNetClient 73.78% <ø> (ø)
unittests-Instrumentation.Hangfire 88.91% <ø> (ø)
unittests-Instrumentation.Http 74.62% <ø> (ø)
unittests-Instrumentation.Owin 88.62% <ø> (ø)
unittests-Instrumentation.Process 100.00% <ø> (ø)
unittests-Instrumentation.Quartz 78.76% <ø> (ø)
unittests-Instrumentation.Remoting 64.28% <ø> (ø)
unittests-Instrumentation.Runtime 100.00% <ø> (ø)
unittests-Instrumentation.ServiceFabricRemoting 40.83% <ø> (ø)
unittests-Instrumentation.SqlClient 84.56% <ø> (ø)
unittests-Instrumentation.StackExchangeRedis 93.63% <ø> (ø)
unittests-Instrumentation.Wcf 80.77% <ø> (ø)
unittests-OpAmp.Client 83.14% <ø> (ø)
unittests-PersistentStorage 68.81% <ø> (-3.22%) ⬇️
unittests-Resources.AWS 74.49% <ø> (ø)
unittests-Resources.Azure 88.31% <ø> (ø)
unittests-Resources.Container 67.34% <ø> (ø)
unittests-Resources.Gcp 71.42% <ø> (ø)
unittests-Resources.Host 72.26% <ø> (ø)
unittests-Resources.OperatingSystem 76.98% <ø> (ø)
unittests-Resources.Process 100.00% <ø> (ø)
unittests-Resources.ProcessRuntime 79.59% <ø> (ø)
unittests-Sampler.AWS 96.39% <87.30%> (-1.14%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
.../OpenTelemetry.Sampler.AWS/AWSXRayRemoteSampler.cs 88.23% <87.30%> (-3.77%) ⬇️

... and 8 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.

@martincostello martincostello marked this pull request as ready for review April 24, 2026 12:54
@martincostello martincostello requested a review from a team as a code owner April 24, 2026 12:54
Copilot AI review requested due to automatic review settings April 24, 2026 12:54
Copy link
Copy Markdown
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 aims to reduce flakiness in the AWS X-Ray remote sampler tests by refactoring the sampler’s background polling/timer handling, along with a few test-stability improvements across the test suite.

Changes:

  • Refactors AWSXRayRemoteSampler polling callbacks and disposal behavior (timer handling, cancellation, locking) and reduces allocations for client-id generation.
  • Updates AWS sampler tests to reduce duplication and avoid reflective invocation of private methods.
  • Improves test robustness by broadening tolerated HttpListenerException cases and increasing a SpinWait timeout in ASP.NET Core tests.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
test/Shared/TestHttpServer.cs Treats an additional HttpListenerException error code as a benign shutdown/disconnect case.
test/OpenTelemetry.Sampler.AWS.Tests/TestAWSXRayRemoteSampler.cs Refactors tests to use a helper and calls the new internal async target-update method.
test/OpenTelemetry.Instrumentation.AspNetCore.Tests/BasicTests.cs Increases export wait timeout to reduce flakiness under parallel load.
src/OpenTelemetry.Sampler.AWS/AWSXRayRemoteSampler.cs Refactors polling execution/disposal and updates client-id generation to reduce allocations.

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

Comment thread src/OpenTelemetry.Sampler.AWS/AWSXRayRemoteSampler.cs Outdated
Avoid potential deadlock.
@Kielek Kielek added this pull request to the merge queue Apr 30, 2026
Merged via the queue into open-telemetry:main with commit 41a289c Apr 30, 2026
316 checks passed
@martincostello martincostello deleted the fix-flaky-tests branch April 30, 2026 10:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp:instrumentation.aspnetcore Things related to OpenTelemetry.Instrumentation.AspNetCore comp:sampler.aws Things related to OpenTelemetry.Samplers.AWS

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants