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

Include a list of paths in the logging process #234

Closed
Nisreen123 opened this issue May 9, 2018 · 9 comments
Closed

Include a list of paths in the logging process #234

Nisreen123 opened this issue May 9, 2018 · 9 comments

Comments

@Nisreen123
Copy link

how can we add a condition to include a list of paths, i Know i can use
.condition(Conditions.requestTo("/pathOne"))
.condition(Conditions.requestTo("/pathTwo"))
.condition(Conditions.requestTo("/pathThree")), but is there any better approach?

@whiskeysierra
Copy link
Collaborator

Using condition(Predicate) multiple times will actually not work. The last one will win. From what I can gather I assume that you want a whitelist of paths for which you want to enable logging? There is indeed no support right now.

You can craft your own helper methods:

@SafeVarargs
public static <T extends BaseHttpMessage> Predicate<T> include(final Predicate<T>... predicates) {
    return include(Arrays.asList(predicates));
}

public static <T extends BaseHttpMessage> Predicate<T> include(final Collection<Predicate<T>> predicates) {
    return predicates.stream()
            .reduce(Predicate::or)
            .orElse($ -> false);
}

Which would allow you to write:

Logbook.builder()
        .condition(include(
            requestTo("/pathOne"),
            requestTo("/pathTwo"),
            requestTo("/pathThree")))
        .build();

Please not that the most important part of this the implementation of include which is essentially a generic implementation of or-ing multiple predicates, something that is unfortunately missing from the JDK.

As you can see, I'm making use of static imports which shorten it some more. If you're lucky and all of your paths actually share the same prefix you could use requestTo("/path*").

@whiskeysierra
Copy link
Collaborator

@whiskeysierra
Copy link
Collaborator

I can derive two potential changes for logbook from this:

  1. Supporting multiple conditions in the builder. Open question: How are they combined? or vs. and?
  2. Next to exclude support include as implemented above.

@lukasniemeier-zalando @AlexanderYastrebov What do you think?

@AlexanderYastrebov
Copy link
Member

Why

Logbook.builder()
        .condition(
                requestTo("/a")
                .or(requestTo("/b"))
                .or(requestTo("/c"))
                .or(contentType("whatever")))
        .build();

will not work for this case?

@whiskeysierra
Copy link
Collaborator

That would of course work. I always forget to use Predicate.or(..) directly.

@AlexanderYastrebov
Copy link
Member

Problem solved? :)

@whiskeysierra
Copy link
Collaborator

Problem solved? :)

@Nisreen123 Is it?

@Nisreen123
Copy link
Author

@whiskeysierra
Thanks for your answer;
Well, waiting for an answer, and inspired by the way, the exclude condition is implemented, i came with similar methods, but this way, every time a new path is added, the code needs to be updated, hence, i was thinking of making the list of paths, a dynamic one, i added an application property "logbook.included.paths=path1,path2" and injected it my configuration class using
@Value("#{'${logbook.included.paths:*}'.split(',')}") List<String> includedPaths;
but i don't know how can i build the condition from the list using the Predicate approach, what do you think?

@whiskeysierra
Copy link
Collaborator

@Nisreen123

.condition(include(includedPaths.stream().map(Conditions::requestTo).collect(toList())))

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants