Skip to content
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 search_query_current and indexing_index_current #485

Merged
merged 8 commits into from
Oct 25, 2021

Conversation

locmai
Copy link
Contributor

@locmai locmai commented Oct 18, 2021

Signed-off-by: Loc Mai [email protected]

We are missing these 2 important metrics for our ES clusters:

indexing_index_current
search_query_current

Copy link
Contributor

@SuperQ SuperQ left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These metric names are very confusing.

collector/indices.go Outdated Show resolved Hide resolved
collector/indices.go Outdated Show resolved Hide resolved
Signed-off-by: Loc Mai <[email protected]>
@locmai locmai requested a review from SuperQ October 20, 2021 08:22
@locmai
Copy link
Contributor Author

locmai commented Oct 20, 2021

@SuperQ Big thanks for helping to review this! The _current naming that I follow similar to search_scroll_current one and also the pattern that already be in the code: QueryTotal => query_total, QueryCurrent => query_current.

Also updated the descriptions ^^

@SuperQ
Copy link
Contributor

SuperQ commented Oct 20, 2021

In Prometheus, we typically don't use direct 1:1 mappings for these kinds of things. The problem is a lot of systems (like Elastic) use statsd-style naming conventions, which are very hard to understand. We fix the naming in order to make metrics easier to use by the end user.

@locmai
Copy link
Contributor Author

locmai commented Oct 20, 2021

In Prometheus, we typically don't use direct 1:1 mappings for these kinds of things. The problem is a lot of systems (like Elastic) use statsd-style naming conventions, which are very hard to understand. We fix the naming in order to make metrics easier to use by the end user.

Great point. But the indices.go looks like everything is 1:1 mapping. I think I will fix the new ones first then.

Co-authored-by: Ben Kochie <[email protected]>
Signed-off-by: Loc Mai <[email protected]>
@locmai locmai force-pushed the add-current-index branch from 3ce5483 to 963823e Compare October 20, 2021 08:38
@locmai locmai force-pushed the add-current-index branch from 537cb2d to 71e4b91 Compare October 20, 2021 08:56
Signed-off-by: Loc Mai <[email protected]>
Signed-off-by: Loc Mai <[email protected]>
@SuperQ SuperQ requested a review from sysadmind October 20, 2021 11:10
@locmai
Copy link
Contributor Author

locmai commented Oct 25, 2021

Hi @sysadmind could you help me take a look at this PR?

collector/indices.go Outdated Show resolved Hide resolved
@locmai locmai requested a review from sysadmind October 25, 2021 19:44
@sysadmind sysadmind merged commit dede9e9 into prometheus-community:master Oct 25, 2021
jnadler pushed a commit to jnadler/elasticsearch_exporter that referenced this pull request Oct 27, 2022
…nity#485)

* Add search_query_current and indexing_index_current

Signed-off-by: Loc Mai <[email protected]>

* update descriptions

Signed-off-by: Loc Mai <[email protected]>

* Update collector/indices.go

Co-authored-by: Ben Kochie <[email protected]>
Signed-off-by: Loc Mai <[email protected]>

* rename to current_indexed_docs

Signed-off-by: Loc Mai <[email protected]>

* update docs

Signed-off-by: Loc Mai <[email protected]>

* fix ordering

Signed-off-by: Loc Mai <[email protected]>

* fix ordering

Signed-off-by: Loc Mai <[email protected]>

* change back to index_current

Signed-off-by: Loc Mai <[email protected]>

Co-authored-by: Ben Kochie <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants