fix(sdk): add input sanitization for LangfuseObservationWrapper metadata - #1863
Open
hariom123-dev wants to merge 3 commits into
Open
fix(sdk): add input sanitization for LangfuseObservationWrapper metadata#1863hariom123-dev wants to merge 3 commits into
hariom123-dev wants to merge 3 commits into
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
TL;DR
Filters out
Noneand empty string ("") values from metadata dictionaries inLangfuseObservationWrapperbefore they get processed and sent to the API.Why this is needed
While working with the SDK, I noticed that when metadata is built dynamically (like from external API responses or scraper outputs), it's really easy to accidentally pass keys that contain
Noneor empty strings.Right now, those empty values just get serialized, sent over the network, and clutter up the Langfuse dashboard UI with blank fields.
What changed
I added a simple dictionary comprehension in
LangfuseObservationWrapper.__init__. It drops these null/empty keys right at the entry point, before the data hits_process_media_and_apply_maskor gets batched for the network.Testing
tests/unit/test_span_sanitization.pythat passes a messy dictionary to the wrapper and verifies the internal methods only receive the cleaned version.uv run pytest tests/unit/test_span_sanitization.pylocally and all checks are green.Greptile Summary
This PR adds constructor-time filtering for observation metadata and a focused unit test.
Confidence Score: 3/5
The PR should not merge until metadata values are filtered correctly and the same behavior is applied to observation updates.
Empty-string metadata values still survive construction, and metadata supplied through the public update path bypasses sanitization entirely; the added test also requires repository-standard formatting.
Files Needing Attention: langfuse/_client/span.py, tests/unit/test_span_sanitization.py
Prompt To Fix All With AI
Reviews (1): Last reviewed commit: "fix(sdk): add input sanitization for Lan..." | Re-trigger Greptile
Context used: