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

ServiceControl APIs are a wild mix and do not follow sane HTTP API guidance #4160

Open
danielmarbach opened this issue May 14, 2024 · 3 comments

Comments

@danielmarbach
Copy link
Contributor

Describe the suggested improvement

ServiceControl has a wild mix of approaches when it comes to HttpAPIs which makes interacting with those APIs vary on a case by case basis. Some things we have observed:

  • ServiceControl uses combined ETAGs that are sometimes used and sometimes not. It is unclear how those ETAGs in the current form provide value (if at all)
  • ServiceControl used the HTTP ReasonPhrase which is no longer supported in HTTP2.0. To workaround, we had to introduce a dedicated X-PARTICULAR-REASONPHRASE header. Probably using something like problem details eventually would be a saner approach.
  • StatusCodes are messy and sometimes make little sense
  • There is no API documentation
  • Scatter/Gather requires deserialization and re-serialization which makes things horribly inefficient

It would be good to introduce OpenAPI specs and rework the APIs to follow sane HTTP guidelines

Additional Context

No response

@johnsimons
Copy link
Member

@danielmarbach regarding the new X header, that convention has been deprecated for a while, https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers

@johnsimons
Copy link
Member

Regarding the etags, if memory serves me and things have not changed, those were introduced by a much younger John 😜, the idea was to improve ServiceInsight performance and hence lean on etags to cache responses since those should be immutable.

@johnsimons
Copy link
Member

Regarding OpenApi, that is great if the rest api is to be consumed by external customers but if that is not the case, there is a “tax” involved in maintaining it.
If we are to enforce certain guidelines in our rest api for example, all POST requests must return 201, I wonder if Roslyn analysers could help us.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants