Skip to content

Improve documentation of how actuator integrates with both Jersey and Spring MVC #17523

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

Closed
bric3 opened this issue Jul 15, 2019 · 6 comments
Closed
Assignees
Labels
type: documentation A documentation update
Milestone

Comments

@bric3
Copy link

bric3 commented Jul 15, 2019

Following a suggestion from @wilkinsona on this #2025 (comment), I'm opening this issue, as I believe there could be some refinement in the documentation and in the code too regarding the exposure of actuator endpoints


Reading this https://docs.spring.io/spring-boot/docs/2.1.6.RELEASE/reference/htmlsingle/#production-ready-customizing-management-server-context-path (Using 2.1.6.RELEASE)

Unless the management port has been configured to expose endpoints by using a different HTTP port, management.endpoints.web.base-path is relative to server.servlet.context-path. If management.server.port is configured, management.endpoints.web.base-path is relative to management.server.servlet.context-path.

I modified the configuration this way

+server:
+  servlet:
+    context-path: /
spring.mvc.servlet.path: /static
spring:
  jersey:
    application-path: /
    servlet:
      load-on-startup: 1

management:
  endpoint:
    health.enabled: true
+  endpoints:
+    web:
+      base-path: /actuator
+      path-mapping.health: healthcheck

However actuators keep showing up under /static instead of /

  • curl localhost:8080/static/actuator/healthcheck => 200
  • curl localhost:8080/actuator/healthcheck => 404

So either there's something missing on the documentation or they are some limitations to how actuator integrate when there's both spring mvc and jersey.

Even switching the dispatcher servlet to / didn't work

-spring.mvc.servlet.path: /static
+spring.mvc.servlet.path: /

From my points over there #2025 (comment) I iterated a bit on the refinement in this area.

  1. The doc should state which technology it will take precedence. And as such how other properties are affected, i.e. server.servlet.context-path has no effect when Spring MVC is also present.
  2. Is there a way to configure which technology will be used to expose actuators?
  3. Is there a way to configure an actuator only (Spring MVC) servlet ?
  4. If there's no way to do the above maybe indicate how to play with the DispatcherServlet or a filter that can make use of a request dispatcher.

When the spring.mvc.servlet.path: / there this log:

Mapping servlets: oldMetricsService urls=[/metrics], company.web.JerseyConfig urls=[/*], dispatcherServlet urls=[/]

This makes sense as the Jersey app intercepts everything on /*

This last is actually interesting if one want to migrate one endpoint at a time from one technology to another.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jul 15, 2019
@bric3
Copy link
Author

bric3 commented Jul 15, 2019

Actually I went ahead on point 4 and found a way to redirect to SpringMVC, the code is a bit rough and need some more polishing in particular on the configurability, and it is probably not suited when there is many stuff handled by Spring MVC, but when there's just two path handled by Spring MVC's dispatcherServlet, this works quite well.

import org.springframework.core.annotation.Order;
import org.springframework.stereotype.Component;

import javax.servlet.DispatcherType;
import javax.servlet.Filter;
import javax.servlet.FilterChain;
import javax.servlet.FilterConfig;
import javax.servlet.ServletContext;
import javax.servlet.ServletException;
import javax.servlet.ServletRequest;
import javax.servlet.ServletResponse;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import java.io.IOException;

import static org.springframework.core.Ordered.HIGHEST_PRECEDENCE;

@Component
@Order(HIGHEST_PRECEDENCE)
public class SpringMvcDispatchingFilter implements Filter {
    private ServletContext servletContext;
    private Set<String> springMvcPrefixes = Set.of("/actuator", "/doc");

    @Override
    public void init(FilterConfig filterConfig) {
        servletContext = filterConfig.getServletContext();
    }

    @Override
    public void doFilter(ServletRequest request,
                         ServletResponse response,
                         FilterChain chain) throws IOException,
                                                   ServletException {
        if (request.getDispatcherType() == DispatcherType.FORWARD) {
            chain.doFilter(request, response);
            return;
        }

        if (request instanceof HttpServletRequest && response instanceof HttpServletResponse) {
            doHttpFilter((HttpServletRequest) request,
                         (HttpServletResponse) response,
                         chain);
            return;
        }

        chain.doFilter(request, response);
    }

    private void doHttpFilter(HttpServletRequest request, HttpServletResponse response, FilterChain chain) throws ServletException, IOException {
        if (springMvcPrefixes.stream().anyMatch(request.getRequestURI()::startsWith)) {
            servletContext.getNamedDispatcher("dispatcherServlet")
                          .forward(request, response);
        }
        chain.doFilter(request, response);
    }
}

@philwebb philwebb added type: documentation A documentation update and removed status: waiting-for-triage An issue we've not yet triaged labels Jul 24, 2019
@philwebb philwebb added this to the 2.1.x milestone Jul 24, 2019
@bric3
Copy link
Author

bric3 commented Aug 5, 2019

By the way, auto forwarding to index.html do not work.

e.g. requesting GET /doc/ do not end up forwarded to the /doc/index.html

https://github.com/spring-projects/spring-framework/blob/5.0.x/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/ResourceHttpRequestHandler.java#L438-L444

// For very general mappings (e.g. "/") we need to check 404 first
Resource resource = getResource(request);
if (resource == null) {
	logger.trace("No matching resource found - returning 404");
	response.sendError(HttpServletResponse.SC_NOT_FOUND);
	return;
}

Will find no resource and return 404.

If uses a WebMvcConfigurer configured like this

@Override
public void addViewControllers(ViewControllerRegistry registry) {
    registry.addViewController("/doc/").setViewName("forward:/doc/index.html");
}

Then this code

https://github.com/spring-projects/spring-framework/blob/5.0.x/spring-webmvc/src/main/java/org/springframework/web/servlet/view/InternalResourceView.java#L149-L150

// Obtain a RequestDispatcher for the target resource (typically a JSP).
RequestDispatcher rd = getRequestDispatcher(request, dispatcherPath);

Will return the RequestDispatcher of the jersey servlet, hence forwarding

https://github.com/spring-projects/spring-framework/blob/5.0.x/spring-webmvc/src/main/java/org/springframework/web/servlet/view/InternalResourceView.java#L170

rd.forward(request, response);

will fail because there's no jersey resource lie that.

The only option currently is to forward in the example servlet above.

@mbhave
Copy link
Contributor

mbhave commented Aug 6, 2019

@bric3 Let's keep this issue for documenting how the actuator endpoints behave if both Spring MVC and Jersey are on the classpath. This comment doesn't seem it's related to that documentation issue.
If you have any questions about how things work, please join us on Gitter or ask on StackOverflow. If you think you've found a bug, please raise a new issue with a minimal sample that we can use to reproduce the bug.

@bric3
Copy link
Author

bric3 commented Aug 6, 2019

@mbhave Yes sure! I was commenting on my issue so that others know what this is about and how to work around the "issue". This could be especially important when one want to migrate from Jaxrs (and possibly Jersey) to Spring MVC endpoint, but gradually without doing big-bang changes.

@bric3
Copy link
Author

bric3 commented Aug 8, 2019

For reference, I have created a simple project that highlights the issue (via failing tests). This project also provides two mitigation of the issue. One that is based on the solution I provide in the above comment, and one that configures Spring Boot.

@wilkinsona Do you thing this spring boot configuration (code) could land in some way in the spring boot repo, of course I should open a new issue / pr if you think so.

https://github.com/bric3/jersey-webmvc

Currently I'm not quite happy to have to configure manually the url-mappings, but that's ok.
Also the full configuration approach requires to hack Spring WebMvc quite a bit.

@wilkinsona
Copy link
Member

Thanks for the sample, @bric3. You can get things working without relying on your mitigations by configuring the Jersey auto-configuration to register Jersey as a Filter (rather than as a Servlet) and by configuring it to forward to the rest of the filter chain in the event of it being a 404 from Jersey's perspective. To do this, set spring.jersey.type: filter in application.yaml and add the following to JerseyStuff:

property(ServletProperties.FILTER_FORWARD_ON_404, true);

With these two changes in place, JerseyWebmvcApplicationNoMitigationTests passes. I'll make some updates to the documentation along these lines.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: documentation A documentation update
Projects
None yet
Development

No branches or pull requests

6 participants