Deleting a logstream should delete its stats. #419
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #412 .
Description
The intention is to fix the bug mentioned in the issue, where get_stats reported the last recorded data of a logstream, even though it has already been deleted. Since the logstream has already been deleted, no stats data should be available.
Returning a 404
Currently, the
stats::get_current_stats
also returns Stats for logstreams that do not exist and fills the struct with 0 values. This leads to callers not actually knowing, whether stats for this stream actually exist. While the signature actually uses anOption<Stats>
, theNone
variant is only returned, if the underlying prometheus function returns anErr
. According to the docs, this should only happen, if the number of labels don't match.Since the underlying
MetricVec
s containing the stats, don't seem to have a method to check if the passed in labels already exist, it seems to be better to check at the caller's side whether a stream exists usingmetadata::STREAM_INFO.stream_exists(..)
. If it does, check the stored metrics (stats), if not, return a 404.Deleting stats when a stream is deleted
I added a function in the
stats
module to delete the metrics for the deleted logstream, which is then called in the delete logstream handler.Since the
prometheus::vec::MetricVec::get_metric_with_label_values
andprometheus::vec::MetricVec::remove_label_values
explicitly warn of passing multiple labels into the respective function, I extracted the label creation into their own functions, so both thestats::get_current_stats
and the newstats::delete_stats
always use the same labels.Warning
Previously, the
get_stats
handler returned a200
, even for logstreams that have never been created. Now, it returns a404
for logstreams that do not exist.This PR has: