feat(logbook-spring-boot-autoconfigure): header include/exclude filters

[p-8-z]

Added header filtering capability to logbook-spring-boot-autoconfigure module.

Description

Added header filtering capability to logbook-spring-boot-autoconfigure module for spring-boot autoconfiguration.
Example of usage:

logbook.predicate.exclude[0].path=/actuator/**
logbook.predicate.exclude[1].headers.host=localhost

This configuration will create predicate that will log requests that wont contain /actuator/** path in url or
header with key host and value localhost.

Motivation and Context

I'd like to filter out some request/response logs by header values.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have added tests to cover my changes.

[kasmarian]

Thank you for the PR, @p-8-z! I think overall the idea is very nice and would be a good extension to the Logbook.

[kasmarian]

This piece of logic is a bit counterintuitive to me. If someone specifies that they want to filter out all requests with a specific header they should be able to do so by just adding them to the exclude configuration. It negates the predicates already.
For addressing any value a similar approach as for urls could be used, where * would stand for any header value. E.g. if you need to include in logs only requests with a header XYZ regardless of its value, you'd put

logbook:
  predicate:
    include:
      - headers:
          XYZ: *

in the configuration.

[p-8-z]

I don't think you meant Regex pattern so I just hardcoded star character that checks key and ignores value.

if ("*".equals(value)){
    return header(key, $ -> true);
}

If this was not what you wanted just let me know.
Maybe this star use-case should be documented in README.md separately, what do you propose?

[kasmarian]

I think that would be fine to have it hardcoded, yes. We can extend it to regex in the future, not sure if this is really needed at this point.

Maybe this star use-case should be documented in README.md separately, what do you propose?

I think it does deserve an description, yes. Could you adjust the Configuration table to have this part explained? logbook.predicate.include and logbook.predicate.exclude properties descriptions should be updated to include the header predicates configuration.

[kasmarian]

Can't say I'm convinced that this separate method is needed to be honest. You can achieve the same result with header(final String key, final Predicate<String> predicate) method.

[p-8-z]

conditionalHeader method wraps withoutHeader and header logic, or it can be extracted outside, whatever you prefer just let me know.

[kasmarian]

With the change in 2530fa4 this method just uses header() with a predicate. I think it's better to have the logic in the autoconfiguration module directly, as it doesn't add anything new to the core module.

We could have a header(String key) method, that would tell if the header is present in the message regardless of the value. But I'm also fine with having this logic direct in the autoconfiguration module.

[kasmarian]

There are two tests org.zalando.logbook.autoconfigure.ExcludeTest and org.zalando.logbook.autoconfigure.IncludeTest in the logbook-spring-boot-autoconfigure module which verify that the predicates configuration works as expected. Could you also add the header use cases to it?

[p-8-z]

Sure thing, added.
If I forgot any use-case or broke you code styles just let me know, I'll fix it.

[kasmarian]

With the change in 2530fa4 this method just uses header() with a predicate. I think it's better to have the logic in the autoconfiguration module directly, as it doesn't add anything new to the core module.

We could have a header(String key) method, that would tell if the header is present in the message regardless of the value. But I'm also fine with having this logic direct in the autoconfiguration module.

[kasmarian]

Could you please unwrap the imports? We're trying to not use * in this project. In the other files as well.

[kasmarian]

It would be also good to use test application-exclude.yml and application-include.yml files for the tests instead of defining the predicate beans. This would test that the autoconfiguration logic that you adjusted convertToPredicate method works as expected. It will also work as a good example how this configuration can look like together with the rest of predicates.

The PathConfig was a way to test how custom predicates work in a combination with the ones from .yml ones.

[kasmarian]

Speaking about the test configuration in .yaml files. Would be nice to have examples of purely header predicates as well as the combination with other predicates.
E.g.

logbook:
  include:
  - /api/**
  predicate:
    include:
      - path: /another-api
      - path: /yet-another-api
        methods:
          - DELETE
      - methods:
          - PUT
      - path: /secured-api
        headers: 
           authorization: *
      - path: /only-gets-with-json
        method: GET
        headers: 
           accept: application/json

[kasmarian]

I think that would be fine to have it hardcoded, yes. We can extend it to regex in the future, not sure if this is really needed at this point.

Maybe this star use-case should be documented in README.md separately, what do you propose?

I think it does deserve an description, yes. Could you adjust the Configuration table to have this part explained? logbook.predicate.include and logbook.predicate.exclude properties descriptions should be updated to include the header predicates configuration.