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

prometheus/promhttp: actually observe values in trace in example #544

Merged
merged 1 commit into from Mar 7, 2019

Conversation

s-urbaniak
Copy link
Contributor

This is similar to
#535 fixing ExampleInstrumentRoundTripperDuration.

cc @brancz @mxinden @beorn7

This is similar to
prometheus#535 fixing ExampleInstrumentRoundTripperDuration.

Signed-off-by: Sergiusz Urbaniak <sergiusz.urbaniak@gmail.com>
@brancz
Copy link
Member

brancz commented Mar 6, 2019

lgtm 👍 as a side note .. we should probably test in this test that after doing the request we actually get some metrics populated, no? (as this test wasn't present before I don't mind whether that happens in this or a follow up PR)

@s-urbaniak
Copy link
Contributor Author

@brancz that's actually a good idea, let me see how I can add such a test.

@beorn7
Copy link
Member

beorn7 commented Mar 7, 2019

It would be great to verify, but also note that this is just an example and mostly meant to document usage than actually testing anything. If the example gets confusing by adding the verification, it would defeat the main purpose of the example.

In different news: This code queries http://google.com. Since the example code is actually run, this actually performs the request, and if it fails, it will fail the test. That's another wart here.

In any case, merging this now to have a correct example in the documentation. Other concerns about this can be addressed separately.

@beorn7 beorn7 merged commit c5e1469 into prometheus:master Mar 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants