Skip to content

feat(ai): Add support for AI token span data #3250

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 8 commits into from
Mar 26, 2024
Merged

feat(ai): Add support for AI token span data #3250

merged 8 commits into from
Mar 26, 2024

Conversation

colin-sentry
Copy link
Member

@colin-sentry colin-sentry commented Mar 11, 2024

Upstream in SDKs (getsentry/sentry-python#2791) we added strings like "ai.prompt_tokens.used"

This adds them as known data fields, otherwise they are removed as PII since most configurations consider any superstring of "token" to be a password.

Copy link
Contributor

@olksdr olksdr left a comment

Choose a reason for hiding this comment

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

Also, please, add tests for expected behaviour.

Copy link
Contributor

@iker-barriocanal iker-barriocanal left a comment

Choose a reason for hiding this comment

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

As a side note, could you reword the PR description to align with Sentry's commit message guidelines? Thanks beforehand!

@olksdr
Copy link
Contributor

olksdr commented Mar 20, 2024

@colin-sentry are there any plans to follow up on this ?

@colin-sentry colin-sentry changed the title Do not consider the word "tokens" as a password feat(ai): Do not consider the word "tokens" as a password Mar 25, 2024
@colin-sentry colin-sentry force-pushed the tokens branch 2 times, most recently from 2bcb443 to 5be051d Compare March 25, 2024 17:07
@colin-sentry
Copy link
Member Author

@olksdr I'm having a bit of trouble writing a test for this - the test included in this PR still passes if you revert the regex change.

@phacops
Copy link
Contributor

phacops commented Mar 25, 2024

Do we need all the new fields on span data?

@colin-sentry colin-sentry changed the title feat(ai): Do not consider the word "tokens" as a password feat(ai): Add support for AI token metrics Mar 26, 2024
@colin-sentry colin-sentry changed the title feat(ai): Add support for AI token metrics feat(ai): Add support for AI token span data Mar 26, 2024
@colin-sentry
Copy link
Member Author

Do we need all the new fields on span data?

Discussed, by adding them to span data we can avoid them being removed as PII

@colin-sentry colin-sentry enabled auto-merge (squash) March 26, 2024 15:31
@colin-sentry colin-sentry merged commit f924c15 into master Mar 26, 2024
@colin-sentry colin-sentry deleted the tokens branch March 26, 2024 16:03
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants