-
Notifications
You must be signed in to change notification settings - Fork 528
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: added min_over_time TraceQL metric #3975
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As discussed offline - we need to add a new aggregator for mode Sum/Final, which computes the min across time-series (i.e. the min of all mins), and use it in MetricsAggregate.initSum
and initFinal
. Right now it is still falling back to SimpleAdditionAggregator.
Back to draft, we need to come out with a solution for missing/uninitialized time-series values |
Is this something we'll want to document? |
Yes, once we agreed on a solution we should document this new function |
This may be a good spot for your content. The compare function content went on the traceql/metrics-queries.md file in the Functions section. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is looking great. I'm really happy with this PR because we are straightening up some ast and value handling quirks that needed it. There is 1 more required change to drop series with no values, and then 1 small nit that I would prefer but not blocking.
@knylander-grafana I've added some documentation. Maybe is a good idea to have a section for all the aggregation functions over_time. Like prometheus does |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work 👍
If you have a PR with doc updates, add the type/docs label to it please. |
@javiermolinar Thank you for adding the doc! We have a doc issue to update the metrics query docs (they need some love). Before we restructure sections in the page, we should look at the content on the metrics query pages to consider who our target audience is, what are the topics that audience needs to know, what are they interested in doing (determine any tasks -- which helps us create meaningful examples), etc. The Prometheus example is a good way of just listing an item and defining what it does. It could be a good way to do an introduction to a section, and then provide either an example for every function (in which case you use the subheadings like we have) or your subheadings are focused on examples and use cases for the functions. Does that difference make sense? I'd love to see the latter (function list like Prom docs and then real-world examples and explanations), but it takes more time than providing the function heading, with a brief explanation, and then a simple example. I've captured this conversation in the metrics query doc issue. |
What this PR does:
It adds a new TraceQL metric
min_over_time
used to compute the minimum value of a span series over a specified date range.Checklist
CHANGELOG.md
updated - the order of entries should be[CHANGE]
,[FEATURE]
,[ENHANCEMENT]
,[BUGFIX]