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

Add health registry support #99

Closed
xstefank opened this issue May 26, 2020 · 9 comments · Fixed by #106
Closed

Add health registry support #99

xstefank opened this issue May 26, 2020 · 9 comments · Fixed by #106
Assignees
Milestone

Comments

@xstefank
Copy link
Member

API tracked in spec issue - microprofile/microprofile-health#247

@xstefank xstefank self-assigned this May 26, 2020
@kenfinnigan
Copy link
Contributor

Is exposing a registry the preferred option?

Would it expose the ability for user applications to retrieve health checks?

Would a builder API be more appropriate?

@xstefank
Copy link
Member Author

I have the code almost ready for injection similar to metric registry in metrics.

Users will not see all checks through the registry. Not required by the spec. However, in SR users can inject SRHealthRegistry to get all checks.

The idea is to add/remove checks dynamically so I don't see how builder would be appropriate. Can you give an example?

@kenfinnigan
Copy link
Contributor

What's the use case for being able to remove checks?

I think that's what I'm not grokking

@xstefank
Copy link
Member Author

Well, judging by the upstream issue, you can for instance exchange client protocol dynamically. Or something like I am dependent on service A when deployed but this is no longer true after A finishes its processing so it's unnecessary to continue checking its availability, even maybe dangerous if the service A is killed after its finished?

@kenfinnigan
Copy link
Contributor

TBH I don't understand the use case that is being described.

I think it needs to be a lot clearer as to the use case to determine an appropriate solution.

For instance, maybe the right approach is a separate API to "de-register" through CDI Events, or the like, without exposing the health checks as a registry

@kenfinnigan
Copy link
Contributor

The main use case appears to be multi-tenet usage, which I think opens a whole other set of issues with MP generally if it's "multi tenet"

@xstefank
Copy link
Member Author

xstefank commented Jun 8, 2020

@kenfinnigan however, the spec discussion isn't concluded in this sense. How should we proceed with this as an experimental API?

@kenfinnigan
Copy link
Contributor

I'm not sure there's a critical use case to be addressed at present, so I would be inclined to not do anything for it

@xstefank
Copy link
Member Author

xstefank commented Jun 8, 2020

but that doesn't solve the issue which is proposed in the specification. I would like to keep this open until the spec issue is resolved in one way or the other.

@xstefank xstefank added this to the 2.2.2 milestone Jul 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants