format, todo clean ups

open-telemetry · Aug 19, 2020 · c0e74ff · c0e74ff
1 parent d066f4e
commit c0e74ff
Show file tree

Hide file tree

Showing 4 changed files with 22 additions and 49 deletions.
diff --git 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<T>
     }
   }
 
-  // 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<T>
   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<T>
       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<T>
     }
   }
 
-  //  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<T>
       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
@@ -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,56 +30,45 @@ 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)
   {
     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)
   {
     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
@@ -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> tracer{new NoopTracer{}};
-  //  auto s1 = tracer->StartSpan("abc");
-  //  EXPECT_EQ(&s1->tracer(), tracer.get());
-  //
-  //  std::map<std::string, std::string> attributes1;
-  //  s1->AddEvent("abc", attributes1);
-  //
-  //  std::vector<std::pair<std::string, int>> attributes2;
-  //  s1->AddEvent("abc", attributes2);
-  //
-  //  s1->AddEvent("abc", {{"a", 1}, {"b", "2"}, {"c", 3.0}});
-  //
-  //  std::vector<std::pair<std::string, std::vector<int>>> attributes3;
-  //  s1->AddEvent("abc", attributes3);
-  //
-  //  s1->SetAttribute("abc", 4);
+  std::shared_ptr<Tracer> tracer{new NoopTracer{}};
+  auto s1 = tracer->StartSpan("abc");
+  EXPECT_EQ(&s1->tracer(), tracer.get());
+
+  std::map<std::string, std::string> attributes1;
+  s1->AddEvent("abc", attributes1);
+
+  std::vector<std::pair<std::string, int>> attributes2;
+  s1->AddEvent("abc", attributes2);
+
+  s1->AddEvent("abc", {{"a", 1}, {"b", "2"}, {"c", 3.0}});
+
+  std::vector<std::pair<std::string, std::vector<int>>> 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
@@ -85,5 +85,3 @@ TEST(HTTPTextFormatTest, PropagateInvalidContext)
   format.Inject(Setter, carrier, ctx);
   EXPECT_TRUE(carrier.count("traceparent") == 0);
 }
-
-// TODO: add trace state tests