logbook icon indicating copy to clipboard operation
logbook copied to clipboard

Allow using multiple strategies for multiple set of paths

Open Marchosiax opened this issue 1 year ago • 3 comments

Detailed Description

Allowing multiple strategies to work alongside each other for different situations. For example one strategy to work for a set of paths, conditions and/or filters, while having another strategy for different paths and conditions.

Context

We have a requirement to log request and response for certain paths regardless of the status of the response. So I use include to log these paths by using the default strategy. However, there are some paths which doesn't need to be logged unless an error happens. In which case we need to see what was the request body or param that lead to this error.

I know status-at-least or body-only-if-status-at-least exist but there's no way to have both the default and one of the two mentioned strategies together. I tried to work this out with filters and other configs but had no success.

Your Environment

  • Version used: 3.9.0
  • Using logbook-spring-boot-webflux-autoconfigure with Spring WebFlux

Marchosiax avatar Jul 29 '24 11:07 Marchosiax

I don't think that needs to be part of the core itself. If I were you, I'd just implement my own strategy that internally keeps pairs of Predicates and strategies to delegate to. Within each method you find the relevant strategy to delegate to.

On Mon, Jul 29, 2024, 13:00 Peyman @.***> wrote:

Detailed Description

Allowing multiple strategies to work alongside each other for different situations. For example one strategy to work for a set of paths, conditions and/or filters or excluded, while also a different set of paths We have a requirement to log request and response for certain paths regardless of the status of the response. So I use include to log these paths by using the default strategy. However, there are some paths which doesn't need to be logged unless an error happens. In which case we need to see what was the request body or param that lead to this error.

I know status-at-least or body-only-if-status-at-least exist but there's no way to have both the default and one of the two mentioned strategies together. Context

I guess the Possible Implementation Your Environment

  • Version used:
  • Link to your project:

— Reply to this email directly, view it on GitHub https://github.com/zalando/logbook/issues/1875, or unsubscribe https://github.com/notifications/unsubscribe-auth/AADI7HO3EPS4OKQPLJ5JMHDZOYOEVAVCNFSM6AAAAABLUA5FOGVHI2DSMVQWIX3LMV43ASLTON2WKOZSGQZTKMJQG4YDKOI . You are receiving this because you are subscribed to this thread.Message ID: @.***>

whiskeysierra avatar Jul 29 '24 11:07 whiskeysierra

@whiskeysierra How would I approach your suggestion? Could you give me a rough example?

By the way I guess it doesn't have to be a new strategy. It could be a new kind of Predicate which accepts minimum status as parameter. Something like this:

  predicate:
    include:
      - path: /test/logged
        methods:
          - POST
        min-status: 400

Marchosiax avatar Jul 29 '24 14:07 Marchosiax

You should take a look at https://github.com/zalando/logbook?tab=readme-ov-file#strategy and https://github.com/zalando/logbook/tree/main?tab=readme-ov-file#spring-boot-starter. The idea is to implement a custom Strategy in code and when you register that as a Spring bean, it will be used, rather than the default one.

Your proposed snippet there wouldn't solve your problem. You'd need multiple predicates/strategies which aren't supported. Also, predicates only have access to the request which means you can't test on the response status. That's why strategy exists in the first place. Logbook's design goal is to solve the 80% case out of the box (spring boot starter + yaml config) and enable the other 20% to do it themselves (implement Strategy interface directly).

On Mon, Jul 29, 2024 at 4:37 PM Peyman @.***> wrote:

@whiskeysierra https://github.com/whiskeysierra How would I approach your suggestion? Could you give me a rough example?

By the way I guess it doesn't have to be a new strategy. It could be a new kind of Predicate which accepts minimum status as parameter. Something like this:

predicate: include: - path: /test/logged methods: - POST min-status: 400

— Reply to this email directly, view it on GitHub https://github.com/zalando/logbook/issues/1875#issuecomment-2256118970, or unsubscribe https://github.com/notifications/unsubscribe-auth/AADI7HMVVSSO3JUUJ5IVJVLZOZHQ5AVCNFSM6AAAAABLUA5FOGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDENJWGEYTQOJXGA . You are receiving this because you were mentioned.Message ID: @.***>

whiskeysierra avatar Jul 29 '24 22:07 whiskeysierra

In order to prioritize the support for Logbook, we would like to check whether the old issues are still relevant. This issue has not been updated for over six months.

  • Please check if it is still relevant in latest version of the Logbook.
  • If so, please add a descriptive comment to keep the issue open.
  • Otherwise, the issue will automatically be closed after a week.

github-actions[bot] avatar Jan 27 '25 01:01 github-actions[bot]

This issue has automatically been closed due to no activities. If the issue still exists in the latest version of the Logbook, please feel free to re-open it.

github-actions[bot] avatar Feb 10 '25 02:02 github-actions[bot]