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

[Not to Pull. Just discussion] adding some C++11 aspects and a HOT tracer #1

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions app_tracing.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@

#include <app_tracing.h>

namespace opentracing {
template<> MyHotTracer* MyHotTracer::s_tracer = 0;
}

36 changes: 36 additions & 0 deletions app_tracing.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
#ifndef INCLUDED_TRACING_H
#define INCLUDED_TRACING_H

#include <opentracing/tracer.h>
#include <opentracing/noop.h>

#include <hot.h>

using namespace opentracing;

// Defining two "different" tracer implementations
typedef NoopTracer ZipkinV1;
typedef NoopTracer ZipkinV2;

// Setting up stuff... Would be nice if they weren't that many...
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

quoting the comment

Would be nice if they weren't that many... is there any way GenericTracer to deduce those from just MyHotTracer?

// is there any way `GenericTracer` to deduce those from just MyHotTracer?
typedef HotTracer<ZipkinV1, ZipkinV2> MyHotTracer;
typedef HotSpan<ZipkinV1, ZipkinV2> MyHotSpan;
typedef HotOptions<ZipkinV1, ZipkinV2> MyHotOptions;
typedef HotContext<ZipkinV1, ZipkinV2> MyHotContext;
typedef HotAdapter<ZipkinV1, ZipkinV2> MyHotAdapter;

typedef GenericTracer<MyHotTracer,
MyHotSpan,
MyHotOptions,
MyHotContext,
MyHotAdapter>
GlobalTracer;

#include <tracing_cpp11.h>

// Now we "upgrade" the entire system to C++11. Yeiii!
typedef Cpp11Tracer<GlobalTracer> Cpp11GlobalTracer;
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Naming it just Tracer would make thing even more pretty.



#endif // INCLUDED_TRACING_H
Empty file added hot.cpp
Empty file.
297 changes: 297 additions & 0 deletions hot.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,297 @@
#ifndef INCLUDED_OPENTRACING_HOT_H
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Disclaimer: I did just some of Span, Context and Tracer. It's not a complete implementation.

#define INCLUDED_OPENTRACING_HOT_H

// -------------- Copied from noop.h --------------

#include <opentracing/baggage.h>
#include <opentracing/span.h>
#include <opentracing/spancontext.h>
#include <opentracing/spanoptions.h>
#include <opentracing/stringref.h>
#include <opentracing/tracer.h>
#include <stdint.h>

