[PRW-2.0] New config surface allowing content-type and encoding preferences for both sending and receiving. #13968
Conversation
…ces for both sending and receiving. Signed-off-by: bwplotka <bwplotka@gmail.com>
bwplotka
changed the title
| a.Flag("web.enable-remote-write-receiver", "Enable API endpoint accepting remote write requests."). | ||
| Default("false").BoolVar(&cfg.web.EnableRemoteWriteReceiver) | ||
|
|
||
| a.Flag("web.remote-write-receiver.protobuf-types", fmt.Sprintf("List of accepted remote write 2.0 content types to advertise to senders, ordered by the preference. Note that the final decision is on the sender. Supported list values: %v", config.DefaultRemoteWriteProtoTypes.String())). | ||
| Default(config.DefaultRemoteWriteProtoTypes.Strings()...).SetValue(rwProtoTypeFlagValue(&cfg.web.RemoteWriteReceiverProtoTypes)) | ||
|
|
||
| a.Flag("web.remote-write-receiver.compressions", fmt.Sprintf("List of accepted remote write 2.0 content encodings (compressions) to advertise to senders, ordered by the preference. Note that the final decision is on the sender. Supported list values: %v", config.DefaultRemoteWriteCompressions.String())). | ||
| Default(config.DefaultRemoteWriteCompressions.Strings()...).SetValue(rwCompressionFlagValue(&cfg.web.RemoteWriteReceiverCompressions)) | ||
|
|
There was a problem hiding this comment.
I wonder if we can make a case for moving all of this, including the flag to enable the receiver, to the config file
There was a problem hiding this comment.
Nice, totally possible. But it's hard to do in our feature - it's too many changes.
Perhaps NOT exposing any flags for now and adding TODO + issue would be fine for now?
| // TODO(bwplotka): Consider an util for "preference enums" as it's similar code for rw compression, proto type and scrape protocol. | ||
|
|
||
| // RemoteWriteProtoType represents the supported protobuf types for the remote write. | ||
| type RemoteWriteProtoType string |
There was a problem hiding this comment.
ProtoType feels awkward, can we call it RemoteWriteProtoVersion ?
There was a problem hiding this comment.
I tried ProtoMessages, ProtoVersion etc but they might be worse options.
ProtoVersion feels like Protocol Version, so spec version? But it represent protobuf type, to in short prototype, not protocol version and I want to make it explicit. Or do you think ProtoVersion would not mislead?
Perhaps RemoteWriteProtobufType would be better?
There was a problem hiding this comment.
RemoteWriteProtobufType or RemoteWriteProtobufVersion, or maybe RemoteWriteWireType/Format/Version since we've discussed implementing and allow usage of something like Arrow in the future?
| // ServerAcceptEncodingHeaderValue returns server Accept-Encoding header value for | ||
| // given list of compressions as per RFC 9110 https://www.rfc-editor.org/rfc/rfc9110.html#name-accept-encoding | ||
| func (cs RemoteWriteCompressions) ServerAcceptEncodingHeaderValue() string { | ||
| // TODO(bwplotka): Consider implementing an optional quality factor. |
There was a problem hiding this comment.
by not doing so yet we're just interpreting the order as the preference ordering, right?
There was a problem hiding this comment.
Yes, which is part of RFC 9110 spec, q is optional and allow to tell "weight" of the ordering.
| contentEncoding := r.Header.Get("Content-Encoding") | ||
| protoVer := r.Header.Get(RemoteWriteVersionHeader) | ||
| func (h *writeHandler) parseType(contentType string) (config.RemoteWriteProtoType, error) { | ||
| // TODO(bwplotka): TBD |
There was a problem hiding this comment.
I gave up and figured out we need HTTP util pkg for preferences (: TBD, IF we want to go this way (Accet + Accept-Encoding)
TODO(bwplotka):