fix: clear OpenCensus execution context in TestPatch setUp/tearDown#5262
Open
sridhar-3009 wants to merge 2 commits into
Open
fix: clear OpenCensus execution context in TestPatch setUp/tearDown#5262sridhar-3009 wants to merge 2 commits into
sridhar-3009 wants to merge 2 commits into
Conversation
…th single MetricsData field The callback → _receive_metrics → add_metrics_data → deque → yield chain always held at most one MetricsData per collect cycle (the callback triggers exactly one _receive_metrics call before collect drains the queue). Replace the deque[MetricsData] with an Optional[MetricsData] field and simplify collect() to consume it directly, removing the while-loop, popleft(), len() check, and the now-unused `deque` import. This fixes a latent bug where multiple items in the deque would have caused collect() to yield duplicate Prometheus metric families. Fixes open-telemetry#2500
The flaky test_install_shim_affects_existing_tracers failure on Windows was caused by stale OpenCensus thread-local state (a ContextTracer set by a previous test, e.g. test_shim_with_sdk.py) leaking into TestPatch. When that stale tracer was present, a bare Tracer() resolved to ContextTracer instead of the expected NoopTracer. Add execution_context.clean() to setUp (after uninstall_shim) and tearDown to fully reset the OC thread-local store between tests. Fixes open-telemetry#3975
|
This PR has been automatically marked as stale because it has not had any activity for 14 days. It will be closed if no further activity occurs within 14 days of this comment. |
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
1 participant
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.
Problem
test_install_shim_affects_existing_tracersfails non-deterministically, most often on Windows, with:Root cause
TestPatch.setUpcallsuninstall_shim()to remove the monkeypatch, but does not clear the OpenCensus thread-local execution context. Whentest_shim_with_sdk.pyruns beforetest_patch.py(e.g. due to alphabetical ordering on Windows), it leaves a liveContextTracerin the OC thread-local store viaexecution_context. A subsequent bareTracer()intest_install_shim_affects_existing_tracersreads that stale context and produces aContextTracerinstead of the expectedNoopTracer.Fix
Call
execution_context.clean()in bothsetUp(afteruninstall_shim) andtearDownso each test starts and ends with a fully reset OC thread-local state.Testing
All 6 tests in
TestPatchpass locally.Fixes #3975