namespace opentracing {

template <typename T1, typename T2> class HotAdapter;
template <typename T1, typename T2> class HotContext;
template <typename T1, typename T2> class HotOptions;
template <typename T1, typename T2> class HotSpan;
template <typename T1, typename T2> class HotTracer;

template <typename T1, typename T2>
class HotAdapter {
typedef HotContext<T1, T2> MyHotContext;
public:
typedef MyHotContext* iterator;
typedef const MyHotContext* const_iterator;

BaggageRef ref(const const_iterator& it) const
{
static const char empty[] = "";
return BaggageRef(empty, empty);
}

Baggage copy(const const_iterator& it) const
{
return Baggage("", "");
}
};


/**
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Quoting the comment

VERY TRICKY. Pay attention to const-ness here. Span's context mustn't be const but extract()'s has to be const. To be properly thought/implemented.

* Note: VERY TRICKY. Pay attention to const-ness here. Span's context mustnn't
* be const but `extract()`'s has to be const. To be properly thought/implemented.
*/
template <typename T1, typename T2>
class HotContext : public GenericSpanContext<HotContext<T1, T2>, HotAdapter<T1, T2> > {
typedef typename T1::SpanContext SpanContext1;
typedef typename T2::SpanContext SpanContext2;

const SpanContext1 * ctx1; // Note: Might be NULL
const SpanContext2 * ctx2; // Note: Might be NULL

friend class HotTracer<T1, T2>;
friend class HotSpan<T1, T2>;

HotContext(const SpanContext1 * ctx1_, const SpanContext2 * ctx2_)
: ctx1(ctx1_)
, ctx2(ctx2_)
{
}

HotContext()
{
}

public:

typedef GenericSpanContext<HotContext<T1, T2>, HotAdapter<T1, T2> > BaggageIterator;

BaggageIterator baggageBeginImp() const
{
return baggageEndImp();
}
BaggageIterator baggageEndImp() const
{
return BaggageIterator(this);
}

int getBaggageImp(const StringRef&, std::string*) const
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here we have the usual getBaggage() issue mentioned in opentracing/specification#9 (comment)

{
return 1;
}
int getBaggageImp(const StringRef&, std::vector<std::string>*) const
{
return 1;
}
};

template <typename T1, typename T2>
class HotOptions
: public GenericSpanOptions<HotOptions<T1, T2>, HotContext<T1, T2>, HotAdapter<T1, T2> > {
public:
int setOperationImp(const StringRef&)
{
return 0;
}
int setStartTimeImp(const uint64_t)
{
return 0;
}
int setReferenceImp(const SpanReferenceType::Value, const HotContext<T1, T2>&)
{
return 0;
}

template <typename T>
int setTagImp(const StringRef&, const T&)
{
return 0;
}
};

template <typename T1, typename T2>
class HotSpan : public GenericSpan<HotSpan<T1,T2>, HotContext<T1,T2>, HotAdapter<T1,T2> > {
typedef typename T1::Span SpanT1;
typedef typename T2::Span SpanT2;

// Note: Might be NULL
SpanT1 * span1;
// Note: Might be NULL
SpanT2 * span2;

friend class HotTracer<T1, T2>;

HotSpan(SpanT1 * span1_, SpanT2 * span2_)
: span1(span1_)
, span2(span2_)
{
}

public:

const HotContext<T1,T2>* contextImp() const
{
return &m_context;
}

int setOperationImp(const StringRef&)
{
return 0;
}
int setBaggageImp(const StringRef&, const StringRef&)
{
return 0;
}

int getBaggageImp(const StringRef&, std::string*) const
{
return 1;
}
int getBaggageImp(const StringRef&, std::vector<std::string>*) const
{
return 1;
}

int finishImp()
{
return 0;
}
int finishImp(const uint64_t)
{
return 0;
}

template <typename T>
int tagImp(const StringRef&, const T&)
{
return 0;
}

template <typename T>
int logImp(const StringRef&, const T&)
{
return 0;
}

template <typename T>
int logImp(const StringRef&, const T&, const uint64_t)
{
return 0;
}

HotContext<T1,T2> m_context;
};

template <typename T1, typename T2>
class HotTracer : public GenericTracer<HotTracer<T1, T2>,
HotSpan<T1, T2>,
HotOptions<T1, T2>,
HotContext<T1, T2>,
HotAdapter<T1, T2> > {
typedef GenericTracer<HotTracer<T1, T2>,
HotSpan<T1, T2>,
HotOptions<T1, T2>,
HotContext<T1, T2>,
HotAdapter<T1, T2> > Parent;

T1 & t1;
T2 & t2;
public:

HotTracer(T1& t1_, T2&t2_)
: t1(t1_)
, t2(t2_)
{
}

static void installImp(HotTracer*tracer)
{
s_tracer = tracer;
}
static void uninstallImp()
{
s_tracer = 0;
}
static HotTracer* instanceImp()
{
return s_tracer;
}

HotOptions<T1,T2>* makeSpanOptionsImp()
{
return new HotOptions<T1,T2>();
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bad implementation. It should delegate etc. and then HotOptions<T1,T2> would proxy.

}

HotSpan<T1,T2>* startImp(const StringRef&name)
{
typename T1::Span * sp1 = t1.start(name); // note: might be NULL
typename T2::Span * sp2 = t2.start(name); // note: might be NULL

return new HotSpan<T1,T2>(sp1, sp2);
}
HotSpan<T1,T2>* startImp(const HotOptions<T1,T2>& options)
{
typename T1::Span * sp1 = t1.start(options); // note: might be NULL
typename T2::Span * sp2 = t2.start(options); // note: might be NULL

return new HotSpan<T1,T2>(sp1, sp2);
}

template <typename CARRIER>
int injectImp(CARRIER* carrier, const HotSpan<T1,T2> & span) const
{
// What happens if one of those two fails?
if (span.span1)
{
t1.inject(carrier, *span.span1);
}

if (span.span2)
{
t2.inject(carrier, *span.span2);
}

return 0;
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What should be the return value here?

}

template <typename CARRIER>
HotContext<T1,T2>* extractImp(const CARRIER& carrier)
{
const typename T1::SpanContext* ctx1 = t1.extract(carrier);

const typename T2::SpanContext* ctx2 = t2.extract(carrier);

return new HotContext<T1,T2>(ctx1, ctx2);
}

void cleanupImp(const HotOptions<T1,T2>* ob)
{
if (ob)
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this necessary? Think can remove those if's.

{
delete ob;
}
}
void cleanupImp(const typename Parent::Span*ob)
{
if (ob)
{
delete ob;
}
}

void cleanupImp(const HotContext<T1,T2>*ob)
{
if (ob)
{
delete ob;
}
}

private:
static HotTracer* s_tracer;
};

} // namespace opentracing
#endif // INCLUDED_OPENTRACING_HOT_H
35 changes: 35 additions & 0 deletions main.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
#include <app_tracing.h>

/**
* To run:
* clear && g++ -I. -D__USE_XOPEN2K8 main.cpp build/opentracing/libopentracing.a -o main && ./main
*/
int main()
{
// Setup
ZipkinV1 t1;
ZipkinV2 t2;

MyHotTracer imp(t1, t2);

GlobalTracer::install(&imp);


{
// Application code
auto tracer = Cpp11GlobalTracer::instance();

auto span = tracer.start("hi");
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this and ctx below are std::shared_ptr's i.e. there's no mem leak here.


std::vector<opentracing::TextMapPair> pairs;
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice that this compiles but it likely means we have a bit too much freedom. I guess here's what's most of the discussion about. I guess both ZipkinV1 and ZipkinV2 should support something that serializes to text and http headers. If there's a proprietary IPC extension that ZipkinV1 supported, I guess that ZipkinV2 should support it as well to be backwards compatible, so no harm. If instead we had ZipkinV1 and Jaeger, then I can see that Jaeger might not support the proprietary IPC extension (out of the box). It would be nice to be able to #include a single header file (with potentially infinite uglyness) and extend your Jaeger tracer to support the proprietary IPC. If this isn't possible, I guess the next alternative is to git-clone Jaeger and add support to it. In any case, for as long as Jaeger` doesn't support your custom IPC, the custom IPC infrastructure should be able to pretend it's a standard "HTTP" or "TEXT" type and it should work out of the box with potentially some inferior performance.


tracer.inject(&pairs, *span);

auto ctx = tracer.extract(pairs);
}

// Cleanup
GlobalTracer::uninstall();

return 0;
}
Loading