spandex-project / spandex

A platform agnostic tracing library
MIT License
335 stars 53 forks source link

Introduce SpanContext for distributed tracing #74

Closed GregMefford closed 6 years ago

GregMefford commented 6 years ago

Related to https://github.com/spandex-project/spandex_datadog/pull/3, this creates a new Spandex.SpanContext struct that we can use to control the API around distributed trace contexts.

Resolves #72

coveralls commented 6 years ago

Pull Request Test Coverage Report for Build 333


Changes Missing Coverage Covered Lines Changed/Added Lines %
lib/spandex.ex 12 14 85.71%
<!-- Total: 17 19 89.47% -->
Totals Coverage Status
Change from base Build 329: -0.2%
Covered Lines: 197
Relevant Lines: 240

💛 - Coveralls
GregMefford commented 6 years ago

Oh... now that I'm working on implementing the thing that sends the spans with their sampling priorities, Datadog wants to have that one each span and we don't send over the trace context when we send spans to the Sender API. I think we should do that in oder to support propagating this kind of data to a back-end. I'm not sure how other systems (Zipkin/Jaeger) do this, but I assume that if we were to pass the set of completed Span structs and the SpanContext struct, that would capture all the required information. It's another API breakage, though.

zachdaniel commented 6 years ago

We only send the span list? That sounds wrong :/

zachdaniel commented 6 years ago

As in a bad decision, I mean. I think we'll just have to break that API. I doubt there are many people with custom adapters right now, and if they need to upgrade it should be a very easy path.

sourcelevel-bot[bot] commented 6 years ago

Ebert has finished reviewing this Pull Request and has found:

You can see more details about this review at https://ebertapp.io/github/spandex-project/spandex/pulls/74.