-
Notifications
You must be signed in to change notification settings - Fork 295
Remove EnableAdaptiveSampling and replace with TracesPerSecond and SamplingRatio #3085
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
Conversation
There was a problem hiding this 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 removes the EnableAdaptiveSampling option from the .NET Core Application Insights integrations and replaces it with the more granular TracesPerSecond and SamplingRatio options, updating implementation, configuration, tests, and docs accordingly.
Changes:
- Remove
ApplicationInsightsServiceOptions.EnableAdaptiveSamplingfrom ASP.NET Core and WorkerService, including all test and config usages. - Introduce
TracesPerSecondandSamplingRatioonApplicationInsightsServiceOptions, validate their ranges, and wire them intoAzureMonitorExporterOptionsfor both ASP.NET Core and WorkerService. - Update configuration JSON files, integration fixtures, shared configuration tests, public API baselines, and documentation (README, WorkerService guide, breaking changes, changelog) to reflect the new sampling configuration model.
Reviewed changes
Copilot reviewed 25 out of 25 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| NETCORE/test/WorkerIntegrationTests.Tests/WorkerHostFixture.cs | Ensures worker integration tests now force 100% telemetry via SamplingRatio = 1.0f instead of disabling adaptive sampling. |
| NETCORE/test/Shared/ConfigurationTests.cs | Adds/updates shared configuration tests to validate TracesPerSecond and SamplingRatio binding, default exporter sampling behavior, and ignoring invalid sampling values; also repurposes older adaptive-sampling tests. |
| NETCORE/test/Microsoft.ApplicationInsights.WorkerService.Tests/content/config-all-settings-true.json | Removes obsolete EnableAdaptiveSampling from WorkerService “all settings true” JSON config. |
| NETCORE/test/Microsoft.ApplicationInsights.WorkerService.Tests/content/config-all-settings-false.json | Removes obsolete EnableAdaptiveSampling from WorkerService “all settings false” JSON config. |
| NETCORE/test/Microsoft.ApplicationInsights.WorkerService.Tests/content/config-all-default.json | Removes obsolete EnableAdaptiveSampling from WorkerService default JSON config. |
| NETCORE/test/Microsoft.ApplicationInsights.WorkerService.Tests/ExtensionsTest.cs | Cleans up WorkerService extension tests by removing expectations around adaptive sampling processors and options. |
| NETCORE/test/Microsoft.ApplicationInsights.AspNetCore.Tests/content/config-all-settings-true.json | Removes obsolete EnableAdaptiveSampling from ASP.NET Core “all settings true” JSON config. |
| NETCORE/test/Microsoft.ApplicationInsights.AspNetCore.Tests/content/config-all-settings-false.json | Removes obsolete EnableAdaptiveSampling from ASP.NET Core “all settings false” JSON config. |
| NETCORE/test/Microsoft.ApplicationInsights.AspNetCore.Tests/content/config-all-default.json | Removes obsolete EnableAdaptiveSampling from ASP.NET Core default JSON config. |
| NETCORE/test/Microsoft.ApplicationInsights.AspNetCore.Tests/Extensions/ApplicationInsightsExtensionsTests/AddApplicationInsightsTelemetryTests.cs | Removes ASP.NET Core tests that asserted adaptive sampling processors and options, aligning tests with the new OpenTelemetry-based sampling model. |
| NETCORE/test/IntegrationTests.Tests/content/config-samplingratio.json | Adds integration test config with SamplingRatio set to 0.5 for validating sampling ratio config flow. |
| NETCORE/test/IntegrationTests.Tests/content/config-sampling-tracespersecond.json | Adds integration test config with TracesPerSecond set to 10.0 for validating rate-limited sampling config flow. |
| NETCORE/test/IntegrationTests.Tests/content/config-all-settings-true.json | Updates integration “all settings true” config to drop EnableAdaptiveSampling in favor of new defaults. |
| NETCORE/test/IntegrationTests.Tests/content/config-all-settings-false.json | Updates integration “all settings false” config to remove EnableAdaptiveSampling. |
| NETCORE/test/IntegrationTests.Tests/content/config-all-default.json | Updates integration default config to remove EnableAdaptiveSampling. |
| NETCORE/test/IntegrationTests.Tests/CustomWebApplicationFactory.cs | Forces integration web app tests to 100% telemetry by setting SamplingRatio = 1.0f instead of disabling adaptive sampling. |
| NETCORE/src/Shared/Extensions/ApplicationInsightsServiceOptions.cs | Removes EnableAdaptiveSampling and adds TracesPerSecond/SamplingRatio properties (with documentation) plus copies them in CopyPropertiesTo. |
| NETCORE/src/Microsoft.ApplicationInsights.WorkerService/ApplicationInsightsExtensions.cs | Wires WorkerService ApplicationInsightsServiceOptions to AzureMonitorExporterOptions, setting EnableLiveMetrics, respecting TracesPerSecond > 0, and applying valid SamplingRatio (0.0–1.0) while nulling TracesPerSecond when only SamplingRatio is set. |
| NETCORE/src/Microsoft.ApplicationInsights.AspNetCore/Extensions/ApplicationInsightsExtensions.cs | Mirrors the WorkerService sampling wiring for ASP.NET Core, removing the old EnableAdaptiveSampling logic and using TracesPerSecond/SamplingRatio to configure the exporter. |
| NETCORE/WorkerService.md | Updates WorkerService configuration example to remove EnableAdaptiveSampling, keeping QuickPulse as the primary example option. |
| NETCORE/Readme.md | Removes EnableAdaptiveSampling references and examples from ASP.NET Core documentation and migration guidance, aligning docs with the new sampling model. |
| CHANGELOG.md | Adds an unreleased note describing the removal of EnableAdaptiveSampling in favor of TracesPerSecond and SamplingRatio (with a minor naming/grammar typo noted). |
| BreakingChanges.md | Updates breaking changes documentation to mark EnableAdaptiveSampling as removed and documents new TracesPerSecond and SamplingRatio properties for both ASP.NET Core and WorkerService (with a small markdown formatting issue in one bullet). |
| .publicApi/Microsoft.ApplicationInsights.WorkerService.dll/Stable/PublicAPI.Unshipped.txt | Updates the WorkerService public API surface to remove EnableAdaptiveSampling and add TracesPerSecond/SamplingRatio accessors. |
| .publicApi/Microsoft.ApplicationInsights.AspNetCore.dll/Stable/PublicAPI.Unshipped.txt | Updates the ASP.NET Core public API surface similarly, removing EnableAdaptiveSampling and adding TracesPerSecond/SamplingRatio. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
...ORE/src/Microsoft.ApplicationInsights.AspNetCore/Extensions/ApplicationInsightsExtensions.cs
Outdated
Show resolved
Hide resolved
rajkumar-rangaraj
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved, left a minor suggestion for log.
Removes EnableAdaptiveSampling & adds the tracespersecond/sampling ratio for aspnetcore/workerservice