puetzp / prometheus-http-query

Prometheus HTTP API client
MIT License
14 stars 7 forks source link

Latest Thanos Update breaks the library #15

Open jan-kantert opened 2 days ago

jan-kantert commented 2 days ago

We use this library to query thanos. Recent versions added a new entry to the json response:

{
  "status": "success",
  "data": {
    "resultType": "vector",
    "result": [
      {
        "metric": {},
        "value": [
          1733333548.697,
          "0"
        ]
      }
    ],
    "stats": {
      "samples": {
        "totalQueryableSamples": 63,
        "totalQueryableSamplesPerStep": null,
        "peakSamples": 18
      }
    },
    "analysis": {
      "name": "",
      "executionTime": "0s",
      "children": null
    }
  },
  "warnings": [
    "PromQL info: metric might not be a counter, name does not end in _total/_sum/_count/_bucket: \"nginx_ingress_controller_requests\""
  ]
}

This results in this error inside the library:

failed to parse JSON response from server
jan-kantert commented 2 days ago

This starts with thanos v0.37.0 which merged https://github.com/thanos-io/thanos/pull/7852. prometheus-http-query expects a timing field: Error: missing field "timings".

puetzp commented 4 hours ago

Hey there,

I would like to wait how the discussion on the linked issue plays out. Seems like the easiest fix would be to make the timings field in stats optional (which it currently is not) and default to None when it is not provided in the response returned by the server.

However I am hesitant to change the API when there is still a possibility that the Thanos project might fix it on their end to ensure Prometheus API compatibility. I am unsure how likely that is given I that I am not well acquainted with the Thanos project.

Please let me know if you disagree. Otherwise I am sure to keep an eye on the issue but wait a little bit before providing a solution.