search

prometheus-junkyard / tsdb

The Prometheus time series database layer.
Apache License 2.0
835 stars 178 forks source link

add metric for tsdb size retention bytes #667

Closed YaoZengzeng closed 5 years ago

YaoZengzeng commented 5 years ago

Signed-off-by: YaoZengzeng yaozengzeng@zju.edu.cn

fixes: prometheus/prometheus#5790

YaoZengzeng commented 5 years ago

@brian-brazil PTAL

krasi-georgiev commented 5 years ago

Why do you need this?

YaoZengzeng commented 5 years ago

@krasi-georgiev Just fix issue prometheus/prometheus#5790

YaoZengzeng commented 5 years ago

@brian-brazil Updated, naming is always hard for me 😄

But as @krasi-georgiev said, I'm also wondering if this metric is really necessary.

brian-brazil commented 5 years ago

It's generally useful to have metrics like this for flags that set limits.

krasi-georgiev commented 5 years ago

Yes I understand the use case, but seems a bit strange to add metrics that are constants like - version numbers, config flags etc. Anyway if @brian-brazil is ok with this feel free to merge.

brian-brazil commented 5 years ago

Config flags like this which are limits are useful for alerting (though this specific one isn't quite, as retention handling is automatic).

krasi-georgiev commented 5 years ago

Can you please add a test for this metric as well to ensure that the value is the same as the set value when opening a db.

look at the db size tests for some ideas.

YaoZengzeng commented 5 years ago

@brian-brazil @krasi-georgiev PTAL

brian-brazil commented 5 years ago

:+1:

YaoZengzeng commented 5 years ago

@brian-brazil @krasi-georgiev Any new comments?

krasi-georgiev commented 5 years ago

nothing on my side LGTM