From 60f6920acb94089c903aaa9c822c7cbb7125554c Mon Sep 17 00:00:00 2001
From: Nadia Ciobanu <nadiaciobanu@google.com>
Date: Mon, 27 Jul 2020 13:40:11 -0700
Subject: [PATCH] Implement links and event attributes in OTLP Recordable
 (#210)

---
 exporters/otlp/src/recordable.cc       | 27 +++++++++++++----
 exporters/otlp/test/recordable_test.cc | 41 +++++++++++++++++++++++++-
 2 files changed, 61 insertions(+), 7 deletions(-)

diff --git a/exporters/otlp/src/recordable.cc b/exporters/otlp/src/recordable.cc
index c721313ce65..4d19e3ecd1d 100644
--- a/exporters/otlp/src/recordable.cc
+++ b/exporters/otlp/src/recordable.cc
@@ -18,8 +18,9 @@ void Recordable::SetIds(trace::TraceId trace_id,
                            trace::SpanId::kSize);
 }
 
-void Recordable::SetAttribute(nostd::string_view key,
-                              const opentelemetry::common::AttributeValue &value) noexcept
+void PopulateAttribute(opentelemetry::proto::common::v1::KeyValue *attribute,
+                       nostd::string_view key,
+                       const opentelemetry::common::AttributeValue &value)
 {
   // Assert size of variant to ensure that this method gets updated if the variant
   // definition changes
@@ -27,7 +28,6 @@ void Recordable::SetAttribute(nostd::string_view key,
       nostd::variant_size<opentelemetry::common::AttributeValue>::value == kAttributeValueSize,
       "AttributeValue contains unknown type");
 
-  auto *attribute = span_.add_attributes();
   attribute->set_key(key.data(), key.size());
 
   if (nostd::holds_alternative<bool>(value))
@@ -111,6 +111,13 @@ void Recordable::SetAttribute(nostd::string_view key,
   }
 }
 
+void Recordable::SetAttribute(nostd::string_view key,
+                              const opentelemetry::common::AttributeValue &value) noexcept
+{
+  auto *attribute = span_.add_attributes();
+  PopulateAttribute(attribute, key, value);
+}
+
 void Recordable::AddEvent(nostd::string_view name,
                           core::SystemTimestamp timestamp,
                           const trace::KeyValueIterable &attributes) noexcept
@@ -118,14 +125,22 @@ void Recordable::AddEvent(nostd::string_view name,
   auto *event = span_.add_events();
   event->set_name(name.data(), name.size());
   event->set_time_unix_nano(timestamp.time_since_epoch().count());
-  // TODO: handle attributes
+
+  attributes.ForEachKeyValue([&](nostd::string_view key, common::AttributeValue value) noexcept {
+    PopulateAttribute(event->add_attributes(), key, value);
+    return true;
+  });
 }
 
 void Recordable::AddLink(opentelemetry::trace::SpanContext span_context,
                          const trace::KeyValueIterable &attributes) noexcept
 {
-  (void)span_context;
-  (void)attributes;
+  auto *link = span_.add_links();
+  attributes.ForEachKeyValue([&](nostd::string_view key, common::AttributeValue value) noexcept {
+    PopulateAttribute(link->add_attributes(), key, value);
+    return true;
+  });
+  // TODO: Populate trace_id, span_id and trace_state when these are supported by SpanContext
 }
 
 void Recordable::SetStatus(trace::CanonicalCode code, nostd::string_view description) noexcept
diff --git a/exporters/otlp/test/recordable_test.cc b/exporters/otlp/test/recordable_test.cc
index ec1662662db..5a6eec8c3ae 100644
--- a/exporters/otlp/test/recordable_test.cc
+++ b/exporters/otlp/test/recordable_test.cc
@@ -78,7 +78,7 @@ TEST(Recordable, SetStatus)
   EXPECT_EQ(rec.span().status().message(), description);
 }
 
-TEST(Recordable, AddEvents)
+TEST(Recordable, AddEventDefault)
 {
   Recordable rec;
   nostd::string_view name = "Test Event";
@@ -93,6 +93,45 @@ TEST(Recordable, AddEvents)
 
   EXPECT_EQ(rec.span().events(0).name(), name);
   EXPECT_EQ(rec.span().events(0).time_unix_nano(), unix_event_time);
+  EXPECT_EQ(rec.span().events(0).attributes().size(), 0);
+}
+
+TEST(Recordable, AddEventWithAttributes)
+{
+  Recordable rec;
+  const int kNumAttributes              = 3;
+  std::string keys[kNumAttributes]      = {"attr1", "attr2", "attr3"};
+  int values[kNumAttributes]            = {4, 7, 23};
+  std::map<std::string, int> attributes = {
+      {keys[0], values[0]}, {keys[1], values[1]}, {keys[2], values[2]}};
+
+  rec.AddEvent("Test Event", std::chrono::system_clock::now(),
+               trace::KeyValueIterableView<std::map<std::string, int>>(attributes));
+
+  for (int i = 0; i < kNumAttributes; i++)
+  {
+    EXPECT_EQ(rec.span().events(0).attributes(i).key(), keys[i]);
+    EXPECT_EQ(rec.span().events(0).attributes(i).value().int_value(), values[i]);
+  }
+}
+
+TEST(Recordable, AddLink)
+{
+  Recordable rec;
+  const int kNumAttributes              = 3;
+  std::string keys[kNumAttributes]      = {"attr1", "attr2", "attr3"};
+  int values[kNumAttributes]            = {5, 12, 40};
+  std::map<std::string, int> attributes = {
+      {keys[0], values[0]}, {keys[1], values[1]}, {keys[2], values[2]}};
+
+  rec.AddLink(trace::SpanContext(false, false),
+              trace::KeyValueIterableView<std::map<std::string, int>>(attributes));
+
+  for (int i = 0; i < kNumAttributes; i++)
+  {
+    EXPECT_EQ(rec.span().links(0).attributes(i).key(), keys[i]);
+    EXPECT_EQ(rec.span().links(0).attributes(i).value().int_value(), values[i]);
+  }
 }
 
 // Test non-int single types. Int single types are tested using templates (see IntAttributeTest)