Alerting: Switch to snappy-compressed-protobuf for outgoing push requests to Loki #65077
Conversation
alexweav
added
area/alerting
|
Hello @alexweav!
Please, if the current pull request addresses a bug fix, label it with the |
| "golang.org/x/exp/slices" | ||
| ) | ||
|
|
||
| type JsonEncoder struct{} |
There was a problem hiding this comment.
nit: these structs are public but their methods are private, not sure if they should be.
There was a problem hiding this comment.
I made the types public so the anything wishing to use this as a library can pass in the right encoding. Such systems should not need to call any encoder methods directly, only pass one in
| // encoder serializes log streams to some byte format. | ||
| type encoder interface { | ||
| // encode serializes a set of log streams to bytes. | ||
| encode(s []stream) ([]byte, error) | ||
| // headers returns a set of HTTP-style headers that describes the encoding scheme used. | ||
| headers() map[string]string | ||
| } | ||
|
|
There was a problem hiding this comment.
nit: you're using the concrete implementation in tests anyways - I understand the "programming practice" but YAGNI it's a thing.
There was a problem hiding this comment.
We already do need it though - the main path uses snappyProto (since that's the main encoding) and tests use json (since it's human readable) so even within this PR we're already swapping out the implementation.
I originally tried it without the interface - enum for encoding type and if statements to call the right encoding method. The result felt pretty bug prone to me, as we needed branches in a number of places to get it right.
There was a problem hiding this comment.
Makes sense, I missed that detail!
|
The backport to To backport manually, run these commands in your terminal: # Fetch latest updates from GitHub
git fetch
# Create a new branch
git switch --create backport-65077-to-v9.4.x origin/v9.4.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x bf54f2672ef45d2737cd2ef5ea43f82b7312d12b
# Push it to GitHub
git push --set-upstream origin backport-65077-to-v9.4.x
git switch main
# Remove the local backport branch
git branch -D backport-65077-to-v9.4.xThen, create a pull request where the |
grafanabot
added
the
backport-failed
…ests to Loki (#65077) * Encode with snappy, always * JSON encoder type * Headers * Copy labels formatter from promtail * Implement snappy-proto encoding * Create encoder interface, test both encoders, choose snappy-proto by default * Make encoder configurable at the LokiCfg level * Export both encoders * Touch up comment and tests * Drop unnecessary conversions after move to plain strings to appease linter (cherry picked from commit bf54f26)
armandgrillet
added
the
product-approved
…ests to Loki (#65137) * Alerting: Switch to snappy-compressed-protobuf for outgoing push requests to Loki (#65077) * Encode with snappy, always * JSON encoder type * Headers * Copy labels formatter from promtail * Implement snappy-proto encoding * Create encoder interface, test both encoders, choose snappy-proto by default * Make encoder configurable at the LokiCfg level * Export both encoders * Touch up comment and tests * Drop unnecessary conversions after move to plain strings to appease linter (cherry picked from commit bf54f26) * Sample fields got renamed between 9.4 and main
…ests to Loki (#65077) * Encode with snappy, always * JSON encoder type * Headers * Copy labels formatter from promtail * Implement snappy-proto encoding * Create encoder interface, test both encoders, choose snappy-proto by default * Make encoder configurable at the LokiCfg level * Export both encoders * Touch up comment and tests * Drop unnecessary conversions after move to plain strings to appease linter
What is this feature?
Loki's HTTP API accepts exactly two formats:
snappy-compressed-protobufPromtail, the canonical Loki client, uses snappy-compressed-protobuf only. This is not configurable.
Many "loki-like" tools also seem to only accept snappy-compressed-protobuf.
This PR does the same - the default becomes snappy-compressed-protobuf. Right now this is configurable at the code level, when using the client as a library, but not at the grafana configuration level. We can evaluate adding this config option to grafana .ini in the future, but there's precedent to not do so in Promtail.
Why do we need this feature?
Consistency with Promtail. Interoperability with other Loki-like tools. Closer to the main code path in Loki.
Also, we send fewer bytes over the network.
Who is this feature for?
Operators and users who want to run state history against some Loki-like tools that aren't necessarily Loki.
Which issue(s) does this PR fix?:
n/a
Special notes for your reviewer:
Please check that: