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

Fix WebFlux instrumentation after SPR-17395 #14876

Closed
making opened this issue Oct 17, 2018 · 3 comments
Closed

Fix WebFlux instrumentation after SPR-17395 #14876

making opened this issue Oct 17, 2018 · 3 comments
Assignees
Labels
type: task A general task
Milestone

Comments

@making
Copy link
Member

making commented Oct 17, 2018

After update to Spring Boot 2.1.0.RC1, ClassCastException occurred as follows

java.lang.ClassCastException: class java.lang.String cannot be cast to class org.springframework.web.util.pattern.PathPattern (java.lang.String is in module java.base of loader 'bootstrap'; org.springframework.web.util.pattern.PathPattern is in unnamed module of loader 'app')
	at org.springframework.boot.actuate.metrics.web.reactive.server.WebFluxTags.uri(WebFluxTags.java:98)
	at org.springframework.boot.actuate.metrics.web.reactive.server.DefaultWebFluxTagsProvider.httpRequestTags(DefaultWebFluxTagsProvider.java:37)
	at org.springframework.boot.actuate.metrics.web.reactive.server.MetricsWebFilter.error(MetricsWebFilter.java:104)
	at org.springframework.boot.actuate.metrics.web.reactive.server.MetricsWebFilter.lambda$filter$3(MetricsWebFilter.java:86)

While this is an issue in Spring Framework, Spring Boot should have an integration test to detect this detect that.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Oct 17, 2018
@bclozel bclozel self-assigned this Oct 17, 2018
@bclozel bclozel changed the title Add an integration test related to SPR-17395 ClassCastException String cannot be cast to class org.springframework.web.util.pattern.PathPattern in WebFlux Oct 17, 2018
@bclozel bclozel changed the title ClassCastException String cannot be cast to class org.springframework.web.util.pattern.PathPattern in WebFlux ClassCastException String cannot be cast to class PathPattern in WebFlux Oct 17, 2018
@bclozel
Copy link
Member

bclozel commented Oct 17, 2018

Here is a workaround in the meantime - developers can override the default WebFluxTagsProvider bean and provide their own implementation like this:

package com.example.demo;

import java.util.Arrays;

import io.micrometer.core.instrument.Tag;

import org.springframework.boot.actuate.metrics.web.reactive.server.WebFluxTags;
import org.springframework.boot.actuate.metrics.web.reactive.server.WebFluxTagsProvider;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.http.HttpStatus;
import org.springframework.web.reactive.HandlerMapping;
import org.springframework.web.reactive.function.server.RouterFunctions;
import org.springframework.web.server.ServerWebExchange;
import org.springframework.web.util.pattern.PathPattern;

@Configuration
public class WebFluxMetrixConfig {

	@Bean
	public WebFluxTagsProvider customTagsProvider() {
		return new WebFluxTagsProvider() {

			private final Tag URI_NOT_FOUND = Tag.of("uri", "NOT_FOUND");
			private final Tag URI_REDIRECTION = Tag.of("uri", "REDIRECTION");
			private final Tag URI_ROOT = Tag.of("uri", "root");

			@Override
			public Iterable<Tag> httpRequestTags(ServerWebExchange exchange, Throwable exception) {
				return Arrays.asList(WebFluxTags.method(exchange), uriTag(exchange),
						WebFluxTags.exception(exception), WebFluxTags.status(exchange),
						WebFluxTags.outcome(exchange));
			}

			Tag uriTag(ServerWebExchange exchange) {
				String matchingPattern = exchange
						.getAttribute(RouterFunctions.MATCHING_PATTERN_ATTRIBUTE);
				if (matchingPattern != null) {
					return Tag.of("uri", matchingPattern);
				}
				PathPattern pathPattern = exchange
						.getAttribute(HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE);
				if (pathPattern != null) {
					return Tag.of("uri", pathPattern.getPatternString());
				}
				HttpStatus status = exchange.getResponse().getStatusCode();
				if (status != null) {
					if (status.is3xxRedirection()) {
						return URI_REDIRECTION;
					}
					if (status == HttpStatus.NOT_FOUND) {
						return URI_NOT_FOUND;
					}
				}
				String path = exchange.getRequest().getPath().value();
				if (path.isEmpty()) {
					return URI_ROOT;
				}
				return Tag.of("uri", path);
			}
		};
	}
}

Note that this implementation should not be kept around after upgrading to the next Spring Boot version, as underlying Spring Framework changes will make it unfit.

@making
Copy link
Member Author

making commented Oct 17, 2018

It works for me, thanks!

@philwebb philwebb added type: task A general task and removed status: waiting-for-triage An issue we've not yet triaged labels Oct 17, 2018
@philwebb philwebb changed the title ClassCastException String cannot be cast to class PathPattern in WebFlux Add integration test for String cannot be cast to class PathPattern in WebFlux Oct 17, 2018
@philwebb philwebb added this to the 2.1.x milestone Oct 17, 2018
@bclozel bclozel changed the title Add integration test for String cannot be cast to class PathPattern in WebFlux Fix WebFlux instrumentation after SPR-17395 Oct 18, 2018
@bclozel bclozel modified the milestones: 2.1.x, 2.1.0 Oct 18, 2018
bclozel added a commit that referenced this issue Oct 18, 2018
SPR-17395 ensures that WebFlux.fn is adding a request attribute of type
`PathPattern` on the `HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE`.

A specific tag provider for WebFlux.fn is no longer necessary.

See gh-14876
@bclozel
Copy link
Member

bclozel commented Oct 18, 2018

SPR-17395 added integration tests for this case. I've just updated the instrumentation since we don't need a specific case for WebFlux.fn now - everything is configured through RouterFunctionMapping and is using the same request attribute now.

@bclozel bclozel closed this as completed Oct 18, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: task A general task
Projects
None yet
Development

No branches or pull requests

4 participants