do not lose span renames when the rename happens before returning actions in a play controller (#1412)

Author: ivantopo

instrumentation/kamon-play/src/main/scala/kamon/instrumentation/play/PlayServerInstrumentation.scala

@@ -291,7 +291,9 @@ object GenerateOperationNameOnFilterHandler {
   def enter(@Advice.Argument(0) request: RequestHeader): Unit = {
     request.attrs.get(Router.Attrs.HandlerDef).map(handler => {
       val span = Kamon.currentSpan()
-      span.name(_routerNameGenerator.generateOperationName(handler))
+      if (span.operationName() == "http.server.request") {
+        span.name(_routerNameGenerator.generateOperationName(handler))
+      }
       span.takeSamplingDecision()
     })
   }

instrumentation/kamon-play/src/test-common/scala/kamon/instrumentation/play/RequestInstrumentationSpec.scala

@@ -86,6 +86,16 @@ abstract class RequestHandlerInstrumentationSpec extends PlaySpecShim with Guice
       case ("GET", "/not-found") => handler(action { NotFound })
       case ("GET", "/server")    => handler(action { req => Ok(serverImplementationName(req)) })
       case ("GET", "/error")     => handler(action(_ => sys.error("This page generates an error!")))
+      case ("GET", "/rename-outside")    => {
+        Kamon.currentSpan().name("renamed-outside-action")
+        handler(action { req => Ok(serverImplementationName(req)) })
+      }
+      case ("GET", "/rename-inside")    => {
+        handler(action { req =>
+          Kamon.currentSpan().name("renamed-inside-action")
+          Ok(serverImplementationName(req))
+        })
+      }
     }
   }
 
@@ -146,6 +156,36 @@ abstract class RequestHandlerInstrumentationSpec extends PlaySpecShim with Guice
       }
     }
 
+    "do not assign operation names if the server span was renamed before returning an action" in {
+      val wsClient = app.injector.instanceOf[WSClient]
+      val endpoint = s"http://localhost:$port/rename-outside"
+      val response = await(wsClient.url(endpoint).get())
+      response.status mustBe 200
+
+      eventually(timeout(5 seconds)) {
+        val span = testSpanReporter().nextSpan().value
+        span.operationName mustBe "renamed-outside-action"
+        span.metricTags.get(plain("component")) mustBe expectedServer
+        span.metricTags.get(plain("http.method")) mustBe "GET"
+        span.metricTags.get(plainLong("http.status_code")) mustBe 200L
+      }
+    }
+
+    "preserve operation names assigned inside actions" in {
+      val wsClient = app.injector.instanceOf[WSClient]
+      val endpoint = s"http://localhost:$port/rename-inside"
+      val response = await(wsClient.url(endpoint).get())
+      response.status mustBe 200
+
+      eventually(timeout(5 seconds)) {
+        val span = testSpanReporter().nextSpan().value
+        span.operationName mustBe "renamed-inside-action"
+        span.metricTags.get(plain("component")) mustBe expectedServer
+        span.metricTags.get(plain("http.method")) mustBe "GET"
+        span.metricTags.get(plainLong("http.status_code")) mustBe 200L
+      }
+    }
+
     "read headers case-insensitively" in {
       val wsClient = app.injector.instanceOf[WSClient]
       val endpoint = s"http://localhost:$port/request-id"