-
Notifications
You must be signed in to change notification settings - Fork 12
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
Added Spring Boot Actuator and metrics endpoints #1673
Conversation
I am thinking that perhaps the default port for this interface should be different from the default port that is used for the FHIR interface. This is because usually you would want a different level of visibility of this service, e.g. it might only need to be accessible by a monitoring component within the cluster, not exposed externally. |
I've made this change in 280fef1, let me know what you think. |
That's definitely a great change - I've defaulted to the same behavior in the HAPI FHIR Helm chart as well: https://github.com/hapifhir/hapi-fhir-jpaserver-starter/blob/master/charts/hapi-fhir-jpaserver/templates/deployment.yaml#L98. One more thing (we can of course always override any of these properties at runtime), but what do you think about the |
The probes could be on the management port as well right? I'm not aware of any requirement to have the probes available on the same port as the application. I would imagine that something like this in the container spec would work well: startupProbe:
httpGet:
path: /actuator/health/liveness
port: 8081
periodSeconds: 5
failureThreshold: 36
livenessProbe:
httpGet:
path: /actuator/health/liveness
port: 8081
periodSeconds: 5
failureThreshold: 12
readinessProbe:
httpGet:
path: /actuator/health/readiness
port: 8081
periodSeconds: 5
failureThreshold: 3 |
Absolutely. the given rationale for using the same port as the main application:
|
Ok, that makes sense. I'm not worried about the FHIR thing, as this would be outside the FHIR endpoint anyway ( Would this be exposing only the liveness and readiness endpoints, or also the Prometheus endpoint? Are there any security implications to having this accessible externally? |
The Prometheus endpoint will only be exposed on port 8081 at /actuator/prometheus. It's only the health probes that will be available on :8080 as well. There's some docs on security: https://docs.spring.io/spring-boot/docs/current/reference/html/actuator.html#actuator.endpoints.security . Since we explicitly expose only health and metrics endpoints there shouldn't be anything sensitive in there. Edit: for completeness, here's the response from the /actuator/prometheus endpoint. I POSTed some resources and the queried them using
|
Thanks for this! I think we should go ahead and change the default configuration as you suggest, to expose the health endpoints on 8080 and the other actuator information on 8081. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this looks good, and we can roll it in to the next release.
Thanks @chgl!
@chgl Just letting you know that this change is now in v6.4.0 - check it out and let us know what you think. |
@johngrimes , awesome, thanks! Works great! I've already updated my helm chart to use the new endpoints for liveness/readiness and metrics. |
Closes #1671 .
This adds the spring-boot-starter-actuator and micrometer-registry-prometheus dependencies, which expose health and metrics endpoints.
In the default configuration, health-endpoints are exposed under
:8080/actuator/health
and metrics at ``:8080/actuator/prometheus`.