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

feat(crewai): Add SpanKind and Token Count attributes #867

Merged
merged 27 commits into from
Aug 20, 2024
Merged

Conversation

harrisonchu
Copy link
Contributor

@harrisonchu harrisonchu commented Aug 13, 2024

resolves Arize-ai/phoenix#4200

fix(crewai): add basic unit tests

@harrisonchu harrisonchu requested a review from a team as a code owner August 13, 2024 14:31
@dosubot dosubot bot added the size:S This PR changes 10-29 lines, ignoring generated files. label Aug 13, 2024
@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. and removed size:S This PR changes 10-29 lines, ignoring generated files. labels Aug 14, 2024
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@harrisonchu harrisonchu changed the title feat crew AI token count, spankind, and output feat(crewai) Add token count, Fix Crew spanKind, add basic unit tests Aug 15, 2024
@harrisonchu harrisonchu changed the title feat(crewai) Add token count, Fix Crew spanKind, add basic unit tests feat(crewai): Add token count, Fix Crew spanKind, add basic unit tests Aug 15, 2024
@mikeldking mikeldking requested a review from fjcasti1 August 16, 2024 16:22
Copy link
Contributor

@fjcasti1 fjcasti1 left a comment

Choose a reason for hiding this comment

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

Use footers to trigger release please release notes notes instead of making the title a "multi-title". See https://github.com/googleapis/release-please?tab=readme-ov-file#what-if-my-pr-contains-multiple-fixes-or-features

PS: would love a training session on vrcpy ;)

Comment on lines 218 to 223
if "prompt_tokens" in usage_metrics:
span.set_attribute(LLM_TOKEN_COUNT_PROMPT, int(usage_metrics["prompt_tokens"]))
if "completion_tokens" in usage_metrics:
span.set_attribute(LLM_TOKEN_COUNT_COMPLETION, int(usage_metrics["completion_tokens"]))
if "total_tokens" in usage_metrics:
span.set_attribute(LLM_TOKEN_COUNT_TOTAL, int(usage_metrics["total_tokens"]))
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if "prompt_tokens" in usage_metrics:
span.set_attribute(LLM_TOKEN_COUNT_PROMPT, int(usage_metrics["prompt_tokens"]))
if "completion_tokens" in usage_metrics:
span.set_attribute(LLM_TOKEN_COUNT_COMPLETION, int(usage_metrics["completion_tokens"]))
if "total_tokens" in usage_metrics:
span.set_attribute(LLM_TOKEN_COUNT_TOTAL, int(usage_metrics["total_tokens"]))
if prompt_tokens := usage_metrics.get("prompt_tokens"):
span.set_attribute(LLM_TOKEN_COUNT_PROMPT, int(prompt_tokens))
if completion_tokens := usage_metrics.get("completion_tokens"):
span.set_attribute(LLM_TOKEN_COUNT_COMPLETION, int(completion_tokens))
if total_tokens := usage_metrics.get("total_tokens"):
span.set_attribute(LLM_TOKEN_COUNT_TOTAL, int(total_tokens))

Comment on lines +226 to +228
span.set_attribute(LLM_TOKEN_COUNT_PROMPT, usage_metrics.prompt_tokens)
span.set_attribute(LLM_TOKEN_COUNT_COMPLETION, usage_metrics.completion_tokens)
span.set_attribute(LLM_TOKEN_COUNT_TOTAL, usage_metrics.total_tokens)
Copy link
Contributor

Choose a reason for hiding this comment

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

Are we absolutely certain these are set?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yea in later versions it's set right before the end of the kickoff main function

Copy link
Contributor

Choose a reason for hiding this comment

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

Should this litellm notebook be in this PR?

Copy link
Contributor

@fjcasti1 fjcasti1 left a comment

Choose a reason for hiding this comment

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

Reminder about the PR title (comment above)

@harrisonchu harrisonchu changed the title feat(crewai): Add token count, Fix Crew spanKind, add basic unit tests feat(crewai): Add SpanKind and Token Count attributes Aug 20, 2024
@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Aug 20, 2024
@harrisonchu harrisonchu merged commit a61e12a into main Aug 20, 2024
5 checks passed
@harrisonchu harrisonchu deleted the crewai-fixes branch August 20, 2024 20:51
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 size:L This PR changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] CrewAI Instrumentation Bugs
2 participants