Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add the ability to use a custom HTTP Client to the push #341

Closed
shlimp opened this issue Aug 30, 2017 · 1 comment
Closed

Add the ability to use a custom HTTP Client to the push #341

shlimp opened this issue Aug 30, 2017 · 1 comment
Assignees
Milestone

Comments

@shlimp
Copy link

shlimp commented Aug 30, 2017

Currently, the push to gateway function uses http.DefaultClient to make the request to the pushgateway.
By abling to use a custom client, there can be more flexibility on how requests are sent.
For my specific usage when I encoutered the need, I am using the client from GAE, which doesn't allow at all using the http.DefaultClient, which makes the library unusable in GAE

@beorn7
Copy link
Member

beorn7 commented Aug 31, 2017

Good point.

Question is how to implement this best. The naive approach would be to duplicate each existing function in the push package to also accept an http.Client, i.e. func push.Collectors(job string, grouping map[string]string, url string, collectors ...prometheus.Collector) error would be complemented by func push.CollectorsWithClient(job string, grouping map[string]string, url string, client http.Client, collectors ...prometheus.Collector) error. However, the multiplicity of options calls for either an options approach or a builder approach. Mock-up calls:

Options approach:

// Easy case:
push.Collectors("http://example.org/metrics", "my_job", push.Options{}, myCollector)

// Complex case:
push.Collectors(
    "http://example.org/metrics",
    "my_job", 
    push.Options{
        Add: true,
        Grouping: map[string]string{"zone": "xy"},
        Client: &gae.Client{},
    },
    myCollector1, myCollector2,
)

Builder approach

// Easy case:
push.New("http://example.org/metrics", "my_job").Collector(myCollector).Push()

// Complex case:
push.New("http://example.org/metrics", "my_job").
    Collector(myCollector1).
    Collector(myCollector1).
    Grouping("zone", "xy").
    Client(&gae.Client{}).
    Add()

I like the builder approach as it naturally allows for multiple Collectors, Gatherers, and even Groupings without maps and/or variadic args. And it would allow to distinguish between the normal push and the "add" push by simply two different methods. It would be a completely different interface. We could simply have them both in the push package and then deprecate the old ones.

@shlimp Now your feature request triggered something way bigger.
Anybody else, what are your opinions? Is it worth it to add something like the above (with the long-term intention of deprecating the old use)? In particular @grobie with whom I discussed the current functions quite a bit (and we were both not entirely happy with it but couldn't come up with something better at that time).

@beorn7 beorn7 self-assigned this Aug 31, 2017
@beorn7 beorn7 added this to the v0.9 milestone Aug 31, 2017
beorn7 pushed a commit that referenced this issue Feb 12, 2018
beorn7 pushed a commit that referenced this issue Feb 13, 2018
This allows adding more options in elegant ways, showcased here by
HTTP basic auth and by injecting a custom http.Client.

Fixes #341 and #372.
beorn7 pushed a commit that referenced this issue Feb 14, 2018
This allows adding more options in elegant ways, showcased here by
HTTP basic auth and by injecting a custom http.Client.

Fixes #341 and #372.
bobmshannon pushed a commit to bobmshannon/client_golang that referenced this issue Apr 10, 2018
This allows adding more options in elegant ways, showcased here by
HTTP basic auth and by injecting a custom http.Client.

Fixes prometheus#341 and prometheus#372.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants