From 1923f056cb72ce703015aa12311ddf116a1ee10f Mon Sep 17 00:00:00 2001 From: Tom Tan Date: Mon, 7 Jun 2021 12:20:58 -0700 Subject: [PATCH 1/2] Fix endianess of Jaeger IDs for transmission --- .../exporters/jaeger/recordable.h | 34 +++++++++++++++++++ exporters/jaeger/src/recordable.cc | 13 +++++++ .../jaeger/test/jaeger_recordable_test.cc | 18 +++++++--- 3 files changed, 60 insertions(+), 5 deletions(-) diff --git a/exporters/jaeger/include/opentelemetry/exporters/jaeger/recordable.h b/exporters/jaeger/include/opentelemetry/exporters/jaeger/recordable.h index 10da9360a5..c5ce57c3c4 100644 --- a/exporters/jaeger/include/opentelemetry/exporters/jaeger/recordable.h +++ b/exporters/jaeger/include/opentelemetry/exporters/jaeger/recordable.h @@ -7,12 +7,46 @@ #include #include +#if (defined(__BYTE_ORDER__) && defined(__ORDER_LITTLE_ENDIAN__) && \ + __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__) +# define JAEGER_IS_LITTLE_ENDIAN 1 +#elif defined(__BYTE_ORDER__) && defined(__ORDER_BIG_ENDIAN__) && \ + __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__ +# define JAEGER_IS_LITTLE_ENDIAN 0 +#elif defined(_WIN32) +# define JAEGER_IS_LITTLE_ENDIAN 1 +#else +# error "Endian detection needs to be set up for your compiler" +#endif + OPENTELEMETRY_BEGIN_NAMESPACE namespace exporter { namespace jaeger { +#if JAEGER_IS_LITTLE_ENDIAN == 1 + +# if defined(__clang__) || \ + (defined(__GNUC__) && ((__GNUC__ == 4 && __GNUC_MINOR__ >= 8) || __GNUC__ >= 5)) +inline uint64_t bswap_64(uint64_t host_int) +{ + return __builtin_bswap64(host_int); +} + +# elif defined(_MSC_VER) +inline uint64_t bswap_64(uint64_t host_int) +{ + return _byteswap_uint64(host_int); +} + +# else +# error "Port need to support endianess conversion" + +# endif + +#endif + using namespace jaegertracing; class Recordable final : public sdk::trace::Recordable diff --git a/exporters/jaeger/src/recordable.cc b/exporters/jaeger/src/recordable.cc index 8e6cf8d8ff..3b50b9bef6 100644 --- a/exporters/jaeger/src/recordable.cc +++ b/exporters/jaeger/src/recordable.cc @@ -39,12 +39,25 @@ void Recordable::PopulateAttribute(nostd::string_view key, const common::Attribu void Recordable::SetIdentity(const trace::SpanContext &span_context, trace::SpanId parent_span_id) noexcept { + // IDs should be converted to big endian before transmission. + // https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/sdk_exporters/jaeger.md#ids +#if JAEGER_IS_LITTLE_ENDIAN == 1 + span_->__set_traceIdHigh( + bswap_64(*(reinterpret_cast(span_context.trace_id().Id().data())))); + span_->__set_traceIdLow( + bswap_64(*(reinterpret_cast(span_context.trace_id().Id().data()) + 1))); + span_->__set_spanId( + bswap_64(*(reinterpret_cast(span_context.span_id().Id().data())))); + span_->__set_parentSpanId( + bswap_64(*(reinterpret_cast(parent_span_id.Id().data())))); +#else span_->__set_traceIdLow( *(reinterpret_cast(span_context.trace_id().Id().data()))); span_->__set_traceIdHigh( *(reinterpret_cast(span_context.trace_id().Id().data()) + 1)); span_->__set_spanId(*(reinterpret_cast(span_context.span_id().Id().data()))); span_->__set_parentSpanId(*(reinterpret_cast(parent_span_id.Id().data()))); +#endif // TODO: set trace_state. } diff --git a/exporters/jaeger/test/jaeger_recordable_test.cc b/exporters/jaeger/test/jaeger_recordable_test.cc index 5fd2400b18..ad257313a3 100644 --- a/exporters/jaeger/test/jaeger_recordable_test.cc +++ b/exporters/jaeger/test/jaeger_recordable_test.cc @@ -14,10 +14,11 @@ namespace nostd = opentelemetry::nostd; namespace sdktrace = opentelemetry::sdk::trace; using namespace jaegertracing; +using namespace opentelemetry::exporter::jaeger; TEST(JaegerSpanRecordable, SetIdentity) { - opentelemetry::exporter::jaeger::Recordable rec; + Recordable rec; int64_t trace_id_val[2] = {0x0000000000000000, 0x1000000000000000}; int64_t span_id_val = 0x2000000000000000; @@ -39,15 +40,22 @@ TEST(JaegerSpanRecordable, SetIdentity) std::unique_ptr span{rec.Span()}; +#if JAEGER_IS_LITTLE_ENDIAN == 1 + EXPECT_EQ(span->traceIdLow, bswap_64(trace_id_val[1])); + EXPECT_EQ(span->traceIdHigh, bswap_64(trace_id_val[0])); + EXPECT_EQ(span->spanId, bswap_64(span_id_val)); + EXPECT_EQ(span->parentSpanId, bswap_64(parent_span_id_val)); +#else EXPECT_EQ(span->traceIdLow, trace_id_val[0]); EXPECT_EQ(span->traceIdHigh, trace_id_val[1]); EXPECT_EQ(span->spanId, span_id_val); EXPECT_EQ(span->parentSpanId, parent_span_id_val); +#endif } TEST(JaegerSpanRecordable, SetName) { - opentelemetry::exporter::jaeger::Recordable rec; + Recordable rec; nostd::string_view name = "Test Span"; rec.SetName(name); @@ -59,7 +67,7 @@ TEST(JaegerSpanRecordable, SetName) TEST(JaegerSpanRecordable, SetStartTime) { - opentelemetry::exporter::jaeger::Recordable rec; + Recordable rec; std::chrono::system_clock::time_point start_time = std::chrono::system_clock::now(); opentelemetry::common::SystemTimestamp start_timestamp(start_time); @@ -74,7 +82,7 @@ TEST(JaegerSpanRecordable, SetStartTime) TEST(JaegerSpanRecordable, SetDuration) { - opentelemetry::exporter::jaeger::Recordable rec; + Recordable rec; opentelemetry::common::SystemTimestamp start_timestamp; @@ -92,7 +100,7 @@ TEST(JaegerSpanRecordable, SetDuration) TEST(JaegerSpanRecordable, SetStatus) { - opentelemetry::exporter::jaeger::Recordable rec; + Recordable rec; const char *error_description = "Error test"; rec.SetStatus(trace::StatusCode::kError, error_description); From 2965d0b73135f04003b81885071553461d3700c6 Mon Sep 17 00:00:00 2001 From: Tom Tan Date: Mon, 7 Jun 2021 13:30:25 -0700 Subject: [PATCH 2/2] Fix naming conflict --- .../jaeger/test/jaeger_recordable_test.cc | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/exporters/jaeger/test/jaeger_recordable_test.cc b/exporters/jaeger/test/jaeger_recordable_test.cc index ad257313a3..f9e415065e 100644 --- a/exporters/jaeger/test/jaeger_recordable_test.cc +++ b/exporters/jaeger/test/jaeger_recordable_test.cc @@ -18,7 +18,7 @@ using namespace opentelemetry::exporter::jaeger; TEST(JaegerSpanRecordable, SetIdentity) { - Recordable rec; + opentelemetry::exporter::jaeger::Recordable rec; int64_t trace_id_val[2] = {0x0000000000000000, 0x1000000000000000}; int64_t span_id_val = 0x2000000000000000; @@ -41,10 +41,10 @@ TEST(JaegerSpanRecordable, SetIdentity) std::unique_ptr span{rec.Span()}; #if JAEGER_IS_LITTLE_ENDIAN == 1 - EXPECT_EQ(span->traceIdLow, bswap_64(trace_id_val[1])); - EXPECT_EQ(span->traceIdHigh, bswap_64(trace_id_val[0])); - EXPECT_EQ(span->spanId, bswap_64(span_id_val)); - EXPECT_EQ(span->parentSpanId, bswap_64(parent_span_id_val)); + EXPECT_EQ(span->traceIdLow, opentelemetry::exporter::jaeger::bswap_64(trace_id_val[1])); + EXPECT_EQ(span->traceIdHigh, opentelemetry::exporter::jaeger::bswap_64(trace_id_val[0])); + EXPECT_EQ(span->spanId, opentelemetry::exporter::jaeger::bswap_64(span_id_val)); + EXPECT_EQ(span->parentSpanId, opentelemetry::exporter::jaeger::bswap_64(parent_span_id_val)); #else EXPECT_EQ(span->traceIdLow, trace_id_val[0]); EXPECT_EQ(span->traceIdHigh, trace_id_val[1]); @@ -55,7 +55,7 @@ TEST(JaegerSpanRecordable, SetIdentity) TEST(JaegerSpanRecordable, SetName) { - Recordable rec; + opentelemetry::exporter::jaeger::Recordable rec; nostd::string_view name = "Test Span"; rec.SetName(name); @@ -67,7 +67,7 @@ TEST(JaegerSpanRecordable, SetName) TEST(JaegerSpanRecordable, SetStartTime) { - Recordable rec; + opentelemetry::exporter::jaeger::Recordable rec; std::chrono::system_clock::time_point start_time = std::chrono::system_clock::now(); opentelemetry::common::SystemTimestamp start_timestamp(start_time); @@ -82,7 +82,7 @@ TEST(JaegerSpanRecordable, SetStartTime) TEST(JaegerSpanRecordable, SetDuration) { - Recordable rec; + opentelemetry::exporter::jaeger::Recordable rec; opentelemetry::common::SystemTimestamp start_timestamp; @@ -100,7 +100,7 @@ TEST(JaegerSpanRecordable, SetDuration) TEST(JaegerSpanRecordable, SetStatus) { - Recordable rec; + opentelemetry::exporter::jaeger::Recordable rec; const char *error_description = "Error test"; rec.SetStatus(trace::StatusCode::kError, error_description);