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

DefaultJerseyTagsProvider clashes with WebMvcObservationAutoConfiguration #39294

Closed
bergerdenes opened this issue Jan 24, 2024 · 13 comments
Closed
Assignees
Labels
status: invalid An issue that we don't feel is valid

Comments

@bergerdenes
Copy link

When using Jersey (not Spring) annotated endpoints and also using Micrometer Prometheus integration, the "prometheus" actuator endpoint shows "UNKNOWN" in the uri instead of the path pattern.

ServerHttpObservationFilter does not fill pathPattern.
In the org.springframework.http.server.observation.DefaultServerRequestObservationConvention class, the protected KeyValue uri(ServerRequestObservationContext context) method tries to get context.getPathPattern() but it is always null.

Reproduction sample project with instructions: https://github.com/bergerdenes/metrics-repro

The suggested workaround, @SpringBootApplication(exclude=WebMvcObservationAutoConfiguration.class) annotation solves the issue.

Details are at spring-projects/spring-framework#32099 (I was redirected from there)

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jan 24, 2024
jenkins-daemon pushed a commit to hortonworks/cloudbreak that referenced this issue Jan 25, 2024
as it clashes with DefaultJerseyTagsProvider, rendering Prometheus
actuator output useless by showing "UNKNOWN" in the `uri` field
instead of the real path pattern.
This is only happening with Jersey-annotated endpoints that we use.
It does work correctly with Spring-annotated endpoints, just for the record.

See issues:
- spring-projects/spring-framework#32099
- spring-projects/spring-boot#39294
@mhalbritter mhalbritter changed the title DefaultJerseyTagsProvider clashes with WebMvcObservationAutoConfiguration DefaultJerseyTagsProvider clashes with WebMvcObservationAutoConfiguration Jan 25, 2024
@bclozel
Copy link
Member

bclozel commented Jan 25, 2024

I think we can summarize the situation as follows: when Spring Boot auto-configures an application with 'org.springframework.boot:spring-boot-starter-actuator' as well as both 'org.springframework.boot:spring-boot-starter-jersey' and 'org.springframework.boot:spring-boot-starter-web' on the classpath, both Spring MVC and Jersey HTTP instrumentations are contributed as well.

Each instrumentation contributes the same metric name, which can cause conflicts like described in the linked issue.

I think we can consider several solutions:

  • call this out as a limitation in the documentation, as having both Jersey and Spring MVC in the same application might not be common?
  • update the auto-configurations to ensure that we don't contribute the Spring MVC observation fitter if the Jersey instrumentation is auto-configured

@bclozel bclozel added the for: team-meeting An issue we'd like to discuss as a team to make progress label Jan 25, 2024
@bergerdenes
Copy link
Author

The only problem with @SpringBootApplication(exclude=WebMvcObservationAutoConfiguration.class) is that in this case, we cannot obtain metrics from the Spring actuator endpoints (i.e. /info, /health, etc.)

@bclozel
Copy link
Member

bclozel commented Feb 2, 2024

Here two competing systems are trying to contribute to the same metric. Have you considered renaming one of the observations to separate both? You can use actuator.management.observations.http.server.requests.name to rename the server observations.

@bergerdenes
Copy link
Author

Can you please elaborate what you mean? You may open a branch from my repro project (or open a PR)

@bclozel
Copy link
Member

bclozel commented Feb 7, 2024

I mean doing the following in application.properties:

actuator.management.observations.http.server.requests.name=spring.http.server.requests

This will rename the HTTP server observations for Spring and leave the http.server.requests namespace for the Jersey variant.

@bergerdenes
Copy link
Author

The suggestion does not help at all. It keeps the URI properties UNKNOWN.
I tried setting the same for management.observations.http.server.requests.name but in that case the metrics actuator does not return anyting.

@bclozel
Copy link
Member

bclozel commented Feb 8, 2024

Sorry I forgot that the Jersey metrics would also follow the same property. I guess the only choice here is to disable the MVC instrumentation and only rely on the Jersey one.

@bclozel
Copy link
Member

bclozel commented Feb 16, 2024

We have discussed this as a team and think that there are several ways to fix this.

Remove the MVC starter as it's not used

First, if your application does not need Spring MVC then the org.springframework.boot:spring-boot-starter-web dependency should be removed, and you could then map Jersey to the base / path:

