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(metric-stats): Add additional tags to metric stats metrics #3438

Merged
merged 1 commit into from
Apr 17, 2024

Conversation

Dav1dde
Copy link
Member

@Dav1dde Dav1dde commented Apr 16, 2024

Reports additional tags with the metric stats metrics.

#skip-changelog

@Dav1dde Dav1dde requested a review from a team as a code owner April 16, 2024 10:59
@Dav1dde Dav1dde self-assigned this Apr 16, 2024
Comment on lines -166 to -168
if !namespace.has_metric_stats() {
return None;
}
Copy link
Member

Choose a reason for hiding this comment

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

Isn't this important?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes and no, we can now track cardinality limits which are not associated with a metric (Type). The on/off is now only controlled through report: True on the cardinality limit, which I think does make sense.

I'll kill the has_metric_stats in a follow-up, Jan didn't seem to like it anyways, currently it's still needed for the volume metric 👍 .

Base automatically changed from dav1d/card-by-type to master April 16, 2024 12:14
@Dav1dde Dav1dde force-pushed the dav1d/card-report-id-scope branch from 9ce478b to 044595b Compare April 16, 2024 12:15
@Dav1dde Dav1dde force-pushed the dav1d/card-report-id-scope branch from 044595b to 6134468 Compare April 16, 2024 12:19
@Dav1dde Dav1dde mentioned this pull request Apr 16, 2024
@Dav1dde Dav1dde merged commit f43d2b4 into master Apr 17, 2024
20 checks passed
@Dav1dde Dav1dde deleted the dav1d/card-report-id-scope branch April 17, 2024 06:22
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.

2 participants