-
Notifications
You must be signed in to change notification settings - Fork 228
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: Support for sum metrics in charts #622
base: warren/metrics-mvp
Are you sure you want to change the base?
Conversation
|
1754ad5
to
fc37b55
Compare
I didn't add a changeset file here. I'm thinking that we would just have a single changeset on the feature branch level. |
}); | ||
|
||
// it('should generate sql for multiple gauge metrics', async () => { |
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.
supporting multiple metrics in a single query is another ticket.
Add query rendering support for CTEs and then using a CTE to support HyperDX v1 query logic to get sum metrics working in v2. Ref: HDX-1337
fc37b55
to
d609fe7
Compare
The condidion field on the select object is used by the UI filters. To avoid mixing the metric name filter with the UI filter, the name condition has been moved to the `where` condition.
@@ -790,7 +790,7 @@ function translateMetricChartConfig( | |||
any(Attributes) OVER (ROWS BETWEEN 1 PRECEDING AND 1 PRECEDING) AS PrevAttributes, | |||
IF(AggregationTemporality = 1, | |||
Value,IF(Value - PrevValue < 0 AND Attributes = PrevAttributes,Value, | |||
IF(Attributes != PrevAttributes, 0, Value - PrevValue))) as Rate | |||
IF(cityHash64(Attributes) != cityHash64(PrevAttributes), 0, Value - PrevValue))) as Rate |
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.
I think we want to actually run this on the subselect first, otherwise we'll be rehashing a lot more than we need to.
dac30e4
to
915a77c
Compare
Add query rendering support for CTEs and then using a CTE to support HyperDX v1 query logic to get sum metrics working in v2.
Ref: HDX-1337