Add more comments from compatibility section

open-telemetry · Jan 2, 2023 · 36cca42 · 36cca42
1 parent 321a801
commit 36cca42
Show file tree

Hide file tree

Showing 6 changed files with 110 additions and 108 deletions.
diff --git a/opentracing-shim/include/span_context_shim.h b/opentracing-shim/include/span_context_shim.h
@@ -18,33 +18,20 @@ using BaggagePtr = nostd::shared_ptr<opentelemetry::baggage::Baggage>;
 class SpanContextShim final : public opentracing::SpanContext
 {
 public:
-
-  explicit SpanContextShim(const opentelemetry::trace::SpanContext& context, const BaggagePtr& baggage) : 
-    context_(context), baggage_(baggage) {}
-
-  inline SpanContextShim operator=(const SpanContextShim& other) 
-  {
-    return SpanContextShim(other.context_, other.baggage_);
-  }
-
+  explicit SpanContextShim(const opentelemetry::trace::SpanContext& context, const BaggagePtr& baggage)
+    : context_(context), baggage_(baggage) {}
   inline const opentelemetry::trace::SpanContext& context() const { return context_; }
-
   inline const BaggagePtr& baggage() const { return baggage_; }
-
-  SpanContextShim newWithKeyValue(const nostd::string_view &key, const nostd::string_view &value) const noexcept;
-
+  SpanContextShim newWithKeyValue(nostd::string_view key, nostd::string_view value) const noexcept;
   bool BaggageItem(nostd::string_view key, std::string& value) const noexcept;
-
+  // Overrides
   using VisitBaggageItem = std::function<bool(const std::string& key, const std::string& value)>;
   void ForeachBaggageItem(VisitBaggageItem f) const override;
-
   std::unique_ptr<opentracing::SpanContext> Clone() const noexcept override;
 
 private:
-
-  const opentelemetry::trace::SpanContext context_;
-  const BaggagePtr baggage_;
-
+  opentelemetry::trace::SpanContext context_;
+  BaggagePtr baggage_;
 };
 
 } // namespace opentracingshim

diff --git a/opentracing-shim/include/span_shim.h b/opentracing-shim/include/span_shim.h
@@ -24,12 +24,10 @@ using EventEntry = std::pair<opentracing::string_view, opentracing::Value>;
 class SpanShim : public opentracing::Span
 {
 public:
-
-  explicit SpanShim(const TracerShim& tracer, const SpanPtr& span, const BaggagePtr& baggage) : 
-    tracer_(tracer), span_(span), context_(span->GetContext(), baggage) {}
-
+  explicit SpanShim(const TracerShim& tracer, const SpanPtr& span, const BaggagePtr& baggage)
+    : tracer_(tracer), span_(span), context_(span->GetContext(), baggage) {}
   void handleError(const opentracing::Value& value) noexcept;
-
+  // Overrides
   void FinishWithOptions(const opentracing::FinishSpanOptions& finish_span_options) noexcept override;
   void SetOperationName(opentracing::string_view name) noexcept override;
   void SetTag(opentracing::string_view key, const opentracing::Value& value) noexcept override;
@@ -42,14 +40,12 @@ class SpanShim : public opentracing::Span
   inline const opentracing::Tracer& tracer() const noexcept override { return tracer_; };
 
 private:
-
   void logImpl(opentracing::SystemTime timestamp, nostd::span<const EventEntry> fields) noexcept;
 
   const TracerShim& tracer_;
   SpanPtr span_;
   SpanContextShim context_;
   mutable opentelemetry::common::SpinLockMutex context_lock_;
-
 };
 
 } // namespace opentracingshim

