From c0e74ff2850220078f43ce5bc2ba57523cf17a41 Mon Sep 17 00:00:00 2001 From: Tianlin Zhao Date: Wed, 19 Aug 2020 13:34:58 -0400 Subject: [PATCH] format, todo clean ups --- .../trace/propagation/http_trace_context.h | 12 ------- .../opentelemetry/trace/span_context.h | 23 +++---------- api/test/trace/noop_test.cc | 34 +++++++++---------- .../propagation/http_text_format_test.cc | 2 -- 4 files changed, 22 insertions(+), 49 deletions(-) diff --git a/api/include/opentelemetry/trace/propagation/http_trace_context.h b/api/include/opentelemetry/trace/propagation/http_trace_context.h index 2319bdd2eb..4236436eca 100644 --- a/api/include/opentelemetry/trace/propagation/http_trace_context.h +++ b/api/include/opentelemetry/trace/propagation/http_trace_context.h @@ -26,7 +26,6 @@ #include "opentelemetry/trace/propagation/http_text_format.h" #include "opentelemetry/trace/span.h" #include "opentelemetry/trace/span_context.h" -// TODO: include trace_state.h back OPENTELEMETRY_BEGIN_NAMESPACE namespace trace @@ -180,9 +179,6 @@ class HttpTraceContext : public HTTPTextFormat } } - // TODO: - // static void InjectTraceState(TraceState trace_state, T &carrier, Setter setter) - static void InjectTraceParent(const SpanContext &span_context, T &carrier, Setter setter) { char trace_id[32]; @@ -213,7 +209,6 @@ class HttpTraceContext : public HTTPTextFormat static void InjectImpl(Setter setter, T &carrier, const SpanContext &span_context) { InjectTraceParent(span_context, carrier, setter); - // TODO: inject Trace State } static SpanContext ExtractContextFromTraceParent(nostd::string_view trace_parent) @@ -294,7 +289,6 @@ class HttpTraceContext : public HTTPTextFormat SpanId span_id_obj = GenerateSpanIdFromString(span_id); TraceFlags trace_flags_obj = GenerateTraceFlagsFromString(trace_flags); return SpanContext(trace_id_obj, span_id_obj, trace_flags_obj, true); - // TODO: Change to new Span Context constructor once TraceState is done } else { @@ -303,9 +297,6 @@ class HttpTraceContext : public HTTPTextFormat } } - // TODO: - // static TraceState ExtractTraceState(nostd::string_view &trace_state_header) - static SpanContext ExtractImpl(Getter getter, const T &carrier) { nostd::string_view trace_parent = getter(carrier, kTraceParent); @@ -314,10 +305,7 @@ class HttpTraceContext : public HTTPTextFormat return SpanContext(false, false); } SpanContext context_from_parent_header = ExtractContextFromTraceParent(trace_parent); - // TODO: - // if (!context_from_parent_header.IsValid()) return context_from_parent_header; - // TODO: extract from trace state } }; } // namespace propagation diff --git a/api/include/opentelemetry/trace/span_context.h b/api/include/opentelemetry/trace/span_context.h index d488c9a4a9..23af936cbe 100644 --- a/api/include/opentelemetry/trace/span_context.h +++ b/api/include/opentelemetry/trace/span_context.h @@ -18,7 +18,6 @@ #include "opentelemetry/trace/span_id.h" #include "opentelemetry/trace/trace_flags.h" #include "opentelemetry/trace/trace_id.h" -// TODO: include trace_state.h back OPENTELEMETRY_BEGIN_NAMESPACE namespace trace @@ -31,33 +30,25 @@ class SpanContext final public: // An invalid SpanContext. SpanContext() noexcept - : trace_flags_(trace::TraceFlags((uint8_t) false)), - // TODO: add trace state as an argument - remote_parent_(false){}; + : trace_flags_(trace::TraceFlags((uint8_t) false)), remote_parent_(false){}; SpanContext(bool sampled_flag, bool has_remote_parent) noexcept - : trace_flags_(trace::TraceFlags((uint8_t)sampled_flag)), - // TODO: add trace state as an argument - remote_parent_(has_remote_parent){}; + : trace_flags_(trace::TraceFlags((uint8_t)sampled_flag)), remote_parent_(has_remote_parent){}; SpanContext(TraceId trace_id, SpanId span_id, TraceFlags trace_flags, - // TODO: add trace state as an argument bool has_remote_parent) noexcept { - trace_id_ = trace_id; - span_id_ = span_id; - trace_flags_ = trace_flags; - // TODO: add trace state as an argument + trace_id_ = trace_id; + span_id_ = span_id; + trace_flags_ = trace_flags; remote_parent_ = has_remote_parent; } SpanContext(SpanContext &&ctx) : trace_id_(ctx.trace_id()), span_id_(ctx.span_id()), trace_flags_(ctx.trace_flags()) - // TODO: add trace state as an argument {} SpanContext(const SpanContext &ctx) : trace_id_(ctx.trace_id()), span_id_(ctx.span_id()), trace_flags_(ctx.trace_flags()) - // TODO: add trace state as an argument {} SpanContext &operator=(const SpanContext &ctx) @@ -65,7 +56,6 @@ class SpanContext final trace_id_ = ctx.trace_id_; span_id_ = ctx.span_id_; trace_flags_ = ctx.trace_flags_; - // TODO: add trace state as an argument return *this; }; SpanContext &operator=(SpanContext &&ctx) @@ -73,14 +63,12 @@ class SpanContext final trace_id_ = ctx.trace_id_; span_id_ = ctx.span_id_; trace_flags_ = ctx.trace_flags_; - // TODO: add trace state as an argument return *this; }; const TraceId &trace_id() const noexcept { return trace_id_; } const SpanId &span_id() const noexcept { return span_id_; } const TraceFlags &trace_flags() const noexcept { return trace_flags_; } - // TODO: add trace state getter bool IsValid() const noexcept { return trace_id_.IsValid() && span_id_.IsValid(); } @@ -94,7 +82,6 @@ class SpanContext final TraceId trace_id_; SpanId span_id_; TraceFlags trace_flags_; - // TODO: add the unique pointer of trace state as a field bool remote_parent_ = false; }; diff --git a/api/test/trace/noop_test.cc b/api/test/trace/noop_test.cc index dcc2d8bcac..fd7ae054b6 100644 --- a/api/test/trace/noop_test.cc +++ b/api/test/trace/noop_test.cc @@ -11,22 +11,22 @@ using opentelemetry::trace::Tracer; // Note: This test is no longer valid as Span no longer has field of tracer. Whether // removing it depends on the creator of this file. -TEST(NoopTest, DISABLE_UseNoopTracers) +TEST(NoopTest, DISABLED_UseNoopTracers) { - // std::shared_ptr tracer{new NoopTracer{}}; - // auto s1 = tracer->StartSpan("abc"); - // EXPECT_EQ(&s1->tracer(), tracer.get()); - // - // std::map attributes1; - // s1->AddEvent("abc", attributes1); - // - // std::vector> attributes2; - // s1->AddEvent("abc", attributes2); - // - // s1->AddEvent("abc", {{"a", 1}, {"b", "2"}, {"c", 3.0}}); - // - // std::vector>> attributes3; - // s1->AddEvent("abc", attributes3); - // - // s1->SetAttribute("abc", 4); + std::shared_ptr tracer{new NoopTracer{}}; + auto s1 = tracer->StartSpan("abc"); + EXPECT_EQ(&s1->tracer(), tracer.get()); + + std::map attributes1; + s1->AddEvent("abc", attributes1); + + std::vector> attributes2; + s1->AddEvent("abc", attributes2); + + s1->AddEvent("abc", {{"a", 1}, {"b", "2"}, {"c", 3.0}}); + + std::vector>> attributes3; + s1->AddEvent("abc", attributes3); + + s1->SetAttribute("abc", 4); } diff --git a/api/test/trace/propagation/http_text_format_test.cc b/api/test/trace/propagation/http_text_format_test.cc index c3d969a2a2..78df34fd17 100644 --- a/api/test/trace/propagation/http_text_format_test.cc +++ b/api/test/trace/propagation/http_text_format_test.cc @@ -85,5 +85,3 @@ TEST(HTTPTextFormatTest, PropagateInvalidContext) format.Inject(Setter, carrier, ctx); EXPECT_TRUE(carrier.count("traceparent") == 0); } - -// TODO: add trace state tests \ No newline at end of file