storegateway: small refactor to simplify #5444
Conversation
…t's used It was not specific to the protobuf package, and the comment overstated its requirements.
bboreham
changed the title
pkg/storegateway/bucket.go
Outdated
| @@ -719,12 +719,23 @@ func logSeriesRequestToSpan(ctx context.Context, l log.Logger, minT, maxT int64, | |||
| "msg", "BucketStore.Series", | |||
| "request min time", time.UnixMilli(minT).UTC().Format(time.RFC3339Nano), | |||
| "request max time", time.UnixMilli(maxT).UTC().Format(time.RFC3339Nano), | |||
| "request matchers", storepb.PromMatchersToString(matchers...), | |||
| "request block matchers", storepb.PromMatchersToString(blockMatchers...), | |||
| "request matchers", promMatchersToString(matchers...), | |||
There was a problem hiding this comment.
Can we use util.MatchersStringer(matchers).String()?
There was a problem hiding this comment.
While util.MatchersStringer(matchers).String() should be suitable, there is the difference that promMatchersToString surrounds the string with curly braces. It's probably no big deal if we stop doing that (surround with curly braces), as it's just for debug logs? Also util.MatchersStringer(matchers).String() is more performant due to using strings.Builder instead of concatenating strings.
There was a problem hiding this comment.
Even better: we can pass the stringer, since both go-kit and OpenTracing allow that as a value.
That means if the trace is not sampled and debug logging is off, we don't format them at all.
What this PR does
Remove some unused code and move other code that was not relevant to the
prompbprotobuf package.I was trying to understand the package, and found that this stuff detracted from that task.
Checklist
CHANGELOG.mdupdated