-
Notifications
You must be signed in to change notification settings - Fork 200
add/fix(langfuse): log levels #2522
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
add/fix(langfuse): log levels #2522
Conversation
|
thanks @Hansehart - I'll take a look @sjrl |
vblagoje
left a comment
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.
Looks mostly good but it seems we have some minor DRY violations where we can have one finally block (for try/else) with flush and token reset. wdyt?
|
@vblagoje You're absolutely right! I have to admit its hard to read. But now its a bit better. |
vblagoje
left a comment
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.
LG

Related Issues
Proposed Changes:
Replaced the try/finally pattern with a try/except/else pattern that properly captures and propagates exception information.
The Langfuse tracer was always calling exit(None, None, None) in a finally block, regardless of whether an exception occurred during pipeline execution. This told Langfuse/OpenTelemetry that all operations completed successfully, even when exceptions were raised.
How did you test it?
unit tests, manual verification
Notes for the reviewer
Checklist
fix:,feat:,build:,chore:,ci:,docs:,style:,refactor:,perf:,test:.