Skip to content

Better trace ids#595

Merged
bvdmitri merged 3 commits intorelease-6from
better-trace-ids
Apr 7, 2026
Merged

Better trace ids#595
bvdmitri merged 3 commits intorelease-6from
better-trace-ids

Conversation

@bvdmitri
Copy link
Copy Markdown
Member

@bvdmitri bvdmitri commented Apr 2, 2026

This PR changes the generation of trace ids (also renamed to span ids) in order to remove massive performance hit for certain models. In my benchmark, the generation of trace IDs affected benchmarks by a factor of 7! And this was even without enabling the callbacks. The current PR changes the implementation a bit by calling an auxiliary function ReactiveMP.generate_span_id that would simply return nothing if callbacks are also nothing. That fixes the performance hit in case when callbacks are not set, but otherwise it still uses uuid4 (definitely the culprit). We may want to use a simple global Int counter for that? Not sure if it would be faster though since this counter would need to be guarded with semaphores to protect from two threads generating the same span ID.

@fonsp wdyt?

@bvdmitri bvdmitri requested a review from fonsp April 2, 2026 12:53
@bvdmitri
Copy link
Copy Markdown
Member Author

bvdmitri commented Apr 2, 2026

The affected benchmark is this one just in case
Screenshot 2026-04-02 at 14 55 11

@codecov
Copy link
Copy Markdown

codecov bot commented Apr 2, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (release-6@d8459de). Learn more about missing BASE report.

Additional details and impacted files
@@             Coverage Diff              @@
##             release-6     #595   +/-   ##
============================================
  Coverage             ?   80.76%           
============================================
  Files                ?      212           
  Lines                ?     6466           
  Branches             ?        0           
============================================
  Hits                 ?     5222           
  Misses               ?     1244           
  Partials             ?        0           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Copy Markdown
Contributor

@fonsp fonsp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Beautiful!

@bvdmitri
Copy link
Copy Markdown
Member Author

bvdmitri commented Apr 3, 2026

@fonsp could you adjust trace_id -> span_id in RxInfer accordingly? I just noticed we do there as well, basically just need to change the fields and use the ReactiveMP.generate_span_id there too

@bvdmitri
Copy link
Copy Markdown
Member Author

bvdmitri commented Apr 7, 2026

@bvdmitri bvdmitri merged commit b8397a7 into release-6 Apr 7, 2026
7 of 8 checks passed
@bvdmitri bvdmitri deleted the better-trace-ids branch April 7, 2026 12:54
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