Skip to content

Commit

Permalink
Properly append route path in VertxHttpServerMetrics
Browse files Browse the repository at this point in the history
We had a method called appendCurrentRoutePath() but it was overwriting
the route path each time.
This is a mistake as the VertxHttpServerMetrics.requestRouted() can be
called several times for the same request when you have additional
routers.
This is the case of Dev UI, where you have a router mounted on /q/ and
then a route mounted on /dev-ui/.

Note that this commit is only addressing the problem in the Micrometer
extension (but is also fixing OpenTelemetry when Micrometer is around as
in this case, this code path is used for both). I need to come up with
another fix when dealing with OpenTelemetry alone.
  • Loading branch information
gsmet committed Jan 10, 2025
1 parent 8d81d78 commit 6338ba6
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,31 @@ public void setTemplatePath(String path) {
}

public void appendCurrentRoutePath(String path) {
if (path != null && !path.isEmpty()) {
// we append the path to the current route path
// if a router is mounted on /q/ for instance, and a route of this router on /dev-ui/
// a request to /q/dev-ui/ needs to be affected to the /q/dev-ui/ path
// appendCurrentRoutePath() will be called twice, once with /q/ and the second time with /dev-ui/
if (path == null || path.isEmpty()) {
return;
}

if (this.currentRoutePath == null) {
this.currentRoutePath = path;
return;
}

boolean currentRoutePathTrailingSlash = this.currentRoutePath.endsWith("/");
boolean pathLeadingSlash = path.startsWith("/");
if (currentRoutePathTrailingSlash) {
if (pathLeadingSlash) {
this.currentRoutePath = this.currentRoutePath + path.substring(1);
} else {
this.currentRoutePath = this.currentRoutePath + path;
}
} else if (pathLeadingSlash) {
this.currentRoutePath = this.currentRoutePath + path;
} else {
this.currentRoutePath = this.currentRoutePath + '/' + path;
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,11 +129,17 @@ public HttpRequestMetric responsePushed(LongTaskTimer.Sample socketMetric, HttpM

@Override
public void requestRouted(HttpRequestMetric requestMetric, String route) {
// note that in the case of a route (e.g. /dev-ui/) mounted on a router (e.g. /q/)
// this method will be called twice, once for the router (route=/q/), the second time for the route (route=/dev-ui/)

log.debugf("requestRouted %s %s", route, requestMetric);
requestMetric.appendCurrentRoutePath(route);
if (route != null) {
requestMetric.request().context().putLocal("VertxRoute", route);

if (route == null) {
return;
}

requestMetric.appendCurrentRoutePath(route);
requestMetric.request().context().putLocal("VertxRoute", requestMetric.currentRoutePath);
}

/**
Expand Down

0 comments on commit 6338ba6

Please sign in to comment.