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

request params not correctly passed to request.get in PrometheusConnect.custom_query #260

Open
GiulioGx opened this issue Apr 5, 2023 · 2 comments

Comments

@GiulioGx
Copy link

GiulioGx commented Apr 5, 2023

At line 346 of prometheus_connect.py we have:

response = self._session.get(
"{0}/api/v1/query".format(self.url),
params={**{"query": query}, **params},
verify=self.ssl_verification,
headers=self.headers,
)

the params dict, ultimately used to pass params (for instance timeouts) to the requests get method is not correctly passed.

It should be modified to:

response = self._session.get(
"{0}/api/v1/query".format(self.url),
params={**{"query": query}},
**params,
verify=self.ssl_verification,
headers=self.headers,
)

@chauhankaranraj
Copy link
Collaborator

Hi @GiulioGx, thanks for opening the issue! Line 346 seems to be a comment, is this referring to line 355 instead?

Anyway, I believe params are the parameters being passed for the query to the Prometheus server (see this line for an example), so the current implementation seems correct to me. I still tried replicating your suggestion as follows:

import requests
from requests.adapters import HTTPAdapter

url = 'https://prometheus.demo.do.prometheus.io'

session = requests.Session()
session.mount(url, HTTPAdapter(max_retries=3))

params = {'time': 1680746497}
response = session.get(
    "{0}/api/v1/query".format(url),
    params={**{"query": "ALERTS"}},
    **params,
)

print(response.content)

But the above gave me an error, so I had to replace the call to be like the current implementation

params = {'time': 1680746497}
response = session.get(
    "{0}/api/v1/query".format(url),
    params={**{"query": "ALERTS"}, **params},
)

which succeeded.

Does this help clarify your issue or did I misunderstand your problem?

@GiulioGx
Copy link
Author

GiulioGx commented Apr 6, 2023

Hi @chauhankaranraj, thanks for the quick reply!

The above code does not work because time is not a parameter accepted by the request() method.
Modifying time to timeout works for me.

(The original issue I had was in fact trying to change the timeout to the http request to the prom server)

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

No branches or pull requests

2 participants