-
Notifications
You must be signed in to change notification settings - Fork 795
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(*): add ilm metrics #513
feat(*): add ilm metrics #513
Conversation
Hi @iishabakaev! It seems some work has already been done in #457. Also, as your work seems very close to @mokrinsky's one, it could be a good idea to liaise with them to avoid duplicate work. |
@arapaho The other PR seems abandoned and isn't getting any response from the original author so would be good to continue with this if @iishabakaev is happy to carry on -- If not I'm happy to pick it up |
a5d9c6a
to
bb4b91c
Compare
Hi @arapaho! Seems like other PR wont be merged due in inactivity. |
Hey :) We're also interested in such a feature, can we ask when the PR will be reviewed/merged? Thanks in advance |
Signed-off-by: iishabakaev <[email protected]>
bb4b91c
to
73d23aa
Compare
Hi! Can I ask you guys to review this PR, please? For those who uses ILM in their clusters (as I do) its very helpful metrics |
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 good functionality wise. Just some refactoring before this is ready to merge.
Thanks! 😄
Hey :) any chance the change will be released? thanks |
Hi, I'm commenting to show interest in this PR, any chance the review can be finished soon? |
8cfda85
to
43daa7b
Compare
Signed-off-by: Шабакаев Илья Исмаилович <[email protected]> Signed-off-by: Шабакаев Илья Исмаилович <[email protected]>
43daa7b
to
5500ee6
Compare
Hi! @sysadmind Sorry for so long changes, but I finally did it |
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. Thanks for the fixes.
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.
The linter found a variable that needs to be fixed.
Signed-off-by: Шабакаев Илья Исмаилович <[email protected]>
aafa91a
to
96b4f36
Compare
Done. |
Can you please update the README (https://github.com/prometheus-community/elasticsearch_exporter#readme) with the new metrics? |
BREAKING CHANGES: The flag `--es.cluster_settings` has been renamed to `--collector.clustersettings`. * [CHANGE] Rename --es.cluster_settings to --collector.clustersettings * [FEATURE] Add ILM metrics #513 * [ENHANCEMENT] Add ElasticCloud node roles to role label #652 * [ENHANCEMENT] Add ability to use AWS IAM role for authentication #653 * [ENHANCEMENT] Add metric for index replica count #483 * [BUGFIX] Set elasticsearch_clusterinfo_version_info guage to 1 #728 * [BUGFIX] Fix index field counts with nested fields #675 --------- Signed-off-by: Joe Adams <[email protected]>
Add ilm cluster status and ilm index metrics from /_all/_ilm/explain