Support passing attributes with AddEvent

.circleci/config.yml

@@ -123,7 +123,9 @@ jobs:
       - checkout
       - run: ./ci/setup_windows_cmake.ps1
       - run: ./ci/setup_windows_ci_environment.ps1
-      - run: ./ci/install_windows_protobuf.ps1
+      - run:
+          command: ./ci/install_windows_protobuf.ps1
+          no_output_timeout: 15m
       - run: ./ci/do_ci.ps1 cmake.test
       - run: ./ci/do_ci.sh cmake.exporter.otprotocol.test
 

api/include/opentelemetry/common/attribute_value.h

@@ -0,0 +1,28 @@
+#pragma once
+
+#include <cstdint>
+
+#include "opentelemetry/nostd/span.h"
+#include "opentelemetry/nostd/string_view.h"
+#include "opentelemetry/nostd/variant.h"
+#include "opentelemetry/version.h"
+
+OPENTELEMETRY_BEGIN_NAMESPACE
+namespace common
+{
+using AttributeValue = nostd::variant<bool,
+                                      int,
+                                      int64_t,
+                                      unsigned int,
+                                      uint64_t,
+                                      double,
+                                      nostd::string_view,
+                                      nostd::span<const bool>,
+                                      nostd::span<const int>,
+                                      nostd::span<const int64_t>,
+                                      nostd::span<const unsigned int>,
+                                      nostd::span<const uint64_t>,
+                                      nostd::span<const double>,
+                                      nostd::span<const nostd::string_view>>;
+}  // namespace common
+OPENTELEMETRY_END_NAMESPACE

api/include/opentelemetry/nostd/variant.h

@@ -1140,7 +1140,7 @@ struct convert_to_bool
     static_assert(std::is_convertible<invoke_result_t<RelOp, Lhs, Rhs>, bool>::value,
                   "relational operators must return a type"
                   " implicitly convertible to bool");
-    return invoke(RelOp{}, std::forward<Lhs>(lhs), std::forward<Rhs>(rhs));
+    return nostd::invoke(RelOp{}, std::forward<Lhs>(lhs), std::forward<Rhs>(rhs));
   }
 };
 }  // namespace detail

api/include/opentelemetry/plugin/tracer.h

@@ -25,6 +25,13 @@ class Span final : public trace::Span
     span_->AddEvent(name, timestamp);
   }
 
+  void AddEvent(nostd::string_view name,
+                core::SystemTimestamp timestamp,
+                const trace::KeyValueIterable &attributes) noexcept override
+  {
+    span_->AddEvent(name, timestamp, attributes);
+  }
+
   void SetStatus(trace::CanonicalCode code, nostd::string_view description) noexcept override
   {
     span_->SetStatus(code, description);

api/include/opentelemetry/trace/key_value_iterable.h

@@ -0,0 +1,34 @@
+#pragma once
+
+#include "opentelemetry/common/attribute_value.h"
+#include "opentelemetry/nostd/function_ref.h"
+#include "opentelemetry/version.h"
+
+OPENTELEMETRY_BEGIN_NAMESPACE
+namespace trace
+{
+/**
+ * Supports internal iteration over a collection of key-value pairs.
+ */
+class KeyValueIterable
+{
+public:
+  virtual ~KeyValueIterable() = default;
+
+  /**
+   * Iterate over key-value pairs
+   * @param callback a callback to invoke for each key-value. If the callback returns false,
+   * the iteration is aborted.
+   * @return true if every key-value pair was iterated over
+   */
+  virtual bool ForEachKeyValue(
+      nostd::function_ref<bool(nostd::string_view, common::AttributeValue)> callback) const
+      noexcept = 0;
+
+  /**
+   * @return the number of key-value pairs
+   */
+  virtual size_t size() const noexcept = 0;
+};
+}  // namespace trace
+OPENTELEMETRY_END_NAMESPACE

api/include/opentelemetry/trace/key_value_iterable_view.h

@@ -0,0 +1,64 @@
+#pragma once
+
+#include <iterator>
+#include <type_traits>
+#include <utility>
+
+#include "opentelemetry/nostd/utility.h"
+#include "opentelemetry/trace/key_value_iterable.h"
+#include "opentelemetry/version.h"
+
+OPENTELEMETRY_BEGIN_NAMESPACE
+namespace trace
+{
+namespace detail
+{
+inline void take_key_value(nostd::string_view, common::AttributeValue) {}
+
+template <class T>
+auto is_key_value_iterable_impl(T iterable)
+    -> decltype(take_key_value(std::begin(iterable)->first, std::begin(iterable)->second),
+                nostd::size(iterable),
+                std::true_type{});
+
+std::false_type is_key_value_iterable_impl(...);
+
+template <class T>
+struct is_key_value_iterable
+{
+  static const bool value = decltype(detail::is_key_value_iterable_impl(std::declval<T>()))::value;
+};
+}  // namespace detail
+
+template <class T>
+class KeyValueIterableView final : public KeyValueIterable
+{
+  static_assert(detail::is_key_value_iterable<T>::value, "Must be a key-value iterable");
+
+public:
+  explicit KeyValueIterableView(const T &container) noexcept : container_{&container} {}
+
+  // KeyValueIterable
+  bool ForEachKeyValue(
+      nostd::function_ref<bool(nostd::string_view, common::AttributeValue)> callback) const
+      noexcept override
+  {
+    auto iter = std::begin(*container_);
+    auto last = std::end(*container_);
+    for (; iter != last; ++iter)
+    {
+      if (!callback(iter->first, iter->second))
+      {
+        return false;
+      }
+    }
+    return true;
+  }
+
+  size_t size() const noexcept override { return nostd::size(*container_); }
+
+private:
+  const T *container_;
+};
+}  // namespace trace
+OPENTELEMETRY_END_NAMESPACE

api/include/opentelemetry/trace/noop.h

@@ -24,13 +24,19 @@ class NoopSpan final : public Span
 public:
   explicit NoopSpan(const std::shared_ptr<Tracer> &tracer) noexcept : tracer_{tracer} {}
 
-  void AddEvent(nostd::string_view name) noexcept override {}
+  void AddEvent(nostd::string_view /*name*/) noexcept override {}
 
-  void AddEvent(nostd::string_view name, core::SystemTimestamp timestamp) noexcept override {}
+  void AddEvent(nostd::string_view /*name*/, core::SystemTimestamp /*timestamp*/) noexcept override
+  {}
+
+  void AddEvent(nostd::string_view /*name*/,
+                core::SystemTimestamp /*timestamp*/,
+                const trace::KeyValueIterable & /*attributes*/) noexcept override
+  {}
 
-  void SetStatus(CanonicalCode code, nostd::string_view description) noexcept override {}
+  void SetStatus(CanonicalCode /*code*/, nostd::string_view /*description*/) noexcept override {}
 
-  void UpdateName(nostd::string_view name) noexcept override {}
+  void UpdateName(nostd::string_view /*name*/) noexcept override {}
 
   void End() noexcept override {}
 

api/include/opentelemetry/trace/span.h

@@ -3,8 +3,10 @@
 #include <cstdint>
 
 #include "opentelemetry/core/timestamp.h"
+#include "opentelemetry/nostd/span.h"
 #include "opentelemetry/nostd/string_view.h"
 #include "opentelemetry/trace/canonical_code.h"
+#include "opentelemetry/trace/key_value_iterable_view.h"
 #include "opentelemetry/version.h"
 
 OPENTELEMETRY_BEGIN_NAMESPACE
@@ -77,11 +79,48 @@ class Span
   // Adds an event to the Span, with a custom timestamp.
   virtual void AddEvent(nostd::string_view name, core::SystemTimestamp timestamp) noexcept = 0;
 
-  // TODO
   // Adds an event to the Span, with a custom timestamp, and attributes.
-  // virtual void AddEvent(nostd::string_view name, core::SteadyTimestamp
-  // timestamp, nostd::span<const std::pair<nostd::string_view name, AttributeValue
-  // value>> attributes) noexcept = 0;
+  virtual void AddEvent(nostd::string_view name,
+                        core::SystemTimestamp timestamp,
+                        const KeyValueIterable &attributes) noexcept = 0;
+
+  virtual void AddEvent(nostd::string_view name, const KeyValueIterable &attributes) noexcept
+  {
+    this->AddEvent(name, std::chrono::system_clock::now(), attributes);
+  }
+
+  template <class T, nostd::enable_if_t<detail::is_key_value_iterable<T>::value> * = nullptr>
+  void AddEvent(nostd::string_view name,
+                core::SystemTimestamp timestamp,
+                const T &attributes) noexcept
+  {
+    this->AddEvent(name, timestamp, KeyValueIterableView<T>{attributes});
+  }
+
+  template <class T, nostd::enable_if_t<detail::is_key_value_iterable<T>::value> * = nullptr>
+  void AddEvent(nostd::string_view name, const T &attributes) noexcept
+  {
+    this->AddEvent(name, KeyValueIterableView<T>{attributes});
+  }
+
+  void AddEvent(nostd::string_view name,
+                core::SystemTimestamp timestamp,
+                std::initializer_list<std::pair<nostd::string_view, common::AttributeValue>>
+                    attributes) noexcept
+  {
+    this->AddEvent(name, timestamp,
+                   nostd::span<const std::pair<nostd::string_view, common::AttributeValue>>{
+                       attributes.begin(), attributes.end()});
+  }
+
+  void AddEvent(nostd::string_view name,
+                std::initializer_list<std::pair<nostd::string_view, common::AttributeValue>>
+                    attributes) noexcept
+  {
+    this->AddEvent(name, std::chrono::system_clock::now(),
+                   nostd::span<const std::pair<nostd::string_view, common::AttributeValue>>{
+                       attributes.begin(), attributes.end()});
+  }
 
   // Sets the status of the span. The default status is OK. Only the value of the last call will be
   // recorded, and implementations are free to ignore previous calls.

api/test/trace/BUILD

@@ -1,5 +1,16 @@
 load("//bazel:otel_cc_benchmark.bzl", "otel_cc_benchmark")
 
+cc_test(
+    name = "key_value_iterable_view_test",
+    srcs = [
+        "key_value_iterable_view_test.cc",
+    ],
+    deps = [
+        "//api",
+        "@com_google_googletest//:gtest_main",
+    ],
+)
+
 cc_test(
     name = "noop_test",
     srcs = [

api/test/trace/CMakeLists.txt

@@ -1,5 +1,5 @@
-foreach(testname noop_test provider_test span_id_test trace_id_test
-                 trace_flags_test)
+foreach(testname key_value_iterable_view_test noop_test provider_test
+                 span_id_test trace_id_test trace_flags_test)
   add_executable(${testname} "${testname}.cc")
   target_link_libraries(${testname} ${GTEST_BOTH_LIBRARIES}
                         ${CMAKE_THREAD_LIBS_INIT} opentelemetry_api)

api/test/trace/key_value_iterable_view_test.cc

@@ -0,0 +1,65 @@
+#include "opentelemetry/trace/key_value_iterable_view.h"
+
+#include <map>
+
+#include <gtest/gtest.h>
+
+using namespace opentelemetry;
+
+static int TakeKeyValues(const trace::KeyValueIterable &iterable)
+{
+  std::map<std::string, common::AttributeValue> result;
+  int count = 0;
+  iterable.ForEachKeyValue([&](nostd::string_view key, common::AttributeValue value) noexcept {
+    ++count;
+    return true;
+  });
+  return count;
+}
+
+template <class T, nostd::enable_if_t<trace::detail::is_key_value_iterable<T>::value> * = nullptr>
+static int TakeKeyValues(const T &iterable)
+{
+  return TakeKeyValues(trace::KeyValueIterableView<T>{iterable});
+}
+
+TEST(KeyValueIterableViewTest, is_key_value_iterable)
+{
+  using M1 = std::map<std::string, std::string>;
+  EXPECT_TRUE(bool{trace::detail::is_key_value_iterable<M1>::value});
+
+  using M2 = std::map<std::string, int>;
+  EXPECT_TRUE(bool{trace::detail::is_key_value_iterable<M2>::value});
+
+  using M3 = std::map<std::string, common::AttributeValue>;
+  EXPECT_TRUE(bool{trace::detail::is_key_value_iterable<M3>::value});
+
+  struct A
+  {};
+  using M4 = std::map<std::string, A>;
+  EXPECT_FALSE(bool{trace::detail::is_key_value_iterable<M4>::value});
+}
+
+TEST(KeyValueIterableViewTest, ForEachKeyValue)
+{
+  std::map<std::string, std::string> m1 = {{"abc", "123"}, {"xyz", "456"}};
+  EXPECT_EQ(TakeKeyValues(m1), 2);
+
+  std::vector<std::pair<std::string, int>> v1 = {{"abc", 123}, {"xyz", 456}};
+  EXPECT_EQ(TakeKeyValues(v1), 2);
+}
+
+TEST(KeyValueIterableViewTest, ForEachKeyValueWithExit)
+{
+  using M = std::map<std::string, std::string>;
+  M m1    = {{"abc", "123"}, {"xyz", "456"}};
+  trace::KeyValueIterableView<M> iterable{m1};
+  int count = 0;
+  auto exit = iterable.ForEachKeyValue([&count](nostd::string_view /*key*/,
+                                                common::AttributeValue /*value*/) noexcept {
+    ++count;
+    return false;
+  });
+  EXPECT_EQ(count, 1);
+  EXPECT_FALSE(exit);
+}

api/test/trace/noop_test.cc

@@ -1,6 +1,8 @@
 #include "opentelemetry/trace/noop.h"
 
+#include <map>
 #include <memory>
+#include <string>
 
 #include <gtest/gtest.h>
 
@@ -12,4 +14,15 @@ TEST(NoopTest, 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);
 }

examples/plugin/plugin/tracer.cc

@@ -27,6 +27,11 @@ class Span final : public trace::Span
   void AddEvent(nostd::string_view /*name*/, core::SystemTimestamp /*timestamp*/) noexcept override
   {}
 
+  void AddEvent(nostd::string_view /*name*/,
+                core::SystemTimestamp /*timestamp*/,
+                const trace::KeyValueIterable & /*attributes*/) noexcept override
+  {}
+
   void SetStatus(trace::CanonicalCode /*code*/,
                  nostd::string_view /*description*/) noexcept override
   {}

sdk/src/trace/span.cc

@@ -38,6 +38,15 @@ void Span::AddEvent(nostd::string_view name, core::SystemTimestamp timestamp) no
   (void)timestamp;
 }
 
+void Span::AddEvent(nostd::string_view name,
+                    core::SystemTimestamp timestamp,
+                    const trace_api::KeyValueIterable &attributes) noexcept
+{
+  (void)name;
+  (void)timestamp;
+  (void)attributes;
+}
+
 void Span::SetStatus(trace_api::CanonicalCode code, nostd::string_view description) noexcept
 {
   std::lock_guard<std::mutex> lock_guard{mu_};