-
Notifications
You must be signed in to change notification settings - Fork 793
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
Add support for adding aliases to index and shard stats #563
Conversation
@sysadmind @SuperQ wondering if you had any thoughts or feedback on this |
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.
Having variable labels is not a good idea. I understand what the intent is, and I think that having aliases exposed is a good idea. I don't think that they belong in a label of the existing metrics however.
I think it would be better to have a new metric elasticsearch_indices_alias
with two labels index
and alias
. The scarping of the aliases would stay the same, but the reporting of the metric would be a loop over the indices and aliases and to expose the above gauge for each index-alias combination with a value of 1. This is a common pattern in prometheus for informational metrics.
How would this help your use case? The alias information would come at query time. You can use your normal query but add group_left(index)
to get the alias label into your existing queries.
@sysadmind thank you very much for the review! I will get started on those changes. Out of curiosity - what is the reason for not wanting aliases in labels of existing metrics? From the end user perspective it seems it would be easier to filter on a label rather than doing |
The main issue is that label values in Prometheus are opaque strings. So you end up with problems trying to do one-to-many relationships. Mapping a list of aliases to a coma separate list is not as useful as mapping to an information metric. Take your example, it's now much easier to match the literal single alias depend on which one you're looking for:
|
@SuperQ that makes sense, thank you for explaining! one last question - would you like the addition of the alias information metrics to be optionally enabled by a cli option, or always enabled? thanks again! |
I think we could enable it by default, with a flag to disable it if it generates too many metrics for some users. |
Optionally will include aliases in a label to index and shard stats Signed-off-by: Steven Cipriano <cipriano@squareup.com>
Signed-off-by: Steven Cipriano <cipriano@squareup.com>
* master: Refactor cluster info collector (prometheus-community#536) Fix linting that was missed in CI run (prometheus-community#568) Grafana dashboard: use new node exporter metric names (prometheus-community#501) publish total shards on a node (prometheus-community#535) Add additional collector for SLM stats (prometheus-community#558) Update common Prometheus files (prometheus-community#565) Update build (prometheus-community#562) Signed-off-by: Steven Cipriano <cipriano@squareup.com>
@SuperQ @sysadmind this is ready for another look, thanks! example stats:
|
- switch boolean flag - fix linting errors Signed-off-by: Steven Cipriano <cipriano@squareup.com>
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.
LGTM I think this is ok now
Nice |
…ommunity#563) * Add support for adding aliases to index and shard stats Optionally will include aliases in a label to index and shard stats Signed-off-by: Steven Cipriano <cipriano@squareup.com> * move aliases to separate informational metrics Signed-off-by: Steven Cipriano <cipriano@squareup.com> * address review - switch boolean flag - fix linting errors Signed-off-by: Steven Cipriano <cipriano@squareup.com> Signed-off-by: iishabakaev <iishabakaev@gmail.com>
…ommunity#563) * Add support for adding aliases to index and shard stats Optionally will include aliases in a label to index and shard stats Signed-off-by: Steven Cipriano <cipriano@squareup.com> * move aliases to separate informational metrics Signed-off-by: Steven Cipriano <cipriano@squareup.com> * address review - switch boolean flag - fix linting errors Signed-off-by: Steven Cipriano <cipriano@squareup.com>
Optionally will include aliases in a label to index and shard stats. Addresses #412
This adds a cli option
es.aliases
which when enabled, will add all aliases pointing to a given index, comma-separated, into a label calledaliases
forindex
andshard
stats.ex:
3 indices are created,
foo_1
,foo_2
, andfoo_3
.foo_2
has one alias pointing to it calledfoo_alias_2_1
.foo_3
has 2 aliases pointing to it calledfoo_alias_3_1
andfoo_alias_3_2
.without enabling
es.aliases
flag:with
es.aliases
flag: