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

Create token_count attribute #2489

Merged
merged 16 commits into from
Mar 14, 2024

Conversation

kaylareopelle
Copy link
Contributor

@kaylareopelle kaylareopelle commented Mar 6, 2024

  • Adds token_count to ChatCompletionMessage and Embedding
  • Calculates token_count using the customer-provided callback set by set_llm_token_count_callback and assigns it in OpenAI instrumentation
  • If the callback is not available, or the result is not an Integer greater than zero or nil, don't send the token_count attribute.

Closes #2450

@@ -101,8 +102,7 @@ def create_chat_completion_messages(parameters, summary_id)
role: message[:role] || message['role'],
sequence: index,
completion_id: summary_id,
vendor: VENDOR,
is_response: true
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is an unrelated fix. This first group of messages are not from a response. They're from a request. The is_response should only be attached to the event if it has a truthy value.

Copy link
Contributor

Choose a reason for hiding this comment

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

nice

end
end

def calculate_token_count(model, content)
# return unless NewRelic::Agent.config['ai_monitoring.record_content.enabled']
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This'll get uncommented once the config PR is merged

@kaylareopelle kaylareopelle marked this pull request as ready for review March 11, 2024 16:36
Copy link
Contributor

@hannahramadan hannahramadan 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 after the completion_id stuff is added back ◡̈

Copy link
Contributor

SimpleCov Report

Coverage Threshold
Line 93.71% 93%
Branch 71.21% 71%

@kaylareopelle kaylareopelle merged commit bd7af5b into openai_instrumentation Mar 14, 2024
28 checks passed
@kaylareopelle kaylareopelle deleted the create_token_count_attribute branch October 9, 2024 19:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants