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

Configuration of non application endpoints #13144

Closed
kenfinnigan opened this issue Nov 5, 2020 · 10 comments · Fixed by #13601
Closed

Configuration of non application endpoints #13144

kenfinnigan opened this issue Nov 5, 2020 · 10 comments · Fixed by #13601
Assignees
Labels
kind/enhancement New feature or request
Milestone

Comments

@kenfinnigan
Copy link
Member

kenfinnigan commented Nov 5, 2020

Description
Combines #7893 and #2450 to enable the full configuration of non-application endpoints such as /metrics and /health.

Implementation ideas
By default, all non-application endpoints will be placed under a /q URL path. Creating /q/health, etc.

There will be configuration to enable the removal of the prefix from the path, reverting to current behavior.

Another piece of configuration will be present to indicate the non-application endpoints should be available on a separate port to the main application endpoints. By default, they will be on the same port as the main application endpoints.

A possibility is having a BuildItem around the current one for Vert.x Routes that can include a flag as to whether it should be present on the non-application endpoint. Extensions can offer configuration to disable it from being included if they prefer it to be present with application endpoints.

@kenfinnigan kenfinnigan added the kind/enhancement New feature or request label Nov 5, 2020
@kenfinnigan kenfinnigan self-assigned this Nov 5, 2020
@stianst
Copy link
Contributor

stianst commented Nov 6, 2020

Would be good to not only have the option of different port, but also different binding.

@kenfinnigan
Copy link
Member Author

@stianst, can you clarify what you mean by different binding?

@ASzc
Copy link

ASzc commented Nov 6, 2020

IP address binding, I assume. Typical server binding is 0.0.0.0 i.e. anything, but it's possible to restrict that to just one address in Socket

@kenfinnigan
Copy link
Member Author

Ah ok, so restricting to local address only. Makes sense

@oscarfh
Copy link
Contributor

oscarfh commented Nov 30, 2020

@kenfinnigan I stumbled on this coming from #2450 and I was thinking about grabbing this one when I have some time.
My initial proposal (I am coming from that issues, where only a prefix is required) would be to have some BuildItem produced by the quarkus-core project, that could be observed by all the extensions and it would be the extension's responsibility to observe it and prepend the value in the path.

Probably adding the port and the binding would make it a little bit more complex, but I think that the same approach applies: Quarkus "natively" has this capability that,if not configured binds to the same IPs and ports as the application, but if configured generates a buildinfo telling all the extensions to bind to a given ip/port. Of course it would ensure that there is a server running on this ip/port.

What do you think about it? Do you think it is the correct approach? Does that make sense at all?

@kenfinnigan
Copy link
Member Author

I'd gone back and forth in figuring this out over the last week.

The solution I'm in the process of implementing is to modify the RouteBuildItem to have a flag indicating whether the endpoint is a "Framework endpoint" and should be treated differently, then modifying the extensions to set this flag.

That then enables the Vert.x processor to create a separate Router for all those endpoints, which can be mounted at the appropriate route, or added to an entirely different server if binding or port is different.

@kenfinnigan
Copy link
Member Author

The following endpoints will be accessible under the new root for non-application endpoints:

  • Health
  • Health UI
  • Metrics
  • Micrometer registries (if applicable)
  • OpenAPI

What I'd like some feedback on is whether Swagger UI should also be on that list or not? As it's only available in dev or test without explicitly configuring otherwise, I'm not sure whether it should be moved to the same root as the others.

@oscarfh
Copy link
Contributor

oscarfh commented Nov 30, 2020

I agree with this list, but I think that this is up to the extension developer to device. I would like to add my own extension (https://github.com/quarkiverse/quarkiverse-logging-ui) under this path.

@kenfinnigan
Copy link
Member Author

Didn't intend to imply it wouldn't, this is just an initial list of where I will be changing the extension to say "I'm a framework endpoint".

It won't preclude any extension from saying it is part of it as well

@kenfinnigan
Copy link
Member Author

Created #13602 as a separate issue for implementing the local binding and port options. Anyone else is welcome to take that on

kenfinnigan added a commit to kenfinnigan/quarkus that referenced this issue Dec 7, 2020
- Fixes quarkusio#13144
- OpenAPI, Metrics, Micrometer, Health, Health UI endpoints are all now served under /q by default
- Previous behavior can be attained by setting `quarkus.http.framework-root-path` to `/`

Subsequent issue will be created for handling port and binding changes for non application endpoints
kenfinnigan added a commit to kenfinnigan/quarkus that referenced this issue Dec 9, 2020
- Fixes quarkusio#13144
- OpenAPI, Metrics, Micrometer, Health, Health UI endpoints are all now served under /q by default
- Previous behavior can be attained by setting `quarkus.http.framework-root-path` to `/`

Subsequent issue will be created for handling port and binding changes for non application endpoints
@ghost ghost added this to the 1.11 - master milestone Dec 11, 2020
cemnura pushed a commit to cemnura/quarkus that referenced this issue Dec 22, 2020
- Fixes quarkusio#13144
- OpenAPI, Metrics, Micrometer, Health, Health UI endpoints are all now served under /q by default
- Previous behavior can be attained by setting `quarkus.http.framework-root-path` to `/`

Subsequent issue will be created for handling port and binding changes for non application endpoints
renegrob pushed a commit to renegrob/quarkus that referenced this issue Dec 23, 2020
- Fixes quarkusio#13144
- OpenAPI, Metrics, Micrometer, Health, Health UI endpoints are all now served under /q by default
- Previous behavior can be attained by setting `quarkus.http.framework-root-path` to `/`

Subsequent issue will be created for handling port and binding changes for non application endpoints
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants