feat: ccr stats - #1145
Conversation
5b746fb to
5a8ab1d
Compare
| []string{"follower_index"}, | ||
| nil, | ||
| ) | ||
| ccrFollowShardInfo = []ccrShardMetric{ |
There was a problem hiding this comment.
All of these should be package vars, not nested in a slice. It's our standard for all new collectors. It helps us prevent unused metric descriptions.
| nil, | ||
| ) | ||
| ccrAutoFollowedClusterTimeSinceLastCheckMilliseconds = prometheus.NewDesc( | ||
| prometheus.BuildFQName(namespace, "ccr_auto_followed_cluster", "time_since_last_check_milliseconds"), |
There was a problem hiding this comment.
Prometheus standards are to align to seconds for time. See https://github.com/prometheus-community/elasticsearch_exporter/blob/master/collector/slm.go#L203
| }, | ||
| { | ||
| desc: prometheus.NewDesc( | ||
| prometheus.BuildFQName(namespace, "ccr_follow_shard", "time_since_last_read_milliseconds"), |
There was a problem hiding this comment.
This should also be normalized to seconds
| registerCollector("ccr", defaultDisabled, NewCCR) | ||
| } | ||
|
|
||
| type ccrShardMetric struct { |
There was a problem hiding this comment.
What value does this struct provide? I think we can drop it.
| if err != nil { | ||
| t.Fatal(err) | ||
| } | ||
| if !hasMetric(descs, "elasticsearch_ccr_follow_index_global_checkpoint_lag") { |
There was a problem hiding this comment.
The tests in this project for new collectors test the full output. This helps us catch breaking changes more easily. See a good example here: https://github.com/prometheus-community/elasticsearch_exporter/blob/master/collector/slm_test.go#L30
| | elasticsearch_ccr_auto_followed_cluster_last_seen_metadata_version | gauge | 1 | Last seen metadata version for an auto-followed cluster | | ||
| | elasticsearch_ccr_auto_followed_cluster_time_since_last_check_milliseconds | gauge | 1 | Time since last auto-follow check in milliseconds for an auto-followed cluster | | ||
| | elasticsearch_ccr_follow_index_global_checkpoint_lag | gauge | 1 | Total global checkpoint lag for a follower index | | ||
| | elasticsearch_ccr_follow_shard_successful_read_requests_total | counter | 4 | Successful read requests for a follower shard (`collector.ccr.detailed=true`) | |
There was a problem hiding this comment.
This section is missing a large number of metrics that are added by this PR.
| | elasticsearch_ccr_auto_follow_successful_follow_indices_total | counter | 0 | Number of indices auto-follow successfully followed | | ||
| | elasticsearch_ccr_auto_follow_recent_errors_total | counter | 0 | Number of recent auto-follow errors currently reported | | ||
| | elasticsearch_ccr_auto_followed_cluster_last_seen_metadata_version | gauge | 1 | Last seen metadata version for an auto-followed cluster | | ||
| | elasticsearch_ccr_auto_followed_cluster_time_since_last_check_milliseconds | gauge | 1 | Time since last auto-follow check in milliseconds for an auto-followed cluster | |
There was a problem hiding this comment.
These will need to be updated when the metrics are changed in ccr.go
|
Thanks for the thorough review, @sysadmind I've addressed all of your feedback:
Please take another look when you get a chance. Happy to make further adjustments if needed |
Signed-off-by: kizuna-lek <asd98041@qq.com>
Signed-off-by: kizuna-lek <asd98041@qq.com>
Signed-off-by: kizuna-lek <asd98041@qq.com>
2194d0e to
b459b5b
Compare
2 participants
Uh oh!
There was an error while loading. Please reload this page.