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): exception when metrics are turned off #2778

Merged
merged 1 commit into from
Mar 13, 2025
Merged

fix(groq): exception when metrics are turned off #2778

merged 1 commit into from
Mar 13, 2025

Conversation

nirga
Copy link
Member

@nirga nirga commented Mar 13, 2025

Fix #2776

  • 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

Fix exception in Groq instrumentation when metrics are turned off by removing unnecessary None assignment in _instrument().

  • Behavior:
    • Fix exception in _instrument() in __init__.py when metrics are turned off by removing unnecessary None assignment.
  • Misc:
    • Minor formatting changes in __init__.py for better readability.

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

Sorry, something went wrong.

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 941dd66 in 40 seconds

More details
  • Looked at 64 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 4 drafted comments based on config settings.
1. packages/opentelemetry-instrumentation-groq/opentelemetry/instrumentation/groq/__init__.py:589
  • Draft comment:
    Tuple length mismatch fixed: _create_metrics returns 3 values, so using (None, None, None) is correct when metrics are off.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50%
    This comment is purely informative, explaining a change that was made. It doesn't suggest any action or ask for confirmation. It doesn't fit the criteria for a useful comment as per the rules.
2. packages/opentelemetry-instrumentation-groq/opentelemetry/instrumentation/groq/__init__.py:199
  • Draft comment:
    Minor formatting adjustments in _set_response_attributes—ensures consistency.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50%
    None
3. packages/opentelemetry-instrumentation-groq/opentelemetry/instrumentation/groq/__init__.py:196
  • Draft comment:
    Good cleanup: using multiline conditions and formatting in _set_response_attributes improves readability. The consistent checking of token_histogram is clear.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50%
    None
4. packages/opentelemetry-instrumentation-groq/opentelemetry/instrumentation/groq/__init__.py:575
  • Draft comment:
    Tuple unpacking now returns three values instead of four, which aligns with _create_metrics. Verify that the unused 'choice_counter' is intentionally omitted downstream.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.

Workflow ID: wflow_Lpe82oHNqsfMU3X9


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

@nirga nirga merged commit f9a379b into main Mar 13, 2025
9 checks passed
@nirga nirga deleted the groq-fix branch March 13, 2025 15:40
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.

🐛 Bug Report: GroqInstrumentor produces error when metrics turned off
2 participants