Skip to content

Close tracing exporter during BatchTraceProcessor.shutdown()#3470

Closed
mshsheikh wants to merge 1 commit into
openai:mainfrom
mshsheikh:patch-38
Closed

Close tracing exporter during BatchTraceProcessor.shutdown()#3470
mshsheikh wants to merge 1 commit into
openai:mainfrom
mshsheikh:patch-38

Conversation

@mshsheikh

Copy link
Copy Markdown
Contributor

Summary

This PR ensures that BatchTraceProcessor.shutdown() closes the tracing exporter after the final batch has been flushed.

Problem

BackendSpanExporter creates a persistent httpx.Client for trace ingestion. Before this change, shutdown drained the queue but did not explicitly close the exporter, which left the HTTP client and connection pool open.

That can leave resources allocated longer than necessary, especially in:

  • long-running services
  • tests that repeatedly create and tear down tracing components
  • worker processes or environments with frequent startup and shutdown cycles

Change

After the final synchronous flush, the shutdown path now looks up close() on the exporter and calls it when available:

close_fn = getattr(self._exporter, "close", None)
if callable(close_fn):
    try:
        close_fn()
    except Exception as exc:
        logger.warning(
            "[non-fatal] Tracing: exporter close failed: %s",
            exc,
        )

Behavior

  • preserves the existing final flush behavior
  • closes exporter resources after export completes
  • keeps cleanup non-fatal
  • remains compatible with custom exporters that do not implement close()

Notes

This change assumes the exporter is safe to close at processor shutdown time. If exporter ownership is later shared more broadly, that lifecycle may need to be made explicit.

### Summary 

This PR ensures that `BatchTraceProcessor.shutdown()` closes the tracing exporter after the final batch has been flushed.

### Problem

`BackendSpanExporter` creates a persistent `httpx.Client` for trace ingestion. Before this change, shutdown drained the queue but did not explicitly close the exporter, which left the HTTP client and connection pool open.

That can leave resources allocated longer than necessary, especially in:

* long-running services
* tests that repeatedly create and tear down tracing components
* worker processes or environments with frequent startup and shutdown cycles

### Change

After the final synchronous flush, the shutdown path now looks up `close()` on the exporter and calls it when available:

```python
close_fn = getattr(self._exporter, "close", None)
if callable(close_fn):
    try:
        close_fn()
    except Exception as exc:
        logger.warning(
            "[non-fatal] Tracing: exporter close failed: %s",
            exc,
        )
```

### Behavior

* preserves the existing final flush behavior
* closes exporter resources after export completes
* keeps cleanup non-fatal
* remains compatible with custom exporters that do not implement `close()`

### Notes

This change assumes the exporter is safe to close at processor shutdown time. If exporter ownership is later shared more broadly, that lifecycle may need to be made explicit.
@mshsheikh

Copy link
Copy Markdown
Contributor Author

Hi @seratch , just wanted to give this a friendly ping since it's been open for a few weeks.

All checks are passing, and this should safely clean up the HTTP client resources during BatchTraceProcessor.shutdown(). Let me know if you have any questions, want any changes, or if we are waiting on a broader tracing update before merging this!

@seratch

seratch commented Jul 16, 2026

Copy link
Copy Markdown
Member

Thanks for the contribution. I confirmed that BackendSpanExporter owns a persistent HTTP client, but this PR does not demonstrate an observable resource leak in a supported lifecycle. Explicitly constructed exporters can already be closed by their caller, while the default shutdown normally runs at process exit.

More importantly, BatchTraceProcessor.shutdown(timeout=...) may return while its worker is still exporting, after which this patch immediately closes that worker's exporter. TracingExporter also has no close or ownership contract, so this can close caller-owned or shared custom exporters unexpectedly.

I am going to close this PR. If a minimal repeated in-process reproduction shows measurable resource growth or warnings, we can revisit a narrower fix for the SDK-owned default exporter after its worker has stopped.

@seratch seratch closed this Jul 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants