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

Spans as Resources #15

Open
tabdulradi opened this issue Jan 8, 2019 · 2 comments
Open

Spans as Resources #15

tabdulradi opened this issue Jan 8, 2019 · 2 comments
Labels
help wanted Extra attention is needed

Comments

@tabdulradi
Copy link
Owner

tabdulradi commented Jan 8, 2019

Spans have lifetime, i.e. they get created, logged against, then finally they must be finished.

Currently we tackle this in Cats DSL by providing inChildSpan block, that uses Bracket to guarantee finishing the Span even in case of failures. The signature looks like this (simplified)

def inChildSpan[A](operationName: String)(thunk: F[A]): F[A]

The aim of this task, is try to change the signature to be

def childSpan(operationName: String): Resource[F, SpanOps[F]]

So that it'd be used like this:

def getUser[F[_]](userId: String) =
  childSpan[F]("getUser").use { span =>
    span.log("userId" -> userId) *> getUserImpl(userId)
  }

Please keep in mind that Span get's propagated along the context of F, creation of child Span should yield a Tree-like lineage of spans. For example:

childSpan[F]("outer").use { _ =>
  childSpan[F]("inner1").use { _ => 
    childSpan[F]("inner11").use { _ => op1(userId) }
  } *>
  childSpan[F]("inner2").use { _ => 
    childSpan[F]("inner22").use { _ => op2(userId) }
  }
}

means that spans inner1 & inner2 are adjacent, and both are children of outer.

outer 
  |_ inner1
     |_ inner11
  |_ inner2
     |_ inner22
@tabdulradi
Copy link
Owner Author

tabdulradi commented Jan 8, 2019

Approach 1

Keeping Propagation as ReaderT

PR #14 attempts to implement childSpan as follows:

def childSpan(operationName: String): Resource[F, SpanOps[F]] = Resource[F, SpanOps[F]] {
  for {
    parent <- tracing.currentSpan()
    span <- tracing.startChild(parent, operationName)
    // ReaderT.local doesn't play nice with Resource.
  } yield span -> tracing.finish(span)
}

The code above doesn't work. Spans look like:

outer 
  |_ inner1
  |_ inner11
  |_ inner2
  |_ inner22

Everything was a direct child of the root span.

Since Propagation is backed by a ReaderT, we can only change the currentSpan by calling ReaderT.local(newSpan)(codeThatRunsWithTheNewSpan), the problem here that we inverted the control, codeThatRunsWithTheNewSpan is not a parameter any more.

@tabdulradi
Copy link
Owner Author

tabdulradi commented Jan 8, 2019

Approach 2

Implement Propagation as StateT

PR #13 attempts to implement Propagation as a StateT, which would allow setting the current span to new one, however we can't set it back to the parent span once the child has been finished.

for {
  parent <- tracing.getSpan()
  span <- tracing.startChild(parent, operationName)
  _ <- tracing.setSpan(span)
  release = tracing.finish(span) *> tracing.setSpan(parent) // this doesn't work, release is not part of the monadic flow
} yield richSpan -> release

As result of this, code in the PR produce wrong spans.

outer 
  |_ inner1
     |_ inner11
        |_ inner2
           |_ inner22

Every span was a child of the previous span, even if it was just adjacent to it.

@tabdulradi tabdulradi added the help wanted Extra attention is needed label Jan 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

1 participant