An extension of enum opentracing::SpanReferenceType, for a new Span. #206

Alek86 · 2020-02-17T22:35:04Z

The PR is the result of this discussion: https://groups.google.com/forum/?utm_medium=email&utm_source=footer#!msg/opentracing/92uhSCO-Tu8/D83rq-j4GQAJ

Basically, I need a way to force a traceId from user's code. Didn't see such way, had to write some dirty code in my copy of jaeger-client-cpp code.

Yuri Shkuro suggested to add a new jaeger specific reference type. He called it SELF, I decided to give a more specific name because of limitations (see below).

It is a bit similar to GO implementation (jaegertracing/jaeger-client-go#397) but I had some limitations:

I couldn't copy whole SpanContext, because jaegertracing::Tracer::StartSpanWithOptions implementation was not completely clear to me. So I decided to copy only traceID and spanID
There is an implicit rule in jaegertracing::Tracer::StartSpanWithOptions: if it's a root span, its spanID is the traceID.low(). I'm not sure if it's safe to overrule, so I didn't.

Only to copy traceID and (for non-root spans) spanID, because spanID for root spans == traceID.low() Signed-off-by: Oleksandr Bukaiev <[email protected]>

AppVeyorBot · 2020-02-17T23:02:08Z

✅ Build jaeger-client-cpp 89 completed (commit 017fb2314e by @)

AppVeyorBot · 2020-02-17T23:30:57Z

✅ Build jaeger-client-cpp 90 completed (commit 5c2ea4fea8 by @)

yurishkuro · 2020-02-17T23:56:59Z

I would prefer to stick with the same name Self and not diverge between different Jaeger clients. There is an open question about the sampling in the Go implementation: jaegertracing/jaeger-client-go#411 (comment). I tend to think that your implementation is actually better, although I can see that both scenarios have merit.

FWIW, it's not a requirement that spanID == traceID.low(), just the typical implementation aimed at saving CPU cycles by not generating yet another random ID.

…lang implementations of Jaeger client 2) SpanId is always taken from the SelfRef context Signed-off-by: Oleksandr Bukaiev <[email protected]>

…alizer_list) is explicit Signed-off-by: Oleksandr Bukaiev <[email protected]>

AppVeyorBot · 2020-02-18T01:06:37Z

❌ Build jaeger-client-cpp 91 failed (commit be62396d5a by @Alek86)

AppVeyorBot · 2020-02-18T01:31:44Z

❌ Build jaeger-client-cpp 92 failed (commit a54cfd8498 by @Alek86)

Signed-off-by: Oleksandr Bukaiev <[email protected]>

AppVeyorBot · 2020-02-18T02:54:37Z

✅ Build jaeger-client-cpp 93 completed (commit 405ab3917e by @Alek86)

yurishkuro

Please add a section to README, similar to Go client.

src/jaegertracing/Tracer.cpp

examples/App.cpp

src/jaegertracing/Reference.h

src/jaegertracing/Tracer.h

…ly reference to enforce this rule Signed-off-by: Oleksandr Bukaiev <[email protected]>

Signed-off-by: Oleksandr Bukaiev <[email protected]>

AppVeyorBot · 2020-02-20T02:50:07Z

✅ Build jaeger-client-cpp 94 completed (commit 42e5fe3877 by @Alek86)

AppVeyorBot · 2020-02-20T03:15:27Z

✅ Build jaeger-client-cpp 95 completed (commit 20a26c413e by @Alek86)

README.md

src/jaegertracing/Tracer.cpp

README.md

src/jaegertracing/Tracer.h

Signed-off-by: Oleksandr Bukaiev <[email protected]>

AppVeyorBot · 2020-02-21T02:37:15Z

✅ Build jaeger-client-cpp 96 completed (commit 7006a29004 by @Alek86)

yurishkuro

@mdouaihy PTAL

mdouaihy

I think a Unit Test is missing to illustrate the usage and to make sure that the behavior is the desired one.

src/jaegertracing/Tracer.cpp

Signed-off-by: Oleksandr Bukaiev <[email protected]>

AppVeyorBot · 2020-02-22T20:37:32Z

❌ Build jaeger-client-cpp 97 failed (commit 4228e083e0 by @Alek86)

Signed-off-by: Oleksandr Bukaiev <[email protected]>

AppVeyorBot · 2020-02-22T22:08:48Z

✅ Build jaeger-client-cpp 99 completed (commit 87741a8404 by @Alek86)

…heseids

AppVeyorBot · 2020-02-29T22:03:04Z

✅ Build jaeger-client-cpp 107 completed (commit c060282a18 by @Alek86)

Signed-off-by: Oleksandr Bukaiev <[email protected]>

AppVeyorBot · 2020-02-29T22:30:20Z

✅ Build jaeger-client-cpp 108 completed (commit dad3412cb6 by @Alek86)

AppVeyorBot · 2020-02-29T22:54:13Z

✅ Build jaeger-client-cpp 109 completed (commit 34e74027f8 by @Alek86)

Signed-off-by: Oleksandr Bukaiev <[email protected]>

AppVeyorBot · 2020-02-29T23:24:16Z

✅ Build jaeger-client-cpp 110 completed (commit 7cc37db36d by @Alek86)

Signed-off-by: Oleksandr Bukaiev <[email protected]>

AppVeyorBot · 2020-03-01T23:34:46Z

✅ Build jaeger-client-cpp 112 completed (commit 464e0f9c1b by @Alek86)

Signed-off-by: Oleksandr Bukaiev <[email protected]>

AppVeyorBot · 2020-03-02T18:31:47Z

✅ Build jaeger-client-cpp 113 completed (commit c49f49314a by @Alek86)

…aegertracing#206) * An extension of enum opentracing::SpanReferenceType, for a new Span. Only to copy traceID and (for non-root spans) spanID, because spanID for root spans == traceID.low() Signed-off-by: Oleksandr Bukaiev <[email protected]> * 1) Renamed useTheseIDs to SelfRef, for consistency between different lang implementations of Jaeger client 2) SpanId is always taken from the SelfRef context Signed-off-by: Oleksandr Bukaiev <[email protected]> * Seems like on some compilers jaegertracing::SpanContext::StrMap(initializer_list) is explicit Signed-off-by: Oleksandr Bukaiev <[email protected]> * Temporarily commented out the new unit test case body Signed-off-by: Oleksandr Bukaiev <[email protected]> * SelfRef only used for creating root spans and can be passed as the only reference to enforce this rule Signed-off-by: Oleksandr Bukaiev <[email protected]> * Apparently std::exception ctor with a const char* is not standard Signed-off-by: Oleksandr Bukaiev <[email protected]> * Updates in README and code comments as per reviewer's request Signed-off-by: Oleksandr Bukaiev <[email protected]> * Added a testTracer subtest with no selfref to check the github CI result Signed-off-by: Oleksandr Bukaiev <[email protected]> * Removed test changes Signed-off-by: Oleksandr Bukaiev <[email protected]> * Added 2 unittests - a simple span child and a simple self ref Signed-off-by: Oleksandr Bukaiev <[email protected]> * Added child span to selfref unittest, small changes in new tests Signed-off-by: Oleksandr Bukaiev <[email protected]> * Show better log if sef ref is used with other references Signed-off-by: Oleksandr Bukaiev <[email protected]> * Change in error log if selfref is used with another ref Signed-off-by: Oleksandr Bukaiev <[email protected]>

An extension of enum opentracing::SpanReferenceType, for a new Span.

d70263f

Only to copy traceID and (for non-root spans) spanID, because spanID for root spans == traceID.low() Signed-off-by: Oleksandr Bukaiev <[email protected]>

Alek86 added 2 commits February 17, 2020 17:40

1) Renamed useTheseIDs to SelfRef, for consistency between different …

af12132

…lang implementations of Jaeger client 2) SpanId is always taken from the SelfRef context Signed-off-by: Oleksandr Bukaiev <[email protected]>

Seems like on some compilers jaegertracing::SpanContext::StrMap(initi…

d1681d4

…alizer_list) is explicit Signed-off-by: Oleksandr Bukaiev <[email protected]>

Temporarily commented out the new unit test case body

d2accfd

Signed-off-by: Oleksandr Bukaiev <[email protected]>

yurishkuro reviewed Feb 18, 2020

View reviewed changes

src/jaegertracing/Tracer.cpp Outdated Show resolved Hide resolved

examples/App.cpp Outdated Show resolved Hide resolved

src/jaegertracing/Reference.h Outdated Show resolved Hide resolved

src/jaegertracing/Tracer.h Outdated Show resolved Hide resolved

Alek86 added 2 commits February 19, 2020 19:24

SelfRef only used for creating root spans and can be passed as the on…

713ebca

…ly reference to enforce this rule Signed-off-by: Oleksandr Bukaiev <[email protected]>

Apparently std::exception ctor with a const char* is not standard

3717a11

Signed-off-by: Oleksandr Bukaiev <[email protected]>