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

W3C context propagation does not properly create local context from received traceparent header #975

Closed
pnerg opened this issue Mar 30, 2021 · 6 comments

Comments

@pnerg
Copy link
Contributor

pnerg commented Mar 30, 2021

Issue

When creating the local context using the header traceparent the local spanID is incorrectly using the received traceID.

Steps to reproduce

Enable w3c propagation

 #configure W3C context propagation
  kamon.propagation.http.default.entries {
    incoming.span = "w3c"
    outgoing.span = "w3c"
  }

Run simple app

import akka.actor.ActorSystem
import akka.http.scaladsl.Http
import akka.http.scaladsl.server.Directives._
import akka.stream.ActorMaterializer
import com.typesafe.config.ConfigFactory
import kamon.Kamon
import org.slf4j.LoggerFactory

object Server extends App {
  Kamon.init(ConfigFactory
    .defaultApplication()
    .withFallback(ConfigFactory.defaultOverrides())
    .withFallback(ConfigFactory.defaultReference())
    .resolve()
  )
  private implicit val system = ActorSystem("simple-kamon-server")
  private implicit val materializer = ActorMaterializer()
  private implicit val executionContext = system.dispatcher
  private val logger = LoggerFactory.getLogger(Server.getClass)
  val route = pathPrefix("api" / "account" / Segment / "details") { id =>
    val span = Kamon.currentSpan()
   logger.info(s"Got request for account details with id [$id] using TraceID [${span.trace.id.string}], SpanID [${span.id.string}] parentSpan [${span.parentId.string}] and sampled [${span.trace.samplingDecision}]")
   complete("OK")
 }

  Http()
    .newServerAt("0.0.0.0", 9696)
    .bindFlow(route)
    .map(_.localAddress)
    .map{addr =>
      val port = addr.getPort
      logger.info(s"Started on port $port")
    }
}

Send request with w3c header

http://localhost:9696/api/account/1234/details
traceparent: 00-0af7651916cd43dd8448eb211c80319c-b7ad6b7169203331-01

The printout from the app is

Got request for account details with id [1234] using TraceID [0af7651916cd43dd8448eb211c80319c], SpanID [0af7651916cd43dd8448eb211c80319c] parentSpan [b7ad6b7169203331] and sampled [sample]

As one can see the traceID and spanID are equal, which they of course should not be.

Reason

Looking at the code where the context is created (snippet below)

   if (traceParentComponents.length != 4) None else {
      val traceID = identityProvider.traceIdFactory.from(traceParentComponents(1))
      val spanID = identityProvider.spanIdFactory.from(traceParentComponents(2))
      val samplingDecision = unpackSamplingDecision(traceParentComponents(3))

      Some(Span.Remote(traceID, spanID, Trace(traceID, samplingDecision)))
    }

https://github.com/kamon-io/Kamon/blob/master/core/kamon-core/src/main/scala/kamon/trace/SpanPropagation.scala#L97
The Span.Remote is not correct, it is using the traceID as the new spanID.
Since W3C only propagates the parent context it means a new spanId needs to be generated and added to the context.

@pnerg
Copy link
Contributor Author

pnerg commented Mar 30, 2021

ping @duxet, the nasty tester is at it again...:)

@SimunKaracic
Copy link
Contributor

Do you two have this under control, or should I spend some time on it? :D

@pnerg
Copy link
Contributor Author

pnerg commented Mar 30, 2021

@SimunKaracic I don't have a recent fork of Kamon so I assume it's much faster for you to fix it.
Should pretty much only be to generate a 8-byte identifier for the local span.

@duxet
Copy link
Contributor

duxet commented Mar 31, 2021

@pnerg Thanks! Your issue description is a piece of really good work :D

I've opened PR #978 with a quick fix.

@pnerg
Copy link
Contributor Author

pnerg commented Mar 31, 2021

Thx, I really hate the typical issue ticket..."nnn doesn't work" without any description nor analysis
In fact I tend to reject tickets without a description on how to reproduce it

@SimunKaracic
Copy link
Contributor

Fixed in 2.1.15!
Thanks for the nice report and quick fix 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants