Skip to content
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.

Sign up for GitHub

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

Fix: Problem with langfuse_tags when using litellm proxy with langfus… #7825

Merged
merged 3 commits into from
Jan 18, 2025
Merged

Fix: Problem with langfuse_tags when using litellm proxy with langfus… #7825

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
47e4a45
Fix: Problem with langfuse_tags when using litellm proxy with langfus…
yuu341 Jan 17, 2025
0e1fc2e
Refactor: Create method get_langfuse_tags
yuu341 Jan 17, 2025
d1dea53
Fix: Exception Handling
yuu341 Jan 18, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion litellm/integrations/langfuse/langfuse.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#### What this does ####
# On success, logs events to Langfuse
import copy
import json
import os
import traceback
from collections.abc import MutableMapping, MutableSequence, MutableSet
Expand Down Expand Up @@ -457,7 +458,7 @@ def _log_langfuse_v2( # noqa: PLR0915
supports_costs = langfuse_version >= Version("2.7.3")
supports_completion_start_time = langfuse_version >= Version("2.7.3")

tags = metadata.pop("tags", []) if supports_tags else []
tags = self._get_langfuse_tags(metadata) if supports_tags else []

standard_logging_object: Optional[StandardLoggingPayload] = cast(
Optional[StandardLoggingPayload],
Expand Down Expand Up @@ -732,6 +733,14 @@ def _log_langfuse_v2( # noqa: PLR0915
verbose_logger.error(f"Langfuse Layer Error - {traceback.format_exc()}")
return None, None

@staticmethod
def _get_langfuse_tags(metadata: dict) -> list[str]:
try:
return json.loads(metadata.pop("tags", "[]"))
except Exception as e:
verbose_logger.exception("error getting langfuse tags %s", str(e))
return []

Copy link
Contributor

Choose a reason for hiding this comment

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

can you try / except this, (don't raise an exception if there is an error)

you can do `verbose_logger.exception("error getting langfuse tags %s", str(e))

Copy link
Contributor

Choose a reason for hiding this comment

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

concern here is json decode / encode errors

Copy link
Contributor Author

@yuu341 yuu341 Jan 18, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Thank you for your feedback.
If the tags cannot be converted to JSON successfully, should I convert an empty array? Or, because the severity is "error", should I avoid sending the data to Langfuse and return a non-200 status code to the client?
The previous behavior was to forcibly append to a string, so the request was treated as not being sent to Langfuse in the first place.

current code.

    @staticmethod
    def _get_langfuse_tags(metadata: dict) -> list[str]:
        try:
            return json.loads(metadata.pop("tags", "[]"))
        except Exception as e:
            verbose_logger.exception("error getting langfuse tags %s", str(e))
            return []

current logs

09:47:02 - LiteLLM:ERROR: langfuse.py:740 - error getting langfuse tags Expecting property name enclosed in double quotes: line 1 column 2 (char 1)
Traceback (most recent call last):
  File "/home/ubuntu/work/xxx/venv/lib/python3.12/site-packages/litellm/integrations/langfuse/langfuse.py", line 738, in _get_langfuse_tags
    return json.loads(metadata.pop("tags", "[]"))
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/json/__init__.py", line 346, in loads
    return _default_decoder.decode(s)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/json/decoder.py", line 337, in decode
    obj, end = self.raw_decode(s, idx=_w(s, 0).end())
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/json/decoder.py", line 353, in raw_decode
    obj, end = self.scan_once(s, idx)
               ^^^^^^^^^^^^^^^^^^^^^^

Copy link
Contributor

Choose a reason for hiding this comment

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

returning an array is fine

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have pushed the implementation above as is. Please review it.

def add_default_langfuse_tags(self, tags, kwargs, metadata):
"""
Helper function to add litellm default langfuse tags
Expand Down