-
Notifications
You must be signed in to change notification settings - Fork 6k
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 request to Observation Context at creation to enable filtering by the request #12854
Comments
Thanks for the suggestion, @jonatan-ivanov. Can you elaborate on how the request details would be read when evaluating the Based on spring-projects/spring-boot#34400 I wonder if it should add |
It can be an uri field on the context, a tag or for full flexibility, does the whole request make sense? I think something like this: @Bean
ObservationPredicate actuatorServerContextPredicate() {
return (name, context) -> {
if (name.startsWith("spring.security.") && context instanceof SecurityObservationContext secObsContext) {
return !secObsContext.getCarrier().getRequestURI().startsWith("/actuator");
}
return true;
};
} But as an uri field this would be quite similar: Here is a working example excluding the actuator http spans. |
In that case, would someone that is trying to turn off spans by request need to know how specifically how to turn off each project's spans? I'm wondering, for example, if Perhaps I'm not seeing the whole picture, but from where I'm standing, I'm seeing an application that needs to know Spring Security specifics -- as well as the specifics of other modules -- to perform an observability-level task of filtering out spans from a request. |
That's the exact problem I have with this solution. And once that this change in micrometer will also not solve: micrometer-metrics/micrometer#3678 It seems like the only way to do this properly is as mentioned on the original issue, by adding an attribute on every span you have in order to ignore them with a predicate. (From Jonathan on the original issue) // This adds the http.url keyvalue to security observations from the root (mvc) observation
// You add an ignoreSpan=true keyValue instead if you want, or something that can signal to the SpanExportingPredicate what to ignore
@Bean
ObservationFilter urlObservationFilter() {
return context -> {
if (context.getName().startsWith("spring.security.")) {
Context root = getRoot(context);
if (root.getName().equals("http.server.requests")) {
context.addHighCardinalityKeyValue(root.getHighCardinalityKeyValue("http.url"));
}
}
return context;
};
}
private Observation.Context getRoot(Observation.Context context) {
if (context.getParentObservation() == null) {
return context;
}
else {
return getRoot((Context) context.getParentObservation().getContextView());
}
}
// This ignores actuator spans but its logic depends on the ObservationFilter above
// Auto-configuration for SpanExportingPredicate was added in 3.1.0-M1
// So either you use 3.1.x or you can add the same to your config : https://github.com/spring-projects/spring-boot/pull/34002
@Bean
SpanExportingPredicate actuatorSpanExportingPredicate() {
return span -> !span.getTags().get("http.url").startsWith("/actuator");
} |
Filtering out spans from anywhere in the flow can be a valid use-case I think (this is about Observations but that does not change much), I think saying that http Observations are off for actuator but Observation XYZ that is triggered by that HTTP call is not is a completely valid use-case though I feel not as frequent. |
Yes, that's the case I'm referring to. I imagine something like the following:
Or, in pseudocode, something like this: ObservationPredicate isNotActuator = (name, context) -> {
RequestAwareObservationContext requestAware = context.getFirstAncestorOfType(RequestAwareObservationContext.class);
return requestAware == null || !requestAware.getCarrier().getRequestURI().endsWith("/actuator");
}; I'm not sure what other complexities I am ignoring or unaware of, so please take the code as just an observation. Regardless of the best way to solve this, I think it would be better to expose the request in a non-Spring-Security-specific way. |
@braunsonm Could you please try out one thing? @Bean
ObservationPredicate noRootlessHttpObservations() {
return (name, context) -> {
RequestAttributes requestAttributes = RequestContextHolder.getRequestAttributes();
if (requestAttributes instanceof ServletRequestAttributes servletRequestAttributes) {
Observation observation = (Observation) requestAttributes.getAttribute(ServerHttpObservationFilter.class.getName() + ".observation", SCOPE_REQUEST);
return observation == null || !observation.isNoop();
}
return true;
};
} |
@jonatan-ivanov Appears to work! Here is a crude example of using it for the original use case of ignoring actuator calls @Bean
ObservationPredicate noRootlessHttpObservations() {
return (name, context) -> {
RequestAttributes requestAttributes = RequestContextHolder.getRequestAttributes();
if (requestAttributes instanceof ServletRequestAttributes servletRequestAttributes) {
Observation observation = (Observation) requestAttributes.getAttribute(ServerHttpObservationFilter.class.getName() + ".observation", SCOPE_REQUEST);
return (!servletRequestAttributes.getRequest().getRequestURI().startsWith("/actuator")) || (observation == null || !observation.isNoop());
}
// The root observation is not stored in the `RequestContextHolder` in this stage and needs to be handled separately
if (context instanceof ServerRequestObservationContext serverContext) {
return (!serverContext.getCarrier().getRequestURI().startsWith("/actuator"));
}
return true;
};
} Unfortunately I don't think this solution is portable to Webflux though. I think you'd have to use |
It seems that provided code doesn't work if management context is a child context (when it is running on separate port). There are 2 issues:
From what I was able to find, when management server is started as child context, it has slightly separate configuration, that doesn't register Child Context configuration: As a workaround I've added own configuration that adds required filter and registers this configuration in |
Are you doing any kind of filtering to disable this? |
No, not doing any filtering. As self-check I've simply restarted app with management port set equal to main port and span was created as expected. Should I create ticket for spring-framework or spring-boot? It looks like specific to actuator. |
Framework has the instrumentation, I would start there but now that you mentioned that you are setting the management port, I think this might be a known issue but I don't find it. |
This issue was originally discussed in spring-projects/spring-boot#34400.
The scenario is the following: if Spring Security is used and users want to ignore
/actuator
endpoints, there is no easy way to ignoreObservation
s created by Spring Security because the details to make this decision are missing when theObservationPredicate
is tested.One potential solution would be adding the request to the
Context
before the Observation is created, one example here:spring-security/web/src/main/java/org/springframework/security/web/ObservationFilterChainDecorator.java
Lines 195 to 202 in ac1d269
Would be something like:
Expected behavior: having extra details on the Observations created by Spring Security that allows the users to ignore these Observation based on the request.
There is a reproducer in the issue mentioned above.
The text was updated successfully, but these errors were encountered: