search

blue-yonder / azure-cost-mon

Prometheus exporter for the Azure billing API
MIT License
61 stars 13 forks source link

Use 'Scrape-Timeout-Seconds' header instead of hardcoded timeout. #7 #11

Closed treebee closed 7 years ago

treebee commented 7 years ago

This implements #7

Not really happy with the test and that we have to catch this RuntimeError only for the unit test case, but I don't see a way to pull the value of the request header into the collector besides using flask's request object.

coveralls commented 7 years ago

Coverage Status

Coverage remained the same at 100.0% when pulling 743e167ab87b17cda448cf9f276207072ba59208 on treebee:scrape_timeout_header into 2abd1cbce6f9d315a145638b3fb0f046a962e1bc on blue-yonder:master.

ManuelBahr commented 7 years ago

Maybe @MathMagique has a clever idea how to test this w/o the exception? Therefore I added him to the reviewers

ManuelBahr commented 7 years ago

As an idea: If we hold a reference to the collector, we could modify the instance at runtime on each call of /metrics and set the timeout. This way the collector doesn't need to interfere with flask at all.

MathMagique commented 7 years ago

I also thought about that. This solution would require to provide a custom /metrics endpoint plus I have no idea what would happen should concurrent requests ever come in. Probably not much due to the global interpreter lock and how Python handles parallelism, but still it feels awkward. The real solution would be to get this is a first-class feature into Prometheus Python client.

ManuelBahr commented 7 years ago

metrics is custom already. We would just shift the header parsing code there. The concurrency is indeed not trivial, because it might depend on the wsgi http server which concurrency model is used.

Ahh, good old days when I constructed the metrics output manually.

coveralls commented 7 years ago

Coverage Status

Coverage remained the same at 100.0% when pulling 7830a5a5a0ada1a97752649b7cf9cf6514953a48 on treebee:scrape_timeout_header into 2abd1cbce6f9d315a145638b3fb0f046a962e1bc on blue-yonder:master.

MathMagique commented 7 years ago

Looks good to me as well. I like the local registry approach you have taken that basically solves any concurrency problems that may have appeared with the global registry. Also, I like that the collector does not know it is supposed to run in a request context. Good job!