Apply sample rate to all profiler subsystem telemetry to bypass ingestion sampling#4786
Open
johnoliver wants to merge 7 commits into
Open
Apply sample rate to all profiler subsystem telemetry to bypass ingestion sampling#4786johnoliver wants to merge 7 commits into
johnoliver wants to merge 7 commits into
Conversation
…stion sampling
Make SAMPLE_RATE_TO_DISABLE_INGESTION_SAMPLING public and set it on the
ServiceProfilerIndex event, the StartProfiler/StopProfiler messages, and
GcEvent telemetry. These events are emitted directly via TelemetryClient
(not through the trace sampler), so previously they had no sample rate set,
leaving them eligible for backend ingestion sampling. Using 99.99 keeps the
item count at 1 while ensuring the events are not dropped.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
johnoliver
requested review from
harsimar,
mattsains-msft,
rajkumar-rangaraj,
ramthi,
trask and
xiang17
as code owners
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the profiler subsystem’s telemetry emission to explicitly set a sample rate intended to bypass ingestion-time sampling, ensuring key profiler-related events (GC events, ServiceProfilerIndex, and certain message telemetry) are always ingested. It also exposes the shared “disable ingestion sampling” sample-rate constant and adds a unit test to validate the GC event behavior.
Changes:
- Set
sampleRateonGcEvent,ServiceProfilerIndex, and selected message telemetry toSamplerUtil.SAMPLE_RATE_TO_DISABLE_INGESTION_SAMPLING. - Expose
SAMPLE_RATE_TO_DISABLE_INGESTION_SAMPLINGas a public constant for reuse across modules. - Add a targeted unit test to assert
GcEventtelemetry is emitted with the expected sample rate.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| agent/agent-tooling/src/test/java/com/microsoft/applicationinsights/agent/internal/profiler/triggers/GcEventInitTest.java | Adds a unit test that observes emitted GcEvent telemetry and asserts the sample rate used to bypass ingestion sampling. |
| agent/agent-tooling/src/main/java/com/microsoft/applicationinsights/agent/internal/sampling/SamplerUtil.java | Makes the “disable ingestion sampling” sample-rate constant publicly accessible for shared use. |
| agent/agent-tooling/src/main/java/com/microsoft/applicationinsights/agent/internal/profiler/triggers/GcEventInit.java | Applies the bypass-ingestion sample rate to emitted GcEvent telemetry. |
| agent/agent-tooling/src/main/java/com/microsoft/applicationinsights/agent/internal/profiler/triggers/AlertingSubsystemInit.java | Applies the bypass-ingestion sample rate to ServiceProfilerIndex and message telemetry emitted by the alerting/profiler init path. |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ervers Root cause: TelemetryClient.trackAsync() returns early when connectionString is null, before notifying TelemetryObservers. The test built a TelemetryClient without a connection string, so the observer CompletableFuture was never completed, causing TimeoutException. Fix: supply a dummy connection string in the test builder so trackAsync() proceeds past the null check and correctly fires the registered observer. Test-only change; production code is unmodified.
Stabilize GcEventInitTest: fix deterministic TimeoutException in agent-tooling CI
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request ensures that specific telemetry events (
GcEvent,ServiceProfilerIndex, and certain message telemetry) are emitted with a sample rate that bypasses ingestion sampling, guaranteeing they are always ingested. It also makes the sample rate constant publicly accessible and adds a targeted unit test to verify this behavior.Telemetry sampling changes:
GcEvent,ServiceProfilerIndex, and related message telemetry toSamplerUtil.SAMPLE_RATE_TO_DISABLE_INGESTION_SAMPLING, ensuring these events bypass ingestion sampling (AlertingSubsystemInit.java,GcEventInit.java). [1] [2] [3]SAMPLE_RATE_TO_DISABLE_INGESTION_SAMPLINGa public constant inSamplerUtilfor use across modules (SamplerUtil.java).Testing improvements:
GcEventInitTest.javato verify thatGcEventtelemetry is emitted with the correct sample rate, including supporting test utilities and mocks (GcEventInitTest.java). [1] [2]Dependency updates:
SamplerUtilwhere needed (AlertingSubsystemInit.java,GcEventInit.java,GcEventInitTest.java). [1] [2] [3]