fix: use slog.DiscardHandler in promslog.NopLogger - #960
Open
FUSAKLA wants to merge 2 commits into
Open
Conversation
Signed-off-by: Martin Chodur <m.chodur@seznam.cz>
Signed-off-by: Martin Chodur <m.chodur@seznam.cz>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Investigating huge latencies in pushgateway after upgrade from 1.10 to 1.11 I found out that the cause is the
promslog.NopLogger(lets put aside that pushgateway logging might be too excessive)It turned out that
NewNopLogger()returnsNew(&Config{Writer: io.Discard})— a regularslog.TextHandlerat info level.Its
Enabled()returns true, so slog builds the record, the handler formats every attribute, and only then drops the bytes atio.Discard. For larger attributes this leads to meaningless serialization just to be thrown away.checkWriteRequest→processWriteRequestruns on every push.logger.Info(..., "new", mf, "old", existingMF)— both*dto.MetricFamilywith all their metrics.Fix:
Use the slog.DiscardHandler added in Go 1.24 which is really no-op.
slog.DiscardHandlerneeds Go 1.24; this module is on 1.25.0.LogValuerimplementations with side effects will no longer be called.NewNopLogger()and expectingEnabled()to be true will stop seeing those calls.Benchmark
Benchmark details —
go test ./promslog/ -run XXX -bench BenchmarkNopLogger -benchmem, go1.26.5 linux/amd64, AMD Ryzen AI 7 445 (12 threads).Oldis the current implementation, the other one is the patch.