-
Notifications
You must be signed in to change notification settings - Fork 177
chore: Aerospike Mixin Modernization #1475
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
chore: Aerospike Mixin Modernization #1475
Conversation
…ikei/jsonnet-libs into chore/aerospike-mixin-modernization
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.
Just a couple minor comments on this one, really appreciate you including the rendered dashboards in the dashboards_out here, made it a lot easier to diff the changes as a sanity check.
Validated locally as well, all looks good to me.
| unit: 'none', | ||
| sources: { | ||
| prometheus: { | ||
| expr: 'sum by(job, aerospike_cluster, instance) (aerospike_namespace_ns_cluster_size{%(queriesSelector)s})', |
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.
Same comment as on other PRs on whether we want to have a selector for sum (and others?)
I'm generally in favour, but if you have strong concerns I'm good to leave it as-is
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.
Ah I think I caught on within my recent commit in #1454, seems like a great idea for extensibility :) I'll go ahead and make similar changes here shortly for this PR 👍
… instanceLabels, improve legend re-usage where appropriate
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.
Looks great, thanks Keith!
This modernizes the aerospike mixin to use signals and commonlib to hopefully provide a more maintainable mixin for the future.
Throughout this we have also added support for some breaking changes around Aerospike 7.0+ versions which removed a couple of prometheus metrics that this mixin relied upon:
https://aerospike.com/docs/database/reference/metrics#namespace__device_available_pct
https://aerospike.com/docs/database/reference/metrics#namespace__memory_free_pct
To get around this we used signals to combine the queries.
Aerospike overview

Aerospike instance overview


Aerospike namespace overview

Aerospike logs
