Expose Thanos HTTP APIs to OpenAPI/protobuf Proposal #4426

Hangzhi · 2021-07-08T08:24:52Z

Signed-off-by: Hangzhi [email protected]

Related to : #4102

I added CHANGELOG entry for this change.
Change is not relevant to the end user.

Changes

Verification

Signed-off-by: Hangzhi <[email protected]>

bwplotka

Nice, great work! I love it, LGTM 💪🏽

bwplotka · 2021-07-08T10:27:47Z

docs/proposals-accepted/202107-protobuf-openapi-httpapi.md

@@ -0,0 +1,73 @@
+| type     | title                                                        | status   | menu               |


I think we use different format of front matter - hopefully we can get rid of this too, but right now it's required to be yaml between --- letters.

bwplotka · 2021-07-08T10:28:36Z

docs/proposals-accepted/202107-protobuf-openapi-httpapi.md

+
+## **Motivations**
+
+In order to improve Thanos usage for users, we would like to define our HTTP APIs in protobuf/OpenAPI and expose those in the website. This would allow users to use tools for documentation, validation, type checking, and even code generation to use our APIs efficiently.


Suggested change

In order to improve Thanos usage for users, we would like to define our HTTP APIs in protobuf/OpenAPI and expose those in the website. This would allow users to use tools for documentation, validation, type checking, and even code generation to use our APIs efficiently.

In order to improve Thanos usage for users, we would like to define our HTTP APIs in protobuf/OpenAPI and expose those in our repository. This would allow users to use tools for documentation, validation, type checking, and even code generation to use our APIs efficiently.

Similarly we want reuse this work in Prometheus.

For readability it would be good to expand on the following parts:

In order to improve Thanos usage for users - what do we mean by improve here? What positive benefit will this bring to users?

protobuf/OpenAPI - as far as I am aware these are two different & non-interchangeable things?

bwplotka · 2021-07-08T10:39:51Z

docs/proposals-accepted/202107-protobuf-openapi-httpapi.md

+
+### **Pitfalls of the current solution**
+
+- Documentation needs to be written manually.


Suggested change

- Documentation needs to be written manually.

- Documentation, Server code and client code boilerplate needs to be written manually.

- It's hard to discover the current API programmatically.

- It's easy to make an accidental breaking changes when modifying API

I would reword the final point to be something like

When modifying the API, it is very difficult to know if this breaks downstream users

Or similar :)

bwplotka · 2021-07-08T10:41:06Z

docs/proposals-accepted/202107-protobuf-openapi-httpapi.md

+- Be able to generate OpenAPI3 from protobuf.
+- Auto-generate documentation with OpenAPI3 specification.
+- Generate server code from API specification (OpenAPI3 or protobuf).
+- Define all configuration potentially in protobuf too: https://github.com/openproto/protoconfig.


