-
Notifications
You must be signed in to change notification settings - Fork 160
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
Commonlib table support #1290
Commonlib table support #1290
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.
Overall I think this looks fantastic, and today I learned more about sparklines and tables than I knew before, so thank you :D
One minor comment, but you're basically good to go from my PoV
@@ -38,7 +38,7 @@ local m1 = signal.init( | |||
expect: 'API server requests', | |||
}, | |||
testUnit: { | |||
actual: m1.asTimeSeries().fieldConfig.defaults.unit, | |||
actual: m1.asTimeSeries().fieldConfig.overrides[0].properties[1].value, |
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.
minor nit here as overrides[0].properties[1].value
is not quite as readable as defaults.unit
.
Would be good to have a comment for future maintainers explaining just what field is being looked at and why it's done in this manner
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.
There is a global place to to put units into any panel (fieldConfig.defaults.unit). But it will be used by multiple signals you can attach to a panel.
So in commonlib I decided to move units into overrides in order to make sure that units setting only applies to the specific signal. So that is why so long path in unit tests.
New transformations in commonlib to easily compose tables with two types of metric formats:
a) Prometheus metrics with instant=true, format=table, where all queries must have matching labels (drop with PromQL aggregations if necessary)
b) Prometheus metrics with instant=false, format=timeseries, where all queries must have matching labels ( drop with PromQL aggregations if necessary). Useful for building tables with Sparklines: https://grafana.com/docs/grafana/latest/panels-visualizations/visualizations/table/#sparkline
Example A:
Example b: