feat(push): add format builder option #540
Conversation
|
What's your use case? My first spontaneous reaction was: The PGW wants the protobuf format. Why add the complication? My second reaction: You probably have another push target. It would be nice to know which. And then I would suggest to explore HTTP content negotiation. My thirst reaction: This should be dealt with in the same way as the other options, i.e. with another of the "builder pattern" methods, and not with an additional parameter to |
|
My use case is a custom push endpoint that only accepts text encoding. I saw content negotiation as a handler in the promhttp.HandleFor method, but it isn't a pusher and I can't find any examples of agent driven/reactive content negotiation in go so it doesn't appear to be the right path. |
Allow users to specify which format the pusher should push in. Signed-off-by: Frank Spitulski <fspituls@ucsd.edu>
|
I think this implementation fits better @beorn7. |
FrankSpitulski
changed the title
|
Indeed, agent-driven content negotiation in HTTP seems to be quite hard and needed quite a bit of work on both sides. Thus, it's probably a good thing to let the user of the I'll review the code ASAP. |
| @@ -197,7 +208,7 @@ func (p *Pusher) push(method string) error { | |||
| return err | |||
| } | |||
| buf := &bytes.Buffer{} | |||
| enc := expfmt.NewEncoder(buf, expfmt.FmtProtoDelim) | |||
| enc := expfmt.NewEncoder(buf, p.expfmt) | |||
There was a problem hiding this comment.
p.expfmt has to be used, too, down in line 236, instead of the hardcoded expfmt.FmtProtoDelim.
There was a problem hiding this comment.
good catch, updated
prometheus/push/push.go
Outdated
| @@ -182,6 +185,14 @@ func (p *Pusher) BasicAuth(username, password string) *Pusher { | |||
| return p | |||
| } | |||
|
|
|||
| // Format configures the Pusher to use an encoding format given by the | |||
| // provided expfmt.Format. For convenience, this method returns a | |||
| // pointer to the Pusher itself. | |||
There was a problem hiding this comment.
Perhaps add what the default is (expfmt.FmtProtoDelim) and that that's the one most natural for the Prometheus Pushgateway, and that the Pushgateway will understand all the formats, but custom implementations of a push receiver might require a specific one.
Signed-off-by: Frank Spitulski <fspituls@ucsd.edu>
|
Looks great. Thank you very much. |
Allow users to specify which format the pusher should push in.
Also added tests related to changing the format of the pusher.
This does not break existing functionality since the argument
is varadic.
@beorn7
Signed-off-by: Frank Spitulski fspituls@ucsd.edu