-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Compact : parameter to control warnings for empty chunks #7405
Conversation
Signed-off-by: Vanshikav123 <vanshikav928@gmail.com>
@@ -113,7 +116,11 @@ func (w *streamedBlockWriter) WriteSeries(lset labels.Labels, chunks []chunks.Me | |||
} | |||
|
|||
if len(chunks) == 0 { | |||
level.Warn(w.logger).Log("msg", "empty chunks happened, skip series", "series", strings.ReplaceAll(lset.String(), "\"", "'")) | |||
if w.logEmptyChunks { | |||
level.Warn(w.logger).Log("msg", "empty chunks happened, skip series", "series", strings.ReplaceAll(lset.String(), "\"", "'")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't get the purpose of this parameter if it is always true. Why not just convert this line to level.Debug
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is there any option to configure it through yaml ? or any config struct which can implement this ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the only way i found was through the NewStreamedBlockWriter
function which is called in downsample.go
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 for jus tusing level.Debug
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's no way to control and it doesn't make sense to add a tunable parameter for this kind of minute thing. We already have way too many parameters and it is confusing for our users.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
or other options to fix this issue is by aggregating the warnings ? wdyt .. i think directly changing the level to debug is not an good option
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's no way to control and it doesn't make sense to add a tunable parameter for this kind of minute thing. We already have way too many parameters and it is confusing for our users.
yaa ! that's a point too .. so is there any need to change the level to debug?
Changes
#7182
added a parameter
logEmptyChunks
which helps in controlling warnings when prometheus creates empty check which can be configured byNewStreamedBlockWriter
functionVerification