SkipPattern for Management Endpoints doesn't work with context-path about java-spring-web HOT 9 CLOSED

cc) @larchanjo @ask4gilles could you please have a look. You worked on the actuator part.

Does the context path affect the actuator endpoints?

I think we should remove the context path from the actuator skip pattern and add e2e test with configured context-path.

Would you be interested in submitting a PR?

from java-spring-web.

I'm trying to understand the unit tests and found this while debugging SkipEndPointsWithContextPathAndBasePathTest#testTraceHealthCareEndpoint:

from java-spring-web.

The TestRestTemplate expands the URL automatically by context-path (RestTemplate#execute).
When you prefix your URL to call for tests with the context-path
private static final String HEALTHCARE = CONTEXT_PATH + "/healthcare";
it will be there twice and therefore can be easily removed once and still match the pattern.

I think, the unit test is wrong and hides the actual error I mentioned in my initial post.

I would not like to submit a PR because it affects a lot of unit test methods and I can't estimate the impact.

from java-spring-web.

@pavolloffay @ancore I'll have a look at this tomorrow.

from java-spring-web.

@ancore In order to gain time, can you please provide a minimal project sample to reproduce your issue? Thanks!

from java-spring-web.

You will see the effect when you remove CONTEXT_PATH + from
private static final String HEALTHCARE = CONTEXT_PATH + "/healthcare"

from java-spring-web.

@ancore What's "the effect"?
If I removed it, I still see it traced, which is normal.
What's your issue exactly? An actuator endpoint traced which should not or the other way around?
When you say:

The following is added to the configured default skip pattern:
/user/api/management/(health|health/.|info|info/.)

Did you add this configuration yourself or is it the result that you see?
It would be easier to have a simple example with your configuration. Thanks

from java-spring-web.

Sorry, it is not happening in HEALTHCARE as I mentioned before.

Module: io.opentracing.contrib:opentracing-spring-web-starter:2.1.3-SNAPSHOT
Class: SkipEndPointsWithContextPathAndBasePathTest

Let's focus one one test:

private static final String INFO_ACTUATOR = CONTEXT_PATH + ACTUATOR_PATH + INFO_PATH;

@Test
public void testSkipInfoEndpoint() {
   invokeEndpoint(INFO_ACTUATOR);
   assertNoSpans();
}

Debugging: Breakpoint at line 271 in io.opentracing.contrib.web.servlet.filter.TracingFilter

Inspection of httpServletRequest.getRequestURI shows /myapp two times.
When using a TestRestTemplate, the configured context-path is added automatically (RestTemplate line 669). We must not add it to the test URL (INFO_ACTUATOR).

So, adding CONTEXT_PATH to the test URLs is wrong in these unit tests. The test fails (rightly so) if you setup the test URL correctly:

private static final String INFO_ACTUATOR = ACTUATOR_PATH + INFO_PATH;

The following happens:

• test starts with '/actuator/info'
• rest template prefixes /myapp, so you get '/myapp/actuator/info'
• TracingFilter.isTraced removes '/myapp' again
• no match of skippPattern because the pattern contains a context-path for actuator: '/myapp/actuator/info' (shortened the fragment of pattern)

The problem is the skip pattern, it should not contain the context-path.

from java-spring-web.

@ancore Thanks, makes sense. See attached PR.

from java-spring-web.