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

Provide a way to make decisions based on the parent in ObservationPredicate #3678

Closed
jonatan-ivanov opened this issue Mar 8, 2023 · 3 comments · Fixed by #3867
Closed

Provide a way to make decisions based on the parent in ObservationPredicate #3678

jonatan-ivanov opened this issue Mar 8, 2023 · 3 comments · Fixed by #3867

Comments

@jonatan-ivanov
Copy link
Member

jonatan-ivanov commented Mar 8, 2023

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 do it today because Spring Security does not add details about the request. One potential solution would be checking the parent (root) Observation but it is not available through the context when the predicate is tested.

There is a reproducer in the issue mentioned above.

Also see: spring-projects/spring-security#12854

@jonatan-ivanov jonatan-ivanov added enhancement A general enhancement internal An issue that needs input from a member on another Team labels Mar 8, 2023
@braunsonm
Copy link

Thanks for opening this @jonatan-ivanov
Can you clarify if the intention of this issue is to also have the parent set if the parent was ignored in a previous predicate?

ie:

@Configuration
public class TestConfiguration {
    @Bean
    ObservationPredicate ignoreActuator() {
        return (s, context) -> ignoreActuatorRecursive(context);
    };

    public boolean ignoreActuatorRecursive(Observation.ContextView context) {
        if (context instanceof ServerRequestObservationContext serverRequestObservationContext) {
            HttpServletRequest carrier = serverRequestObservationContext.getCarrier();
            return !carrier.getServletPath().startsWith("/actuator"); // <----- Line (*)
        }

        if (context.getParentObservation() != null) {  // <-- Will the parent still be set if it was ignored in line (*)?
            return ignoreActuatorRecursive(context.getParentObservation().getContextView());
        }

        return true;
    }
}

It would be great for the parent to remain set even if the parent observation was ignored in a previous predicate. The reason being so that you can properly ignore any subsequent observations deriving from a root span that was ignored.

@jonatan-ivanov
Copy link
Member Author

jonatan-ivanov commented Mar 9, 2023

No, and there the parent can't be really set in these cases. If an Observation is ignored we will create a noop one that is hopefully eliminated by JIT.

@quaff
Copy link
Contributor

quaff commented May 31, 2023

@braunsonm context.getParentObservation() will always be null in ObservationPredicate, I'm trying to fix it by #3867.

@shakuzen shakuzen added module: micrometer-observation and removed internal An issue that needs input from a member on another Team labels Aug 16, 2023
@shakuzen shakuzen added this to the 1.12.0-M3 milestone Aug 16, 2023
shakuzen pushed a commit that referenced this issue Aug 16, 2023
#3867)

Before these changes, the parent is not available in the context when the ObservationPredicate instances are called, which means decisions cannot be made based on the parent.

Resolves gh-3678
@shakuzen shakuzen changed the title Provide a simple way to make decisions based on the parent in ObservationPredicate Provide a way to make decisions based on the parent in ObservationPredicate Aug 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants