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.

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

parse litellm attributes (#171) #172

Merged
merged 1 commit into from
Nov 7, 2024
Merged

parse litellm attributes (#171) #172

merged 1 commit into from
Nov 7, 2024

Conversation

dinmukhamedm
Copy link
Member

@dinmukhamedm dinmukhamedm commented Nov 7, 2024

Important

Enhance Span class to parse and handle new LiteLLM attributes for input and output in spans.rs.

  • Behavior:
    • Updated Span class in spans.rs to parse new LiteLLM attributes SpanAttributes.LLM_PROMPTS and SpanAttributes.LLM_COMPLETIONS.
    • Modified input_chat_messages_from_prompt_content() and output_from_completion_content() to handle new attribute prefixes.
    • Updated should_keep_attribute() to exclude LiteLLM attributes from being stored redundantly.
  • Functions:
    • Added tool_call_attribute() to generate attribute keys for tool calls.
    • Updated output_from_completion_content() to handle tool call attributes with or without index.
  • Misc:
    • Fixed typo in comment in spans.rs.

This description was created by Ellipsis for 4d567e1. It will automatically update as commits are pushed.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me! Reviewed everything up to 4d567e1 in 54 seconds

More details
  • Looked at 190 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 2 drafted comments based on config settings.
1. app-server/src/traces/spans.rs:545
  • Draft comment:
    Consider compiling the regex pattern once and reusing it to improve performance.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The regex pattern is compiled every time the function is called, which is inefficient. It should be compiled once and reused.
2. app-server/src/traces/spans.rs:553
  • Draft comment:
    Consider compiling the regex pattern once and reusing it to improve performance. This comment also applies to the regex pattern on line 545.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The regex pattern is compiled every time the function is called, which is inefficient. It should be compiled once and reused.

Workflow ID: wflow_c4SLXAGtrbGnsZ8c


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

@dinmukhamedm dinmukhamedm merged commit 7ae1f99 into main Nov 7, 2024
1 check passed
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.

1 participant