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

Add ability to ignore request path prefixes in Observability server filters #29210

Closed
bclozel opened this issue Sep 27, 2022 · 3 comments
Closed
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) status: declined A suggestion or change that we don't feel we should currently apply theme: observability An issue related to observability and tracing type: enhancement A general enhancement

Comments

@bclozel
Copy link
Member

bclozel commented Sep 27, 2022

Both HttpRequestsObservationFilter and HttpRequestsObservationWebFilter record observations for Spring's web frameworks. It would be generally useful to provide an option to ignore some path prefixes entirely from instrumentation - for example, the Spring Boot Actuator endpoint should not be instrumented in an application.

@bclozel bclozel added in: web Issues in web modules (web, webmvc, webflux, websocket) type: enhancement A general enhancement theme: observability An issue related to observability and tracing labels Sep 27, 2022
@bclozel bclozel added this to the 6.0.0-RC1 milestone Sep 27, 2022
@bclozel bclozel self-assigned this Sep 27, 2022
@bclozel bclozel changed the title Add ability to ignore prefixes in Observability server filters Add ability to ignore request path prefixes in Observability server filters Sep 27, 2022
@bclozel bclozel modified the milestones: 6.0.0-RC1, 6.0.x Oct 4, 2022
@marcingrzejszczak
Copy link
Contributor

I'm wondering if it wouldn't be better to opt in for paths instead of opt out of them. The log in Sleuth is really complicated to automatically skip certain patterns. Maybe it would be easier to just say what you want to observe instead of what you don't want to observe?

@jonatan-ivanov
Copy link
Member

I've seen lots of teams building apps that has lot of "rpc-like" endpoints: every operation is a different endpoint which is named after what it does.
In these situations, I think I would say opting-out is better. Also maybe we should opt-out actuator endpoints by default (something like this).

I guess the most important thing though is to make the path, host, method, etc. easily available through common getters from the context.

@bclozel
Copy link
Member Author

bclozel commented Apr 24, 2023

After considering different options here, it seems that there is no simple approach for filtering requests. Path prefixes might work for some cases, but the two comments above seem to show that the use cases can also be much more complex. This would also introduce a filtering mechanism that would cause a performance impact in all cases, even if not needed.

Additionally, "skipping" the observation here in some cases could confuse other instrumentations down the line. We could of course turn observations into "no-ops". At this stage, the ObservationPredicate is a much better place for that since it's got the entire observation context for matching and has the power to turn the observation into "no-ops".

As a result, I'm declining this change in favor of micrometer-metrics/micrometer#3678 (that should allow to "no-op" observations and child observations) and spring-projects/spring-boot#34801 (which should drive the entire feature through predicates).

@bclozel bclozel closed this as not planned Won't fix, can't repro, duplicate, stale Apr 24, 2023
@bclozel bclozel removed their assignment Apr 24, 2023
@bclozel bclozel added the status: declined A suggestion or change that we don't feel we should currently apply label Apr 24, 2023
@bclozel bclozel removed this from the 6.1.x milestone Apr 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) status: declined A suggestion or change that we don't feel we should currently apply theme: observability An issue related to observability and tracing type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

3 participants