Skip to content

TraceQL Metrics: Vulture checks #4886

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

Merged

Conversation

ruslan-mikhailov
Copy link
Contributor

@ruslan-mikhailov ruslan-mikhailov commented Mar 21, 2025

What this PR does

it adds TraceQL Metrics check:

  1. For an existing trace, check thatthe overall count_over_time() is not zero
  2. Check the exact number by searching the trace

Which issue(s) this PR fixes

Fixes #

How it has been tested

Run tempo and vulture locally via docker-compose.
Results:
image

High amount of notfound_metrics due to bug: #4896

After the fix #4962, checked locally via docker-compose with Rhytm enabled and with one binary mode, Rhythm disabled. Additionally, run in k8s with Rhythm enabled

image

Checklist

  • Tests updated
  • Documentation added
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

@ruslan-mikhailov ruslan-mikhailov force-pushed the traceql-metrics-vulture-test branch 3 times, most recently from 758c196 to c20aeb7 Compare March 24, 2025 11:20
@ruslan-mikhailov ruslan-mikhailov force-pushed the traceql-metrics-vulture-test branch 2 times, most recently from 91db817 to ab712af Compare April 7, 2025 19:44
@ruslan-mikhailov ruslan-mikhailov marked this pull request as ready for review April 8, 2025 08:37
Copy link
Contributor

@javiermolinar javiermolinar left a comment

Choose a reason for hiding this comment

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

Great job!

}
timeSeries := resp.Series[0]
if timeSeries == nil {
tm.incorrectResult++
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we have incorrectResultMetrics?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Makes sense. Changed

@ruslan-mikhailov ruslan-mikhailov force-pushed the traceql-metrics-vulture-test branch from 7af4d0f to 369344b Compare April 8, 2025 11:16
@ruslan-mikhailov
Copy link
Contributor Author

+ review fix and minor changes in comments

Copy link
Contributor

@javiermolinar javiermolinar left a comment

Choose a reason for hiding this comment

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

LGTM!

@ruslan-mikhailov ruslan-mikhailov merged commit dab8106 into grafana:main Apr 8, 2025
15 checks passed
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