Maybe we can drop that to focus more on APIs in this phase (:

bwplotka · 2021-07-08T10:43:46Z

docs/proposals-accepted/202107-protobuf-openapi-httpapi.md

+
+## **Non-Goals**
+
+- Not define Thanos gRPC APIs in protobuf specification.


Suggested change

- Not define Thanos gRPC APIs in protobuf specification.

- Don't mix gRPC with HTTP APIs in same protobuf package

bwplotka · 2021-07-08T10:43:56Z

docs/proposals-accepted/202107-protobuf-openapi-httpapi.md

+
+## **How**
+
+- Define Thanos APIs in protobuf specification.


Suggested change

- Define Thanos APIs in protobuf specification.

- Define Thanos HTTP APIs in protobuf specification.

bwplotka · 2021-07-08T10:44:25Z

docs/proposals-accepted/202107-protobuf-openapi-httpapi.md

+- Define Thanos APIs in protobuf specification.
+- Generate OpenAPI from protobuf with gnostic extension.
+- Generate documentation from OpenAPI with [swagger](https://github.com/swagger-api/swagger-codegen).
+- Generate server API client and server stubs potentially from OpenAPI with [swagger](https://github.com/swagger-api/swagger-codegen) or [oapi-codegen](https://github.com/deepmap/oapi-codegen).


Suggested change

- Generate server API client and server stubs potentially from OpenAPI with [swagger](https://github.com/swagger-api/swagger-codegen) or [oapi-codegen](https://github.com/deepmap/oapi-codegen).

- Generate server and client API stubs potentially from OpenAPI with [swagger](https://github.com/swagger-api/swagger-codegen) or [oapi-codegen](https://github.com/deepmap/oapi-codegen).

I'm not a big fan of having 'potentially' in proposals - we're either going to do it or not :)

bwplotka · 2021-07-08T10:44:44Z

docs/proposals-accepted/202107-protobuf-openapi-httpapi.md

+
+## **Alternatives**
+
+## **Only define HTTP RESTful API only in OpenAPI. Not in protobuf.**


Suggested change

## **Only define HTTP RESTful API only in OpenAPI. Not in protobuf.**

## **Define HTTP RESTful API only in OpenAPI. Not in protobuf.**

bwplotka · 2021-07-08T10:46:44Z

docs/proposals-accepted/202107-protobuf-openapi-httpapi.md

+## **Only define HTTP RESTful API only in OpenAPI. Not in protobuf.**
+
+1. Pros:
+   1. We are not sure whether we can definitely generate OpenAPI specifications from protobuf.


There are might be some complexity, edge case and extra tooling to make 3 step process (proto -> open API -> documentation to work

bwplotka · 2021-07-08T10:49:44Z

docs/proposals-accepted/202107-protobuf-openapi-httpapi.md

+   1. We can have gRPC and RESTful APIs at the same time.
+   2. We have gRPC APIs like rules API.
+2. Cons:
+   1. We don't know clearly how grpc-gateway works.


We need run another sidecar (complexity of running the system)

Semantics of gRPC and HTTP might be different and surprising for end user.

We want to reuse in Prometheus and Prometheus does not support gRPC (gRPC dependency was removed from codebase)

Same port library is not maintained. (cmux)

bill3tt · 2021-07-08T12:06:01Z

@Hangzhi make check-docs to see what is causing CI to fail.

bill3tt

Looks good - the content is good, but I think our motivations can be better explained.

bill3tt · 2021-07-09T13:23:21Z

docs/proposals-accepted/202107-protobuf-openapi-httpapi.md

+
+## **Motivations**
+
+In order to improve Thanos usage for users, we would like to define our HTTP APIs in protobuf/OpenAPI and expose those in the website. This would allow users to use tools for documentation, validation, type checking, and even code generation to use our APIs efficiently.


For readability it would be good to expand on the following parts:

In order to improve Thanos usage for users - what do we mean by improve here? What positive benefit will this bring to users?

protobuf/OpenAPI - as far as I am aware these are two different & non-interchangeable things?

bill3tt · 2021-07-09T13:26:32Z

docs/proposals-accepted/202107-protobuf-openapi-httpapi.md

+
+### **Pitfalls of the current solution**
+
+- Documentation needs to be written manually.


I would reword the final point to be something like

When modifying the API, it is very difficult to know if this breaks downstream users

Or similar :)

bill3tt · 2021-07-09T13:34:37Z

docs/proposals-accepted/202107-protobuf-openapi-httpapi.md

+- Define all APIs in protobuf.
+- Be able to generate OpenAPI3 from protobuf.
+- Auto-generate documentation with OpenAPI3 specification.
+- Generate server code from API specification (OpenAPI3 or protobuf).
+- Define all configuration potentially in protobuf too: https://github.com/openproto/protoconfig.


IMO these are not goals per-se but the steps we take to satisfy the goals - in fact, they are very similar to the How section below.

I think the goals here is to make the Thanos APIs easier to share, document and consume?

@ianbillett Thank you for your great reviews. Yes, make the Thanos APIs easier to share, document and consume seems like a better phrase. But in LFX mentorship program, I think we need a clear goal to decide whether this project is accepted. This goal seems to be vague and hard to quantify. 😃

bill3tt · 2021-07-09T13:36:06Z

docs/proposals-accepted/202107-protobuf-openapi-httpapi.md

+- Define Thanos APIs in protobuf specification.
+- Generate OpenAPI from protobuf with gnostic extension.
+- Generate documentation from OpenAPI with [swagger](https://github.com/swagger-api/swagger-codegen).
+- Generate server API client and server stubs potentially from OpenAPI with [swagger](https://github.com/swagger-api/swagger-codegen) or [oapi-codegen](https://github.com/deepmap/oapi-codegen).


I'm not a big fan of having 'potentially' in proposals - we're either going to do it or not :)

Signed-off-by: Hangzhi <[email protected]>

Hangzhi · 2021-07-22T02:47:13Z

Hello @ianbillett. The test shows that the file is not formatted, but it's not clear how not formatted. And it results in errors in the e2e test, though this PR should not involve the e2e test 🤔 . Do you have any hints on how I can resolve these errors?

onprem

Looking good to me, just some small non-blocker nits.

docs/proposals-accepted/202107-protobuf-openapi-httpapi.md

bill3tt · 2021-07-27T17:03:57Z

@Hangzhi

Since you are not modifying the e2e tests we can safely conclude that failed due to a transient flake. You can trigger a re-test with an empty commit git commit --allow-empty -m 'retest'.

If you inspect the output of the failing test it shows you why the linter is failing. If you inspect the makefile that is running the tests, you will find the binary we run (mdox) which you can run locally to fix the issues. The command will be something like mdox fmt -l --links.validate.config-file=./.mdox.validate.yaml docs/proposals/20201019-prometheus-rules-in-observatorium-api.md

Signed-off-by: Hangzhi <[email protected]>

…proposal

Signed-off-by: Hangzhi <[email protected]>

Hangzhi · 2021-07-28T12:51:30Z

@ianbillett Thank you for your very helpful information! I finally got the error info in detail with mdox fmt -l docs/proposals-accepted/202107-protobuf-openapi-httpapi.md.

bwplotka

Amazing work!

Let's merge it, it has all details we want, problem and goals outlined well 💪🏽

On our 1:2 we acknowledged that this proposal has some risk - there are things we need to experiment with. This is fine though -we can iterate on it. Edit the "HOW" when we find new things or reject it at then end if needed. But current version shows the latest direction.

openapi-protobuf-proposal

101647d

Signed-off-by: Hangzhi <[email protected]>

bwplotka previously approved these changes Jul 8, 2021

View reviewed changes

bill3tt reviewed Jul 9, 2021

View reviewed changes

Hangzhi added 3 commits July 20, 2021 19:21

revision of proposal

e0ddb6c

Signed-off-by: Hangzhi <[email protected]>

test make check docs

f3644c8

Signed-off-by: Hangzhi <[email protected]>

small nit

b4ce47b

Signed-off-by: Hangzhi <[email protected]>

Hangzhi dismissed bwplotka’s stale review via e0ddb6c July 21, 2021 09:41

Hangzhi force-pushed the protobuf-restulpai-proposal branch from 574ff45 to b4ce47b Compare July 21, 2021 14:02

onprem previously approved these changes Jul 22, 2021

View reviewed changes

docs/proposals-accepted/202107-protobuf-openapi-httpapi.md Outdated Show resolved Hide resolved

docs/proposals-accepted/202107-protobuf-openapi-httpapi.md Outdated Show resolved Hide resolved

docs/proposals-accepted/202107-protobuf-openapi-httpapi.md Outdated Show resolved Hide resolved

small nit

5127aa3

Signed-off-by: Hangzhi <[email protected]>

Hangzhi dismissed onprem’s stale review via 5127aa3 July 28, 2021 07:47

Hangzhi added 2 commits July 28, 2021 16:56

Merge remote-tracking branch 'upstream/main' into protobuf-restulpai-…

60e4565

…proposal

small nit for mdox

53dc303

Signed-off-by: Hangzhi <[email protected]>

bwplotka approved these changes Jul 29, 2021

View reviewed changes

bwplotka merged commit b7735a6 into thanos-io:main Jul 29, 2021

Hangzhi mentioned this pull request Aug 12, 2021

Proposal iteration - HTTP APIs to OpenAPI/protobuf Proposal #4561

Closed

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Expose Thanos HTTP APIs to OpenAPI/protobuf Proposal #4426

Expose Thanos HTTP APIs to OpenAPI/protobuf Proposal #4426

Hangzhi commented Jul 8, 2021 •

edited

Loading

bwplotka left a comment

bwplotka Jul 8, 2021

bwplotka Jul 8, 2021

bill3tt Jul 9, 2021

bwplotka Jul 8, 2021

bill3tt Jul 9, 2021

bwplotka Jul 8, 2021

bwplotka Jul 8, 2021

bwplotka Jul 8, 2021

bwplotka Jul 8, 2021

bill3tt Jul 9, 2021

bwplotka Jul 8, 2021

bwplotka Jul 8, 2021

bwplotka Jul 8, 2021

bill3tt commented Jul 8, 2021

bill3tt left a comment

bill3tt Jul 9, 2021

bill3tt Jul 9, 2021

bill3tt Jul 9, 2021

Hangzhi Jul 21, 2021

bill3tt Jul 9, 2021

Hangzhi commented Jul 22, 2021

onprem left a comment

bill3tt commented Jul 27, 2021

Hangzhi commented Jul 28, 2021

bwplotka left a comment


		## Motivations

		In order to improve Thanos usage for users, we would like to define our HTTP APIs in protobuf/OpenAPI and expose those in the website. This would allow users to use tools for documentation, validation, type checking, and even code generation to use our APIs efficiently.

-In order to improve Thanos usage for users, we would like to define our HTTP APIs in protobuf/OpenAPI and expose those in the website. This would allow users to use tools for documentation, validation, type checking, and even code generation to use our APIs efficiently.
+In order to improve Thanos usage for users, we would like to define our HTTP APIs in protobuf/OpenAPI and expose those in our repository. This would allow users to use tools for documentation, validation, type checking, and even code generation to use our APIs efficiently.
+Similarly we want reuse this work in Prometheus.


		### Pitfalls of the current solution

		- Documentation needs to be written manually.

-- Documentation needs to be written manually.
+- Documentation, Server code and client code boilerplate needs to be written manually.
+- It's hard to discover the current API programmatically.
+- It's easy to make an accidental breaking changes when modifying API


		## Non-Goals

		- Not define Thanos gRPC APIs in protobuf specification.

	- Not define Thanos gRPC APIs in protobuf specification.
	- Don't mix gRPC with HTTP APIs in same protobuf package

	- Define Thanos APIs in protobuf specification.
	- Define Thanos HTTP APIs in protobuf specification.

	- Generate server API client and server stubs potentially from OpenAPI with [swagger](https://github.com/swagger-api/swagger-codegen) or [oapi-codegen](https://github.com/deepmap/oapi-codegen).
	- Generate server and client API stubs potentially from OpenAPI with [swagger](https://github.com/swagger-api/swagger-codegen) or [oapi-codegen](https://github.com/deepmap/oapi-codegen).


		## Alternatives

		## Only define HTTP RESTful API only in OpenAPI. Not in protobuf.

	## Only define HTTP RESTful API only in OpenAPI. Not in protobuf.
	## Define HTTP RESTful API only in OpenAPI. Not in protobuf.

Expose Thanos HTTP APIs to OpenAPI/protobuf Proposal #4426

Expose Thanos HTTP APIs to OpenAPI/protobuf Proposal #4426

Conversation

Hangzhi commented Jul 8, 2021 • edited Loading

Changes

Verification

bwplotka left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bill3tt commented Jul 8, 2021

bill3tt left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Hangzhi commented Jul 22, 2021

onprem left a comment

Choose a reason for hiding this comment

bill3tt commented Jul 27, 2021

Hangzhi commented Jul 28, 2021

bwplotka left a comment

Choose a reason for hiding this comment

Hangzhi commented Jul 8, 2021 •

edited

Loading