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

returns memcached_up = 0 when fail to parse a stats values. #53

Merged

Conversation

f110
Copy link
Contributor

@f110 f110 commented Apr 17, 2019

fix #38

If there's an error when parse a stats value, returns memcached_up = 0.
because other metrics which succeeded in parsing value are useful.

If the LRU crawler does not run, ignoring some keys of related to it.

Signed-off-by: Fumihiro Itoh <fmhrit@gmail.com>
Copy link
Member

@grobie grobie left a comment

Choose a reason for hiding this comment

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

Thanks a lot for your contribution! I think we should not break the exporter for older memcached versions, so we'll need to find a solution how to handle the situation where a metric doesn't exist at all. See my comment below.

main.go Outdated Show resolved Hide resolved
main.go Outdated Show resolved Hide resolved
main.go Outdated Show resolved Hide resolved
Signed-off-by: Fumihiro Itoh <fmhrit@gmail.com>
Signed-off-by: Fumihiro Itoh <fmhrit@gmail.com>
for old memcached.

Signed-off-by: Fumihiro Itoh <fmhrit@gmail.com>
@f110
Copy link
Contributor Author

f110 commented Apr 18, 2019

Thank you for your review.

I fixed a compatible and some problems.
Could you please review again?

@f110
Copy link
Contributor Author

f110 commented Jun 1, 2019

@grobie ping

@f110
Copy link
Contributor Author

f110 commented Aug 25, 2019

@grobie Is there anything else to need for merging?

Copy link
Member

@grobie grobie left a comment

Choose a reason for hiding this comment

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

Thanks for the updates @f110! What do you think about logging key issues only on debug level, to not pollute the logs for old memcached versions?

main.go Outdated Show resolved Hide resolved
Signed-off-by: Fumihiro Itoh <fmhrit@gmail.com>
@f110
Copy link
Contributor Author

f110 commented Aug 25, 2019

@grobie Thank you for review!

You are right!
This log is very noisy for older memcached. for example memcached 1.4.17 doesn't have the stats which relevant to lru_crawler ( lru_crawler_starts, crawler_items_checked etc...)

And, There is already to output very noisy log by the previous my PR (#46). So i decided to fix that problem too on this PR.

@grobie grobie merged commit 4a111e3 into prometheus:master Aug 25, 2019
@grobie
Copy link
Member

grobie commented Aug 25, 2019

Thanks a lot for your contribution @f110. I squashed your commits together for readability and will release a new version shortly.

@f110 f110 deleted the failed-scrape-when-fail-to-parse-valuve branch August 25, 2019 15:36
@f110
Copy link
Contributor Author

f110 commented Aug 25, 2019

Thank you for the merge.
And I appreciate your patience for my English.

@grobie
Copy link
Member

grobie commented Aug 26, 2019

Version v0.6.0 has been published https://groups.google.com/forum/#!topic/prometheus-announce/INfriYx3o1g

Thanks again @f110 and don't worry at all, your comments were easy to read and understand.

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.

Don't use NaNs as error values
2 participants