@ApplicationPath("/")
@Configuration
public class EndpointConfig extends ResourceConfig {

and adapt the application configuration as a result:

management:
  endpoints:
    web:
      base-path: "/"
      exposure:
        include: info,health,prometheus
      path-mapping:
        prometheus: metrics

I've checked that all requests are being instrumented as expected.

Configure the MVC Observation filter

If your application needs both Jersey and MVC, manually configuring the Observation Filter makes sense: Spring Boot won't necessarily know which paths your application intends to use with MVC or Jersey and which instrumentation should prevail in each case. Something like:

    @Bean
    public FilterRegistrationBean<ServerHttpObservationFilter> webMvcObservationFilter(ObservationRegistry registry) {
        ServerHttpObservationFilter filter = new ServerHttpObservationFilter(registry);
        FilterRegistrationBean<ServerHttpObservationFilter> registration = new FilterRegistrationBean<>(filter);
        registration.setOrder(Ordered.HIGHEST_PRECEDENCE + 1);
        registration.setDispatcherTypes(DispatcherType.REQUEST, DispatcherType.ASYNC);
        registration.addUrlPatterns("/mvc/*");
        return registration;
    }

In light of that, I'm closing this issue as we don't think we can make auto-configurations depend on each other and deduce what's expected for the application.

Thanks!

@bclozel bclozel closed this as not planned Won't fix, can't repro, duplicate, stale Feb 16, 2024
@bclozel bclozel added status: invalid An issue that we don't feel is valid and removed status: waiting-for-triage An issue we've not yet triaged labels Feb 16, 2024
@carlos-silva24
Copy link

Hey @bclozel, I was facing a similar problem but on my side, prometheus metrics are working fine. The problem that I'm facing is related to the tracing. I created a repo (https://github.com/carlos-silva24/springboot-observability) with a demo.

In summary, the traces have the URI defined as unknown, however in prometheus the metrics are fine.
If i remove the org.springframework.boot:spring-boot-starter-web like mentioned above, I don't get any traces at all.

@bclozel
Copy link
Member

bclozel commented Feb 19, 2024

Hey @carlos-silva24 this looks like an issue with the Jersey instrumentation, which is not maintained in Spring Boot but in Micrometer. Can you open an issue there? Thanks!

@carlos-silva24
Copy link

@bclozel thanks for the quick feedback.

I'll open an issue there also, but was thinking if the need to also include the spring-boot-starter-web in order to report traces could be a problem.
I was expecting the spring-boot-starter-jersey to have something similar as JerseyServerMetricsAutoConfiguration but for traces.

@bclozel
Copy link
Member

bclozel commented Feb 19, 2024

@carlos-silva24 Sorry, I don't think you'll need to create a new issue in Micrometer. In fact, to get traces you'll need an instrumentation that uses the Observation API and the Micrometer project contributed it to the Jersey project.

This means that the Spring Boot auto-configuration should use the new org.glassfish.jersey.micrometer.server.ObservationRequestEventListener instead of the current instrumentation which only supports metrics. Can you create a new Spring Boot issue for that?

Thanks!

@ftdinh
Copy link

ftdinh commented Apr 26, 2024

Hi @bclozel, I have had the same problem as described in the original issue title (unknown URIs). Excluding @SpringBootApplication(exclude=WebMvcObservationAutoConfiguration.class) does fix that issue, but then spring boot actuator endpoints like /metrics do not get recorded.

Repro project: https://github.com/ftdinh/metrics-repro

I have forked the original reproduction sample project and tried the suggested solution to configure the web MVC observation filter.

  @Bean
  public FilterRegistrationBean<ServerHttpObservationFilter> webMvcObservationFilter(ObservationRegistry registry) {
      ServerHttpObservationFilter filter = new ServerHttpObservationFilter(registry);
      FilterRegistrationBean<ServerHttpObservationFilter> registration = new FilterRegistrationBean<>(filter);
      registration.setOrder(Ordered.HIGHEST_PRECEDENCE + 1);
      registration.setDispatcherTypes(DispatcherType.REQUEST, DispatcherType.ASYNC);
      registration.addUrlPatterns("/metrics");
      return registration;
  }

I am finding that only the URI that is first requested is being reported:

  1. If first request is to /metrics, then calls to /jersey/test are not reported
  2. If first request is to /jersey/test, then calls to /metrics are not reported

Wasn't sure if I need to open a new issue for this -- if so, then I can do so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: invalid An issue that we don't feel is valid
Projects
None yet
Development

No branches or pull requests

6 participants