-
Notifications
You must be signed in to change notification settings - Fork 447
chore(llmobs): fix flaky writer test #13920
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On second thought - should we consider changing this test to just assert the class/type of LLMObs span writer? Will that be more reliable than asserting error/stdout logs?
Bootstrap import analysisComparison of import times between this PR and base. SummaryThe average import time from this PR is: 278 ± 3 ms. The average import time from base is: 280 ± 3 ms. The import time difference between this PR and base is: -1.8 ± 0.1 ms. Import time breakdownThe following import paths have shrunk:
|
hmm yea @Yun-Kim i also thought of just directly asserting that the endpoint the writer has is what we expect it to be, but looking at the original pr for this test being introduced it relates to "duplicate spans" being enqueued to potentially different writers? #11889 idk if simply asserting on the endpoint for |
This test has been flaky and relies on the subprocess outputting certain error logs that verify spans are being enqueued to the correct writer
Try to make it less flaky by
Checklist
Reviewer Checklist