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

fix(groq): Updated the instrumentation to collect the token histogram #2685

Merged
merged 3 commits into from
Feb 25, 2025

Conversation

adharshctr
Copy link
Contributor

@adharshctr adharshctr commented Feb 24, 2025

image

  • I have added tests that cover my changes.
  • If adding a new instrumentation or changing an existing one, I've added screenshots from some observability platform showing the change.
  • PR name follows conventional commits format: feat(instrumentation): ... or fix(instrumentation): ....
  • (If applicable) I have updated the documentation accordingly.

Important

Enhances Groq instrumentation by recording token histograms for prompt and completion tokens in __init__.py.

  • Behavior:
    • Updates _set_response_attributes in __init__.py to record token histogram for prompt_tokens and completion_tokens.
    • Records tokens only if they are non-negative integers.
  • Instrumentation:
    • Passes token_histogram to _set_response_attributes in _wrap and _awrap functions.
  • Misc:
    • No changes to existing tests or documentation.

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

@adharshctr adharshctr marked this pull request as ready for review February 24, 2025 11:32
@dosubot dosubot bot added size:S This PR changes 10-29 lines, ignoring generated files. python Pull requests that update Python code labels Feb 24, 2025
Copy link

@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.

❌ Changes requested. Reviewed everything up to bb8ce0c in 1 minute and 45 seconds

More details
  • Looked at 66 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 2 drafted comments based on config settings.
1. packages/opentelemetry-instrumentation-groq/opentelemetry/instrumentation/groq/__init__.py:192
  • Draft comment:
    Usage is accessed immediately without checking if it is None. Guard against None before calling usage.get().
  • Reason this comment was not posted:
    Comment looked like it was already resolved.
2. packages/opentelemetry-instrumentation-groq/opentelemetry/instrumentation/groq/__init__.py:206
  • Draft comment:
    Safe-check needed: token_histogram.record is called without verifying if token_histogram is not None. If metrics are disabled (and token_histogram is None), this will raise an exception. Consider adding a guard before calling record.
  • Reason this comment was not posted:
    Marked as duplicate.

Workflow ID: wflow_B5g1FZrqkqtDXzen


Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

@adharshctr adharshctr changed the title fix(groq): Updated the instrumentation to collect the token histogram feat(groq): Updated the instrumentation to collect the token histogram Feb 24, 2025
Copy link
Member

@nirga nirga left a comment

Choose a reason for hiding this comment

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

Thanks @adharshctr! Can you address the comments from Elipsis - I think they can actually improve the reliability here :)

@adharshctr adharshctr requested a review from nirga February 25, 2025 10:11
@adharshctr
Copy link
Contributor Author

Thanks @adharshctr! Can you address the comments from Elipsis - I think they can actually improve the reliability here :)

Done

Copy link
Member

@nirga nirga left a comment

Choose a reason for hiding this comment

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

Thanks!

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Feb 25, 2025
@nirga nirga changed the title feat(groq): Updated the instrumentation to collect the token histogram fix(groq): Updated the instrumentation to collect the token histogram Feb 25, 2025
@nirga nirga merged commit 394a340 into traceloop:main Feb 25, 2025
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm This PR has been approved by a maintainer python Pull requests that update Python code size:S This PR changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants