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(ai): Extract AI-related metrics from span data #3412

Merged
merged 10 commits into from
Apr 16, 2024
Merged

Conversation

colin-sentry
Copy link
Member

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

This pull request adds support for ai.total_tokens.used metrics to be pulled from submitted spans

@colin-sentry colin-sentry requested a review from a team as a code owner April 11, 2024 15:46
@colin-sentry colin-sentry requested a review from phacops April 11, 2024 15:46
relay-dynamic-config/src/defaults.rs Show resolved Hide resolved
relay-dynamic-config/src/defaults.rs Outdated Show resolved Hide resolved
relay-dynamic-config/src/defaults.rs Show resolved Hide resolved
@colin-sentry colin-sentry changed the title Metrics for AI feat(ai): Extract AI-related metrics from span data Apr 11, 2024
Copy link
Contributor

@phacops phacops left a comment

Choose a reason for hiding this comment

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

description is supposed to be the name of the pipeline, so low cardinality as it's not expected to have thousands of pipelines.

@phacops phacops requested a review from jjbayer April 11, 2024 19:14
@colin-sentry colin-sentry enabled auto-merge (squash) April 11, 2024 19:54
CHANGELOG.md Outdated Show resolved Hide resolved
relay-event-schema/src/protocol/span.rs Outdated Show resolved Hide resolved
relay-event-schema/src/protocol/span.rs Outdated Show resolved Hide resolved
@colin-sentry colin-sentry force-pushed the ai_metrics branch 7 times, most recently from d5a9297 to 6a73ddd Compare April 15, 2024 16:03
@@ -39,6 +39,7 @@
- Scrub transactions before enforcing quotas. ([#3248](https://github.com/getsentry/relay/pull/3248))
- Implement metric name based cardinality limits. ([#3313](https://github.com/getsentry/relay/pull/3313))
- Kafka topic config supports default topic names as keys. ([#3282](https://github.com/getsentry/relay/pull/3282), [#3350](https://github.com/getsentry/relay/pull/3350))
- Extract AI metrics from spans. ([#3412](https://github.com/getsentry/relay/pull/3412))
Copy link
Member

Choose a reason for hiding this comment

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

Needs to be fixed, after the 24.4.0 release

Comment on lines +141 to +143
// `exclusive_time_light` excludes transaction tags (and some others) to reduce cardinality.
let exclusive_time_light_condition = (is_db.clone()
| is_ai.clone()
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 still interested in extracting this metric for AI spans?

Copy link
Member

Choose a reason for hiding this comment

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

+1, could we document the answer to this question here, even if it was discussed offline? Would be interesting to know.

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

The intention is to build a view similar to the index page of the various performance modules like database where it surfaces these ai metrics across the project.

@colin-sentry colin-sentry merged commit 0f2a0e9 into master Apr 16, 2024
20 checks passed
@colin-sentry colin-sentry deleted the ai_metrics branch April 16, 2024 12:59
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.

6 participants