[harshkoli1255]

Description

Add focused unit test coverage for the existing JSON file exporter's stream lifecycle and concurrent export behavior.

The tests verify:

• Caller-provided streams remain open after shutdown.
• Default stdout remains open after shutdown.
• Exporter-owned file streams close after shutdown.
• Closed streams are handled gracefully with a warning.
• Concurrent exports produce complete, valid JSON lines without overlapping writes.
• The concurrency test uses a custom stream to detect overlapping writes.

Testing

• JSON file exporter test suite: 50 passed.
• Ruff lint: passed.
• Ruff formatting: passed.
• No production code, public APIs, or dependencies were modified.

Changelog

Skip Changelog

[opentelemetry-pr-dashboard]

Pull request dashboard status

Waiting on maintainers · refreshed 2026-11-08 20:22 UTC

Merge when ready.

Status above doesn't look right?

• Just replied or pushed? Anything around or after the refresh time above may not be picked up yet — give it a few minutes.
• Anything look wrong? Report it with what you expected; it helps us improve the dashboard.

[harshkoli1255]

Hi maintainers,

The CI workflows for this PR are currently awaiting approval to run.

Could someone approve them when convenient? The local tests and Ruff checks are passing.

Thanks!

[harshkoli1255]

Hi maintainers,

I've pushed fixes for the Pyright and Pylint CI failures on PR #5815. All 50 tests, Pyright, Pylint, and Ruff checks now pass locally.

The updated GitHub Actions workflows are awaiting approval. Could someone approve the runs and apply the Skip Changelog label, since this is a test-only PR?

PR: #5815

Thanks!

[chrikrah]

@harshkoli1255 The new tests hold under mutation at ed9e845: each one fails when I break the behaviour it names in _internal.py.

$ pytest exporter/opentelemetry-exporter-otlp-json-file/tests -q -p no:cacheprovider   # Python 3.12
50 passed in 0.21s                                    # ed9e845
45 passed in 0.30s                                    # test_internal.py from bba3bc3

# mutations of _internal.py at ed9e845, then pytest .../tests/test_internal.py
no `with self._lock:` in export()             1 failed, 8 passed   (concurrent_exports: "Concurrent writes were detected")
no `self._stream.closed` check                1 failed, 8 passed
shutdown() never closes the stream            1 failed, 8 passed
shutdown() closes any stream                  2 failed, 5 passed, 5 errors
same, with the stdout test under mock.patch   2 failed, 7 passed

non-blocking: in the "closes any stream" row, test_default_stdout_remains_open_after_shutdown closes pytest's own sys.stdout, and the 5 errors are the tests after it. Under mock.patch("sys.stdout", new_callable=io.StringIO) the same mutation fails only the two tests that name it. Would you fold that in?

[harshkoli1255]

@harshkoli1255 The new tests hold under mutation at ed9e845: each one fails when I break the behaviour it names in _internal.py.

$ pytest exporter/opentelemetry-exporter-otlp-json-file/tests -q -p no:cacheprovider   # Python 3.12
50 passed in 0.21s                                    # ed9e845
45 passed in 0.30s                                    # test_internal.py from bba3bc3

# mutations of _internal.py at ed9e845, then pytest .../tests/test_internal.py
no `with self._lock:` in export()             1 failed, 8 passed   (concurrent_exports: "Concurrent writes were detected")
no `self._stream.closed` check                1 failed, 8 passed
shutdown() never closes the stream            1 failed, 8 passed
shutdown() closes any stream                  2 failed, 5 passed, 5 errors
same, with the stdout test under mock.patch   2 failed, 7 passed

non-blocking: in the "closes any stream" row, test_default_stdout_remains_open_after_shutdown closes pytest's own sys.stdout, and the 5 errors are the tests after it. Under mock.patch("sys.stdout", new_callable=io.StringIO) the same mutation fails only the two tests that name it. Would you fold that in?

Thanks @chrikrah! I've included the suggested mock.patch change in commit 673b23fc. All tests and lint checks pass locally.

[chrikrah]

@harshkoli1255 with 673b23f the "shutdown closes any stream" mutation now fails only the two tests that name it, with no errors spilling into the rest of the file. Still approving.

$ pytest exporter/opentelemetry-exporter-otlp-json-file/tests -q -p no:cacheprovider   # export of 673b23f, _internal.py imported from it, Python 3.12.3
50 passed in 1.41s
# _internal.py at 673b23f with `if self._owns_stream:` replaced by `if True:` in shutdown()
$ pytest .../tests/test_internal.py -q -p no:cacheprovider
2 failed, 7 passed in 0.77s     # was 2 failed, 5 passed, 5 errors at ed9e845
FAILED .../test_internal.py::TestFileExporter::test_caller_stream_remains_open_after_shutdown
FAILED .../test_internal.py::TestFileExporter::test_default_stdout_remains_open_after_shutdown

[harshkoli1255]

$ pytest exporter/opentelemetry-exporter-otlp-json-file/tests -q -p no:cacheprovider # export of 673b23f, _internal.py imported from it, Python 3.12.3
50 passed in 1.41s

_internal.py at 673b23f with if self._owns_stream: replaced by if True: in shutdown()

$ pytest .../tests/test_internal.py -q -p no:cacheprovider
2 failed, 7 passed in 0.77s # was 2 failed, 5 passed, 5 errors at ed9e845
FAILED .../test_internal.py::TestFileExporter::test_caller_stream_remains

Thanks @chrikrah for reviewing again and verifying the updated tests! Really appreciate the detailed feedback.

[harshkoli1255]

Hi maintainers,

Thanks for reviewing and approving PR #5815!

The PR is still blocked by the changelog and public-symbols checks. I'm investigating the public-symbols failure.

Since this is a test-only change, could someone please apply the Skip Changelog label?

Thanks!

[harshkoli1255]

Hi maintainers! 👋

The PR is still blocked by the changelog and public-symbols CI checks. Since it's a test-only change, could someone please apply the Skip Changelog label and help resolve the remaining CI blocker?

Once the checks are green, I'd appreciate a merge when convenient.

Thanks!

[harshkoli1255]

Thanks for applying the Skip Changelog label!

PR #5815 now has two approvals. I noticed that 4 workflows are still awaiting approval. Could someone please approve and run them when convenient?

Thanks for your help!

[harshkoli1255]

Hi maintainers!
PR #5815 has received two approvals, and the Skip Changelog label has been applied. The changes are approved, and the PR is ready to merge.

Could someone please merge it when convenient?

PR: #5815

[Copilot]

🟢 Approval recommended

There are no active review issues for the added stream-lifecycle and concurrent-export tests.

No open issues 🎉

What changed in this PR

Adds focused unit tests for the OTLP JSON file exporter’s stream lifecycle and concurrent exports, without changing production code, public APIs, or dependencies.

Changes:

• Verify shutdown preserves caller-provided streams and stdout while closing exporter-owned files.
• Check that exporting to a closed stream returns failure and logs a warning.
• Use a custom stream to detect overlapping writes and verify concurrent exports produce complete JSON lines with all expected records.

File	Description
exporter/​opentelemetry-exporter-otlp-json-file/​tests/​test_internal.py	Adds stream ownership, closed-stream handling, and concurrent export tests.

_{Review effort: Balanced}