search

observatorium / obsctl

A cli to interact with Observatorium instances.
Apache License 2.0
10 stars 14 forks source link

Use OpenAPI client #2

Closed saswatamcode closed 2 years ago

saswatamcode commented 2 years ago

This PR adds an OpenAPI spec for the Observatorium API (only metrics routes for now) as well as a client which is generated using deepmap/oapi-codegen and go-chi.

The client is used to send requests (both GET and PUT) to the Observatorium API. Also adds support for URL parameters in requests, by adding flags for obsctl metrics get operations. Fixes #8. Adds functionality to query command as well, which allows for executing both instant and range queries.

bill3tt commented 2 years ago

Nice work :muscle: Looking at the OpenAPI spec, it looks like we're defining the interfaces for all of the Prometheus responses, I'm just wondering if these are defined anywhere else and we can 'just' import them? (I'm not at all familiar with OpenAPI so could be asking a very silly question :sweat_smile: ). No problem if not - just wanted to ask the question!

saswatamcode commented 2 years ago

This is a really good question! We can import schemas using $ref syntax. But currently, there doesn't seem to be any spec for Thanos or Prometheus yet. There are a couple of PRs that I found (https://github.com/thanos-io/thanos/pull/4393, https://github.com/prometheus/prometheus/pull/9197) but nothing in-tree yet, afaik.

But once these are merged, maybe we can utilize them! 🙂

bill3tt commented 2 years ago

Righto! Makes sense - thanks for checking :raised_hands:

philipgough commented 2 years ago

It might well be the case, but I am wondering if this repo is the correct place for the OpenAPI spec to live? 🤔

saswatamcode commented 2 years ago

Yeah, I was wondering about this too! I think it's being used here so makes sense for now, but this should ideally be in observatorium/api and we should just import that here. Maybe once this gets fully reviewed, it can be changed! 🙂

philipgough commented 2 years ago

@saswatamcode - yes that was what I was thinking initially. It seems like the natural fit.

saswatamcode commented 2 years ago

Discussed this with @onprem too, so will be moving the spec to the API repo and importing here! 🙂