-
Notifications
You must be signed in to change notification settings - Fork 307
feat: Add delta() function for gauge metrics #1147
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
base: main
Are you sure you want to change the base?
Conversation
🦋 Changeset detectedLatest commit: 56ed67c The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
The latest updates on your projects. Learn more about Vercel for GitHub.
|
<CheckBoxControlled | ||
control={control} | ||
name={`${namePrefix}isDelta`} | ||
label="Delta" | ||
size="xs" | ||
className="mt-2" | ||
/> |
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.
One thought is that eventually this could be a drop-down with various other functions that could be applied, if we implement other functions. I kept it simple for now though.
Stably Runner - Test Suite - 'Smoke Test'Test Suite Run Result: 🔴 Failure (1/4 tests failed) [dashboard] Failed Tests: This comment was generated from stably-runner-action |
8679ead
to
e8a2892
Compare
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.
Had one question about the timestamp division but looks solid. You'll also want to write a changeset description for the release notes. You can use yarn changeset
if you want a TUI interface to writing the changeset.
e8a2892
to
29ac4c2
Compare
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.
Shoot. I forgot to mention that we have a series of integration tests for these queries in https://github.com/hyperdxio/hyperdx/blob/main/packages/api/src/clickhouse/__tests__/renderChartConfig.test.ts.
These tests run the query against a CH instance and real data. It would be great to have a test case or two about the delta function here. This is one of the few places that are well suited to these kind of automated tests because it's a pure input->output function.
Summary
Closes HDX-2323
This PR adds a delta() function that can be applied to gauge metrics.
The delta() function is intended to match Prometheus' delta() function:
The delta() function is applied to the metric values within each (Time Bucket, Attributes Hash) group. Since there can be multiple Attribute Hashes per time bucket, there can be multiple delta values per bucket. Therefore, the aggregation function (min, max, etc) is then be applied on top of the delta values, to select a value to show for the bucket.
Counter-intuitive Behavior
Testing
Alerts can be configured on the delta values
The delta checkbox is hidden if the selected metric is not a gauge