Skip to content

Flaky BatchProcessor shutdown test fails on Windows/PyPy #5663

Description

@xrmx

Describe your environment

OS: Windows (GitHub Actions runner)
Python version: PyPy 3.10.16, x86
SDK version: repository checkout at commit fce3afd97f52101258e5b3376ec80b4201da3d80
API version: repository checkout at commit fce3afd97f52101258e5b3376ec80b4201da3d80

What happened?

The SDK test job intermittently fails in TestBatchProcessor::test_shutdown_allows_1_export_to_finish for BatchLogRecordProcessor because the test assumes the worker thread must still be alive immediately after BatchProcessor.shutdown() returns.

Relevant failure:

opentelemetry-sdk/tests/shared_internal/test_batch_processor.py::TestBatchProcessor::test_shutdown_allows_1_export_to_finish[BatchLogRecordProcessor-telemetry0] FAILED

>       assert processor._batch_processor._worker_thread.is_alive() is True
E       assert False is True

opentelemetry-sdk/tests/shared_internal/test_batch_processor.py:280: AssertionError

Captured log:
ERROR opentelemetry.sdk._shared_internal:__init__.py:189 Exception while exporting Log.
Traceback (most recent call last):
  File ".../opentelemetry/sdk/_shared_internal/__init__.py", line 187, in _export
    self._exporter.export(batch)
  File ".../tests/shared_internal/test_batch_processor.py", line 66, in export
    raise ValueError("Did not get to finish !")
ValueError: Did not get to finish !

1 failed, 871 passed, 19 skipped, 61 warnings, 47 subtests passed

BatchProcessor.shutdown() waits for the worker, marks the shutdown timeout as exceeded, and then invokes exporter.shutdown(). The test exporter sets an event that interrupts its active export. Depending on thread scheduling, especially on Windows/PyPy, the worker can therefore terminate before shutdown() returns. The equivalent span parameterization passed in this run, which is consistent with a scheduling race rather than a logs processor defect.

Steps to Reproduce

Run the SDK tests on Windows with PyPy 3.10, or repeatedly run:

pytest opentelemetry-sdk/tests/shared_internal/test_batch_processor.py::TestBatchProcessor::test_shutdown_allows_1_export_to_finish -x

The observed CI failure is from run 35034251815, job 104707203196, associated with PR #5647.

Expected Result

The test should reliably verify that shutdown respects its timeout, interrupts the active export through exporter shutdown, prevents a third export, and leaves the worker terminated—without depending on whether the worker is alive at one particular scheduling instant.

Actual Result

On Windows/PyPy, the worker sometimes finishes before shutdown() returns, so the timing-dependent assertion worker_thread.is_alive() is True fails even though shutdown behavior is otherwise correct. The mock exporter's ValueError("Did not get to finish !") is expected when shutdown interrupts the active export.

Additional context

CI failure: https://github.com/open-telemetry/opentelemetry-python/actions/runs/35034251815/job/104707203196?pr=5647

Proposed fix in opentelemetry-sdk/tests/shared_internal/test_batch_processor.py:

-        before = time.time()
+        before = time.monotonic()
         processor._batch_processor.shutdown(timeout_millis=3000)
-        # Shutdown does not kill the thread.
-        assert processor._batch_processor._worker_thread.is_alive() is True
-
-        after = time.time()
+        after = time.monotonic()
         assert after - before < 3.3
-        # Thread will naturally finish after a little bit.
-        time.sleep(0.1)
+
+        # The exporter shutdown interrupts the in-progress export. Depending
+        # on thread scheduling, the worker may stop before or shortly after
+        # shutdown() returns.
+        processor._batch_processor._worker_thread.join(timeout=1)
         assert processor._batch_processor._worker_thread.is_alive() is False
+
         # Expect the second call to be interrupted by shutdown, and the third call to never be made.
         assert exporter.sleep_interrupted is True
         assert 2 == exporter.num_export_calls

This removes the unstable intermediate-state assertion, waits deterministically for eventual termination, and uses time.monotonic() for elapsed-time measurement. No workflow change should be necessary.

Would you like to implement a fix?

No response

Tip

No response

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingtests

    Type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions