Skip to content

fix(tracing): make trace finish idempotent - #4500

Closed
sylvesterkaczmarek wants to merge 2 commits into
openai:mainfrom
sylvesterkaczmarek:fix/trace-finish-idempotent
Closed

fix(tracing): make trace finish idempotent#4500
sylvesterkaczmarek wants to merge 2 commits into
openai:mainfrom
sylvesterkaczmarek:fix/trace-finish-idempotent

Conversation

@sylvesterkaczmarek

Copy link
Copy Markdown
Contributor

Summary

  • make TraceImpl.finish() notify processors at most once
  • preserve the ability to reset a trace context on a later finish(reset_current=True) call
  • align trace lifecycle behaviour with the existing idempotent span finish path

Calling finish() more than once currently emits duplicate on_trace_end events, which can enqueue and export the same trace multiple times. The finished state is recorded after the processor callback succeeds, before any optional context reset.

Test plan

  • added focused regression coverage in tests/test_trace_finish_idempotent.py
  • GitHub Actions

Issue number

N/A

@sylvesterkaczmarek
sylvesterkaczmarek marked this pull request as ready for review August 17, 2026 21:35
@seratch

seratch commented Aug 17, 2026

Copy link
Copy Markdown
Member

I confirmed that calling TraceImpl.finish() twice currently invokes a custom processor's on_trace_end callback twice. However, the default BatchTraceProcessor queues traces on on_trace_start and does no work in on_trace_end, so the PR does not demonstrate the claimed duplicate export on the normal SDK path.

The documented lifecycle is either the context manager or one balanced manual start()/finish() pair. Please provide a concrete supported path or integration where the same trace is finished twice and the duplicate callback causes an observable consequence. Without that evidence, I do not think this additional lifecycle state is justified, so I would close this PR for now.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants