-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
feat: Add metrics for Ingester RF-1 #13510
Conversation
29bee3f
to
7208d9c
Compare
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.
Looks great, and nice way to Observe the compressed size!
@@ -262,6 +279,8 @@ func (b *SegmentWriter) WriteTo(w io.Writer) (int64, error) { | |||
} | |||
total += int64(n) | |||
|
|||
b.outputSize.Store(total) |
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.
Nice!
i.metrics.chunksFlushFailures.Inc() | ||
i.metrics.flushesTotal.Add(1) | ||
if err := i.store.PutObject(ctx, fmt.Sprintf("loki-v2/wal/anon/"+id.String()), r); err != nil { | ||
i.metrics.flushFailuresTotal.Inc() | ||
return fmt.Errorf("store put chunk: %w", err) |
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.
not related to your PR but we should change this error message to "segment" to avoid confusion if it ever appears.
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.
Agree! 👍
} | ||
b.metrics.tenants.Observe(float64(len(tenants))) | ||
b.metrics.inputSizeBytes.Observe(float64(b.inputSize.Load())) | ||
b.metrics.outputSizeBytes.Observe(float64(b.outputSize.Load())) |
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.
Maybe we should only observe the output bytes if we've set a value here? In case we call Observe only after writing to the segment but haven't compressed/read it yet.
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.
Yeah! I was also thinking of documenting that Observe
should be called after it has written the output, but idk?
What this PR does / why we need it:
This pull requests adds a number of initial metrics for Ingester RF-1.
Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
Checklist
CONTRIBUTING.md
guide (required)feat
PRs are unlikely to be accepted unless a case can be made for the feature actually being a bug fix to existing behavior.docs/sources/setup/upgrade/_index.md
production/helm/loki/Chart.yaml
and updateproduction/helm/loki/CHANGELOG.md
andproduction/helm/loki/README.md
. Example PRdeprecated-config.yaml
anddeleted-config.yaml
files respectively in thetools/deprecated-config-checker
directory. Example PR