-
Notifications
You must be signed in to change notification settings - Fork 40.7k
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
Disable actuator http observations #36455
base: main
Are you sure you want to change the base?
Disable actuator http observations #36455
Conversation
cfb70fb
to
f37c5f0
Compare
...actuate/autoconfigure/observation/web/reactive/WebFluxObservationAutoConfigurationTests.java
Outdated
Show resolved
Hide resolved
@@ -31,3 +31,11 @@ management.endpoints.migrate-legacy-ids=true | |||
|
|||
management.endpoints.jackson.isolated-object-mapper=true | |||
spring.jackson.visibility.field=any | |||
|
|||
#management.tracing.sampling.probability=1.0 |
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 left these changes in samples intentionally in the PR (it's a separate commit), one the PR is ready to merge I will drop it.
I flagged this for team-meeting, as we need to discuss the following:
|
6dd92ed
to
9563ba7
Compare
9563ba7
to
f79d931
Compare
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 tried to add the code from ActuatorWebEndpointObservationConfiguration
to one of my projects (Spring Boot 3.1.2 based) and I was greeted by a bean dependency cycle:
webMvcObservationFilter defined in class path resource [org/springframework/boot/actuate/autoconfigure/observation/web/servlet/WebMvcObservationAutoConfiguration.class]
┌─────┐
| observationRegistry defined in class path resource [org/springframework/boot/actuate/autoconfigure/observation/ObservationAutoConfiguration.class]
↑ ↓
| actuatorWebEndpointObservationPredicate defined in class path resource [.../infrastructure/observation/ObservationConfiguration.class]
↑ ↓
| pathMappedEndpoints defined in class path resource [org/springframework/boot/actuate/autoconfigure/endpoint/web/WebEndpointAutoConfiguration.class]
↑ ↓
| quartzEndpoint defined in class path resource [org/springframework/boot/actuate/autoconfigure/quartz/QuartzEndpointAutoConfiguration.class]
↑ ↓
| liquibase defined in class path resource [org/springframework/boot/autoconfigure/liquibase/LiquibaseAutoConfiguration$LiquibaseConfiguration.class]
└─────┘
Not sure if the same happens with the current main
and related code being a part of auto-configuration but just a hint to look into it.
WebMvcProperties webMvcProperties, PathMappedEndpoints pathMappedEndpoints) { | ||
return (name, context) -> { | ||
if (context instanceof ServerRequestObservationContext serverContext) { | ||
String endpointPath = getEndpointPath(serverProperties, webMvcProperties, pathMappedEndpoints); |
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.
Since endpoint path doesn't change in runtime, I think it doesn't need to be resolved on each invocation of the predicate but rather only once when the predicate is set up.
PathMappedEndpoints pathMappedEndpoints) { | ||
return (name, context) -> { | ||
if (context instanceof ServerRequestObservationContext serverContext) { | ||
String endpointPath = getEndpointPath(webFluxProperties, pathMappedEndpoints); |
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.
Same as above.
Will that be part of the next 3.2.x release? |
I doubt it. There are technical subtleties we need to solve before this lands in a Spring Boot release. But hey, never say never. |
We're not happy with all the string concatenation and matching going on here. A better way to solve this would be to cancel the observation when an actuator endpoint has been hit. AFAIK there's no way to do that right now, so I've opened micrometer-metrics/micrometer#4061 to discuss this possibility with the Micrometer team. Let's see what they think about that idea. |
See: #34801