Skip to content

Commit

Permalink
use route pattern string as server side operation name
Browse files Browse the repository at this point in the history
  • Loading branch information
lucasamoroso committed Oct 11, 2022
1 parent 241f2d2 commit ee37133
Show file tree
Hide file tree
Showing 6 changed files with 48 additions and 57 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@

public class ArmeriaHttpServerDecorator extends SimpleDecoratingHttpService {
public static final AttributeKey<HttpServerInstrumentation.RequestHandler> REQUEST_HANDLER_TRACE_KEY =
AttributeKey.valueOf(HttpServerInstrumentation.RequestHandler.class, "REQUEST_HANDLER_TRACE");
AttributeKey.valueOf(HttpServerInstrumentation.RequestHandler.class, "REQUEST_HANDLER_TRACE");

private final Map<Integer, HttpServerInstrumentation> serverInstrumentationMap;

Expand All @@ -47,15 +47,23 @@ public HttpResponse serve(ServiceRequestContext ctx, HttpRequest req) throws Exc
if (httpServerInstrumentation != null) {

final HttpServerInstrumentation.RequestHandler requestHandler =
httpServerInstrumentation.createHandler(KamonArmeriaMessageConverter.toRequest(req));
httpServerInstrumentation.createHandler(KamonArmeriaMessageConverter.toRequest(req));

ctx.log()
.whenComplete()
.thenAccept(log -> {
final Context context = ctx.attr(REQUEST_HANDLER_TRACE_KEY).context();
requestHandler.buildResponse(KamonArmeriaMessageConverter.toResponse(log),context);
requestHandler.responseSent();
});
.whenComplete()
.thenAccept(log -> {
final Context context = ctx.attr(REQUEST_HANDLER_TRACE_KEY).context();
/**
* This is true only when no configured Route was matched.
* Cases where the route is fallback should be managed here {@link kamon.instrumentation.armeria.server.HandleNotFoundMethodAdvisor#around}
*/
if (!ctx.config().route().isFallback()) {
requestHandler.span().name(ctx.config().route().patternString());
}

requestHandler.buildResponse(KamonArmeriaMessageConverter.toResponse(log), context);
requestHandler.responseSent();
});

try (Storage.Scope ignored = Kamon.storeContext(requestHandler.context())) {
ctx.setAttr(REQUEST_HANDLER_TRACE_KEY, requestHandler);
Expand Down
26 changes: 0 additions & 26 deletions instrumentation/kamon-armeria/src/main/resources/reference.conf
Original file line number Diff line number Diff line change
Expand Up @@ -129,32 +129,6 @@ kamon.instrumentation.armeria {
# a given request. Depending on the instrumented framework, it might be possible to apply this operation
# name automatically or not, check the frameworks' instrumentation docs for more details.
unhandled = "unhandled"

# FQCN for a HttpOperationNameGenerator implementation, or ony of the following shorthand forms:
# - default: Uses the set default operation name
# - method: Uses the request HTTP method as the operation name.
#
name-generator = "kamon.instrumentation.armeria.KamonArmeriaHttpOperationNameGenerator"

# Provides custom mappings from HTTP paths into operation names. Meant to be used in cases where the bytecode
# instrumentation is not able to provide a sensible operation name that is free of high cardinality values.
# For example, with the following configuration:
# mappings {
# "/organization/*/user/*/profile" = "/organization/:orgID/user/:userID/profile"
# "/events/*/rsvps" = "EventRSVPs"
# }
#
# Requests to "/organization/3651/user/39652/profile" and "/organization/22234/user/54543/profile" will have
# the same operation name "/organization/:orgID/user/:userID/profile".
#
# Similarly, requests to "/events/aaa-bb-ccc/rsvps" and "/events/1234/rsvps" will have the same operation
# name "EventRSVPs".
#
# The patterns are expressed as globs and the operation names are free form.
#
mappings {

}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
package kamon.instrumentation.armeria.server

import java.util

import com.linecorp.armeria.common.HttpStatus
import com.linecorp.armeria.server._
import com.typesafe.config.Config
Expand Down Expand Up @@ -78,13 +77,27 @@ object HandleNotFoundMethodAdvisor {
*/
@Advice.OnMethodExit(onThrowable = classOf[Throwable])
def around(@Advice.Argument(0) ctx: ServiceRequestContext,
@Advice.Argument(1) routingCtx: RoutingContext,
@Advice.Argument(2) statusException: HttpStatusException,
@Advice.Thrown throwable: Throwable): Unit = {
lazy val unhandledOperationName: String = Kamon.config().getConfig("kamon.instrumentation.armeria.server").getString("tracing.operations.unhandled")

val requestHandler = ctx.attr(REQUEST_HANDLER_TRACE_KEY)

if (throwable != null && statusException.httpStatus.code() == HttpStatus.NOT_FOUND.code()) {
val requestHandler = ctx.attr(REQUEST_HANDLER_TRACE_KEY)
requestHandler.span.name(unhandledOperationName)
requestHandler.span.name(unhandledOperationName)
} else {
/**
* If no exception was thrown then probably the request will be redirected because it doesn't ends with '/'.
* So here we are trying to find the Service config that will handle the request if we add a '/' to the end because Armeria will do that.
* This is the same strategy as the one Armeria uses here {@link com.linecorp.armeria.server.FallbackService# handleNotFound}
*/
val oldPath = routingCtx.path
val newPath = oldPath + '/'
val serviceConfig = ctx.config.virtualHost.findServiceConfig(routingCtx.overridePath(newPath))
if (serviceConfig.isPresent) {
requestHandler.span.name(serviceConfig.value().route().patternString())
}
}
}
}
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ class ArmeriaGrpcServerTracingSpec extends AnyWordSpec

eventually(timeout(3 seconds)) {
val span = testSpanReporter().nextSpan().value
span.operationName shouldBe "ArmeriaHelloService/Hello"
span.operationName shouldBe "/kamon.armeria.instrumentation.grpc.ArmeriaHelloService/Hello"
span.hasError shouldBe false
span.metricTags.get(plain("component")) shouldBe "armeria.http.server"
span.metricTags.get(plain("http.method")) shouldBe "POST"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ class ArmeriaHttpServerTracingSpec extends AnyWordSpec

private def testSuite(protocol: String, interface: String, port: Int): Unit = {

val webClient = newWebClient(protocol,port)
val webClient = newWebClient(protocol, port)

s"The Armeria $protocol server" when {

Expand Down Expand Up @@ -75,8 +75,8 @@ class ArmeriaHttpServerTracingSpec extends AnyWordSpec
val target = s"$protocol://$interface:$port/$pathNotFoundEndpoint"
val expected = "unhandled"

val request = HttpRequest.of(RequestHeaders.of(HttpMethod.GET, pathNotFoundEndpoint))
webClient.execute(request)
val request = HttpRequest.of(RequestHeaders.of(HttpMethod.GET, pathNotFoundEndpoint))
webClient.execute(request)

eventually(timeout(3 seconds)) {
val span = testSpanReporter().nextSpan().value
Expand All @@ -91,12 +91,12 @@ class ArmeriaHttpServerTracingSpec extends AnyWordSpec
}

"set operation name with path + http method" when {
"resource doesn't exist" in {
val target = s"$protocol://$interface:$port/$usersEndpoint/not-found"
val expected = "/users/{}"
"resource doesn't exist" in {
val target = s"$protocol://$interface:$port/$usersEndpoint/not-found"
val expected = "/users/:userId"

val request = HttpRequest.of(RequestHeaders.of(HttpMethod.GET, s"$usersEndpoint/not-found"))
webClient.execute(request)
val request = HttpRequest.of(RequestHeaders.of(HttpMethod.GET, s"$usersEndpoint/not-found"))
webClient.execute(request)

eventually(timeout(3 seconds)) {
val span = testSpanReporter().nextSpan().value
Expand All @@ -111,7 +111,7 @@ class ArmeriaHttpServerTracingSpec extends AnyWordSpec
}

"not include path variables names" in {
val expected = "/users/{}/accounts/{}"
val expected = "/users/:userId/accounts/:accountId"

val request = HttpRequest.of(RequestHeaders.of(HttpMethod.GET, userAccountEndpoint))
webClient.execute(request)
Expand All @@ -123,7 +123,7 @@ class ArmeriaHttpServerTracingSpec extends AnyWordSpec
}

"not fail when request url contains special regexp chars" in {
val expected = "/users/{}/accounts/{}"
val expected = "/users/:userId/accounts/:accountId"

val request = HttpRequest.of(RequestHeaders.of(HttpMethod.GET, s"$userAccountEndpoint**"))
val response = webClient.execute(request).aggregate().get()
Expand All @@ -137,7 +137,7 @@ class ArmeriaHttpServerTracingSpec extends AnyWordSpec

"mark spans as failed when request fails" in {
val target = s"$protocol://$interface:$port/$usersEndpoint/error"
val expected = "/users/{}"
val expected = "/users/:userId"

val request = HttpRequest.of(RequestHeaders.of(HttpMethod.GET, s"$usersEndpoint/error"))
webClient.execute(request)
Expand All @@ -156,9 +156,9 @@ class ArmeriaHttpServerTracingSpec extends AnyWordSpec
"return a redirect status code" when {
"a request to /docs is redirected to /docs/" in {
val target = s"$protocol://$interface:$port/$docsEndpoint"
val expected = s"/docs"
val expected = s"/docs/*"

val request = HttpRequest.of(RequestHeaders.of(HttpMethod.GET, docsEndpoint))
val request = HttpRequest.of(RequestHeaders.of(HttpMethod.GET, s"$docsEndpoint"))
webClient.execute(request)

eventually(timeout(3 seconds)) {
Expand All @@ -172,10 +172,10 @@ class ArmeriaHttpServerTracingSpec extends AnyWordSpec
}
}

"return a ok status code " when {
"return a ok status code" when {
"a request to /docs/ is done" in {
val target = s"$protocol://$interface:$port/$docsEndpoint/"
val expected = s"/docs"
val expected = s"/docs/*"

val request = HttpRequest.of(RequestHeaders.of(HttpMethod.GET, s"$docsEndpoint/"))
webClient.execute(request)
Expand Down

0 comments on commit ee37133

Please sign in to comment.