diff --git a/opentracing-shim/include/tracer_shim.h b/opentracing-shim/include/tracer_shim.h
@@ -19,14 +19,13 @@ using TracerProviderPtr = nostd::shared_ptr<opentelemetry::trace::TracerProvider
 using PropagatorPtr = nostd::shared_ptr<opentelemetry::context::propagation::TextMapPropagator>;
 
 struct OpenTracingPropagators {
-    PropagatorPtr textMap;
-    PropagatorPtr httpHeaders;
+    PropagatorPtr text_map;
+    PropagatorPtr http_headers;
 };
 
 class TracerShim : public opentracing::Tracer
 {
 public:
-
   /**
    * Creates a {@code opentracing::Tracer} shim out of 
    * {@code Provider::GetTracerProvider()} and 
@@ -66,7 +65,7 @@ class TracerShim : public opentracing::Tracer
   {
     return std::shared_ptr<opentracing::Tracer>(new (std::nothrow) TracerShim(tracer, propagators));
   }
-
+  // Overrides
   std::unique_ptr<opentracing::Span> StartSpanWithOptions(opentracing::string_view operation_name, 
                                                           const opentracing::StartSpanOptions& options) const noexcept override;
   opentracing::expected<void> Inject(const opentracing::SpanContext& sc,
@@ -81,19 +80,16 @@ class TracerShim : public opentracing::Tracer
   inline void Close() noexcept override { is_closed_ = true; };
 
 private:
-
   explicit TracerShim(const TracerPtr& tracer, const OpenTracingPropagators& propagators)
     : tracer_(tracer), propagators_(propagators) {}
-
   template <typename T>
   opentracing::expected<void> injectImpl(const opentracing::SpanContext& sc, 
                                          const T& writer, 
                                          const PropagatorPtr& propagator) const;
-
   template <typename T>
   opentracing::expected<std::unique_ptr<opentracing::SpanContext>> extractImpl(const T& reader, 
                                                                                const PropagatorPtr& propagator) const;
-  
+
   TracerPtr tracer_;
   OpenTracingPropagators propagators_;
   bool is_closed_ = false;

diff --git a/opentracing-shim/src/span_context_shim.cc b/opentracing-shim/src/span_context_shim.cc
@@ -6,10 +6,10 @@
 #include "span_context_shim.h"
 
 OPENTELEMETRY_BEGIN_NAMESPACE
-namespace opentracingshim 
+namespace opentracingshim
 {
 
-SpanContextShim SpanContextShim::newWithKeyValue(const nostd::string_view &key, const nostd::string_view &value) const noexcept
+SpanContextShim SpanContextShim::newWithKeyValue(nostd::string_view key, nostd::string_view value) const noexcept
 {
   return SpanContextShim{context_, baggage_->Set(key, value)};
 }
@@ -22,8 +22,8 @@ bool SpanContextShim::BaggageItem(nostd::string_view key, std::string& value) co
 void SpanContextShim::ForeachBaggageItem(VisitBaggageItem f) const
 {
   baggage_->GetAllEntries([&f](nostd::string_view key, nostd::string_view value)
-    { 
-      return f(key.data(), value.data()); 
+    {
+      return f(key.data(), value.data());
     }
   );
 }

diff --git a/opentracing-shim/src/span_shim.cc b/opentracing-shim/src/span_shim.cc
@@ -13,29 +13,35 @@
 #include <opentracing/ext/tags.h>
 
 OPENTELEMETRY_BEGIN_NAMESPACE
-namespace opentracingshim 
+namespace opentracingshim
 {
 
 void SpanShim::handleError(const opentracing::Value& value) noexcept
 {
   using opentelemetry::trace::StatusCode;
-
+  // The error tag MUST be mapped to StatusCode:
+  // - true maps to Error.
+  // - false maps to Ok
+  // - no value being set maps to Unset.
   auto code = StatusCode::kUnset;
   const auto& str_value = shimutils::stringFromValue(value);
-  if (str_value == "true") 
+
+  if (str_value == "true")
   {
     code = StatusCode::kError;
   }
-  else if (str_value == "false") 
+  else if (str_value == "false")
   {
     code = StatusCode::kOk;
   }
 
   span_->SetStatus(code);
-} 
+}
 
 void SpanShim::FinishWithOptions(const opentracing::FinishSpanOptions& finish_span_options) noexcept
 {
+  // If an explicit timestamp is specified, a conversion MUST
+  // be done to match the OpenTracing and OpenTelemetry units.
   span_->End({{ finish_span_options.finish_steady_timestamp }});
 }
 
@@ -46,6 +52,7 @@ void SpanShim::SetOperationName(opentracing::string_view name) noexcept
 
 void SpanShim::SetTag(opentracing::string_view key, const opentracing::Value& value) noexcept
 {
+  // Calls Set Attribute on the underlying OpenTelemetry Span with the specified key/value pair.
   if (key == opentracing::ext::error)
   {
     handleError(value);
@@ -58,72 +65,89 @@ void SpanShim::SetTag(opentracing::string_view key, const opentracing::Value& va
 
 void SpanShim::SetBaggageItem(opentracing::string_view restricted_key, opentracing::string_view value) noexcept
 {
+  // Creates a new SpanContext Shim with a new OpenTelemetry Baggage containing the specified
+  // Baggage key/value pair, and sets it as the current instance for this Span Shim.
   const std::lock_guard<decltype(context_lock_)> guard(context_lock_);
-  context_ = context_.newWithKeyValue(restricted_key.data(), value.data());
+  context_ = context_.newWithKeyValue(restricted_key.data(), value.data());;
 }
 
 std::string SpanShim::BaggageItem(opentracing::string_view restricted_key) const noexcept
 {
+  // Returns the value for the specified key in the OpenTelemetry Baggage
+  // of the current SpanContext Shim, or null if none exists.
   const std::lock_guard<decltype(context_lock_)> guard(context_lock_);
   std::string value;
   return context_.BaggageItem(restricted_key.data(), value) ? value : "";
 }
 
 void SpanShim::Log(std::initializer_list<EventEntry> fields) noexcept
 {
+  // If an explicit timestamp is specified, a conversion MUST
+  // be done to match the OpenTracing and OpenTelemetry units.
   logImpl(opentracing::SystemTime::min(), fields);
 }
 
 void SpanShim::Log(opentracing::SystemTime timestamp, std::initializer_list<EventEntry> fields) noexcept
 {
+  // If an explicit timestamp is specified, a conversion MUST
+  // be done to match the OpenTracing and OpenTelemetry units.
   logImpl(timestamp, fields);
 }
 
 void SpanShim::Log(opentracing::SystemTime timestamp, const std::vector<EventEntry>& fields) noexcept
 {
+  // If an explicit timestamp is specified, a conversion MUST
+  // be done to match the OpenTracing and OpenTelemetry units.
   logImpl(timestamp, fields);
 }
 
-void SpanShim::logImpl(opentracing::SystemTime timestamp, nostd::span<const EventEntry> fields) noexcept 
+void SpanShim::logImpl(opentracing::SystemTime timestamp, nostd::span<const EventEntry> fields) noexcept
 {
-  const auto event = std::find_if(fields.begin(), fields.end(), [](EventEntry item){ return item.first == "event"; });  
+  // The Add Event’s name parameter MUST be the value with the event key
+  // in the pair set, or else fallback to use the log literal string.
+  const auto event = std::find_if(fields.begin(), fields.end(), [](EventEntry item){ return item.first == "event"; });
   auto name = (event != fields.end()) ? shimutils::stringFromValue(event->second) : std::string{"log"};
-
+  // If pair set contains an event=error entry, the values MUST be mapped to an Event
+  // with the conventions outlined in the Exception semantic conventions document:
   bool is_error = (name == opentracing::ext::error);
+  // A call to AddEvent is performed with name being set to exception
   if (is_error) name = "exception";
-
-  std::vector<std::pair<nostd::string_view, common::AttributeValue>> attributes;
+  // Along the specified key/value pair set as additional event attributes...
+  std::vector<std::pair<std::string, common::AttributeValue>> attributes;
   attributes.reserve(fields.size());
-  
-  for (const auto& entry : fields) 
+
+  for (const auto& entry : fields)
   {
     auto key = entry.first;
     const auto& value = shimutils::attributeFromValue(entry.second);
-
-    if (is_error) 
+    // ... including mapping of the following key/value pairs:
+    // - error.kind maps to exception.type.
+    // - message maps to exception.message.
+    // - stack maps to exception.stacktrace.
+    if (is_error)
     {
-      if (key == "error.kind") 
+      if (key == "error.kind")
       {
         key = opentelemetry::trace::SemanticConventions::kExceptionType;
-      } 
-      else if (key == "message") 
+      }
+      else if (key == "message")
       {
         key = opentelemetry::trace::SemanticConventions::kExceptionMessage;
-      } 
-      else if (key == "stack") 
+      }
+      else if (key == "stack")
       {
         key = opentelemetry::trace::SemanticConventions::kExceptionStacktrace;
       }
     }
-    
+
     attributes.emplace_back(key, value);
   }
-
-  if (timestamp != opentracing::SystemTime::min()) 
+  // Calls Add Events on the underlying OpenTelemetry Span with the specified key/value pair set.
+  if (timestamp != opentracing::SystemTime::min())
   {
     span_->AddEvent(name, timestamp, attributes);
-  } 
-  else 
+  }
+  else
   {
     span_->AddEvent(name, attributes);
   }