Merge branch 'master' of https://github.com/open-telemetry/openteleme…

…try-cpp into origin/propagation

# Conflicts:
#	api/include/opentelemetry/trace/trace_state.h
#	api/test/trace/trace_state_test.cc

.github/workflows/ci.yml

@@ -71,6 +71,19 @@ jobs:
     - name: run tests
       run: ./ci/do_ci.sh bazel.test
 
+  bazel_valgrind:
+    name: Bazel valgrind
+    runs-on: ubuntu-latest
+    steps:
+    - uses: actions/checkout@v2
+    - name: setup
+      run: |
+        sudo ./ci/setup_cmake.sh
+        sudo ./ci/setup_ci_environment.sh
+        sudo ./ci/install_bazelisk.sh
+    - name: run tests
+      run: ./ci/do_ci.sh bazel.valgrind
+
   bazel_noexcept:
     name: Bazel noexcept
     runs-on: ubuntu-latest

api/include/opentelemetry/trace/trace_state.h

@@ -45,51 +45,151 @@ class TraceState
   class Entry
   {
   public:
-    Entry() noexcept = default;
-
-    // Copy constructor.
-    Entry(const Entry &copy);
+    Entry() : key_(nullptr), value_(nullptr){};
+
+    // Copy constructor
+    Entry(const Entry &copy)
+    {
+      key_   = CopyStringToPointer(copy.key_.get());
+      value_ = CopyStringToPointer(copy.value_.get());
+    }
+
+    // Copy assignment operator
+    Entry &operator=(Entry &other)
+    {
+      key_   = CopyStringToPointer(other.key_.get());
+      value_ = CopyStringToPointer(other.value_.get());
+      return *this;
+    }
+
+    // Move contructor and assignment operator
+    Entry(Entry &&other) = default;
+    Entry &operator=(Entry &&other) = default;
 
     // Creates an Entry for a given key-value pair.
-    Entry(nostd::string_view key, nostd::string_view value) noexcept;
+    Entry(nostd::string_view key, nostd::string_view value) noexcept
+    {
+      key_   = CopyStringToPointer(key);
+      value_ = CopyStringToPointer(value);
+    }
+
+    // Gets the key associated with this entry.
+    nostd::string_view GetKey() const { return key_.get(); }
+
+    // Gets the value associated with this entry.
+    nostd::string_view GetValue() const { return value_.get(); }
 
-    nostd::string_view GetKey();
-    nostd::string_view GetValue();
+    // Sets the value for this entry. This overrides the previous value.
+    void SetValue(nostd::string_view value) { value_ = CopyStringToPointer(value); }
 
   private:
     // Store key and value as raw char pointers to avoid using std::string.
     nostd::unique_ptr<const char[]> key_;
     nostd::unique_ptr<const char[]> value_;
+
+    // Copies string into a buffer and returns a unique_ptr to the buffer.
+    // This is a workaround for the fact that memcpy doesn't accept a const destination.
+    nostd::unique_ptr<const char[]> CopyStringToPointer(nostd::string_view str)
+    {
+      char *temp = new char[str.size() + 1];
+      memcpy(temp, str.data(), str.size());
+      temp[str.size()] = '\0';
+      return nostd::unique_ptr<const char[]>(temp);
+    }
   };
 
   // An empty TraceState.
-  TraceState() noexcept : num_entries_(0) {}
+  TraceState() noexcept : entries_(new Entry[kMaxKeyValuePairs]), num_entries_(0) {}
 
-  // Returns false if no such key, otherwise returns true and populates value.
-  bool Get(nostd::string_view key, nostd::string_view value) const noexcept { return false; }
+  // Returns false if no such key, otherwise returns true and populates the value parameter with the
+  // associated value.
+  bool Get(nostd::string_view key, nostd::string_view &value) const noexcept
+  {
+    for (const auto &entry : Entries())
+    {
+      if (key == entry.GetKey())
+      {
+        value = entry.GetValue();
+        return true;
+      }
+    }
+    return false;
+  }
+
+  // Creates an Entry for the key-value pair and adds it to entries. Returns true if pair was added
+  // succesfully, false otherwise. If value is null or entries_ is full, this function is a no-op.
+  bool Set(nostd::string_view key, nostd::string_view value) noexcept
+  {
+    if (value.empty() || num_entries_ >= kMaxKeyValuePairs)
+    {
+      return false;
+    }
 
-  // Creates an Entry for the key-value pair and adds it to entries.
-  // If value is null, this function is a no-op.
-  void Set(nostd::string_view key, nostd::string_view value) const noexcept;
+    Entry entry(key, value);
+    (entries_.get())[num_entries_] = entry;
+    num_entries_++;
+    return true;
+  }
 
   // Returns true if there are no keys, false otherwise.
-  bool Empty() const noexcept { return true; }
+  bool Empty() const noexcept { return num_entries_ == 0; }
 
   // Returns a span of all the entries. The TraceState object must outlive the span.
-  nostd::span<Entry> Entries() const noexcept { return {}; }
+  nostd::span<Entry> Entries() const noexcept
+  {
+    return nostd::span<Entry>(entries_.get(), num_entries_);
+  }
 
   // Returns whether key is a valid key. See https://www.w3.org/TR/trace-context/#key
-  static bool IsValidKey(nostd::string_view key);
+  static bool IsValidKey(nostd::string_view key)
+  {
+    if (key.empty() || key.size() > kKeyMaxSize || !IsLowerCaseAlphaOrDigit(key[0]))
+    {
+      return false;
+    }
+
+    int ats = 0;
+
+    for (const char c : key)
+    {
+      if (!IsLowerCaseAlphaOrDigit(c) && c != '_' && c != '-' && c != '@' && c != '*' && c != '/')
+      {
+        return false;
+      }
+      if ((c == '@') && (++ats > 1))
+      {
+        return false;
+      }
+    }
+    return true;
+  }
 
   // Returns whether value is a valid value. See https://www.w3.org/TR/trace-context/#value
-  static bool IsValidValue(nostd::string_view value);
+  static bool IsValidValue(nostd::string_view value)
+  {
+    if (value.empty() || value.size() > kValueMaxSize)
+    {
+      return false;
+    }
+
+    for (const char c : value)
+    {
+      if (c < ' ' || c > '~' || c == ',' || c == '=')
+      {
+        return false;
+      }
+    }
+    return true;
+  }
 
 private:
   // Store entries in a C-style array to avoid using std::array or std::vector.
-  Entry entries_[kMaxKeyValuePairs];
+  nostd::unique_ptr<Entry[]> entries_;
 
   // Maintain the number of entries in entries_. Must be in the range [0, kMaxKeyValuePairs].
   int num_entries_;
+
+  static bool IsLowerCaseAlphaOrDigit(char c) { return isdigit(c) || islower(c); }
 };
 
 }  // namespace trace

api/test/trace/trace_state_test.cc

@@ -1,19 +1,188 @@
-//  The commented out code below are ones that are related to TraceState. Needs to be uncommented
-//  after TraceState is merged.
-//#include "opentelemetry/trace/trace_state.h"
-//
-//#include <gtest/gtest.h>
-//
-// namespace
-//{
-//
-// using opentelemetry::trace::TraceState;
-//
-// TEST(TraceStateTest, DefaultConstruction)
-//{
-//  TraceState s;
-//  EXPECT_FALSE(s.Get("missing_key", "value"));
-//  EXPECT_TRUE(s.Empty());
-//  EXPECT_EQ(0, s.Entries().size());
-//}
-//}  // namespace
+#include "opentelemetry/trace/trace_state.h"
+
+#include <gtest/gtest.h>
+
+namespace
+{
+
+using opentelemetry::trace::TraceState;
+
+// Random string of length 257. Used for testing strings with max length 256.
+const char *kLongString =
+    "4aekid3he76zgytjavudqqeltyvu5zqio2lx7d92dlxlf0z4883irvxuwelsq27sx1mlrjg3r7ad3jeq09rjppyd9veorg"
+    "2nmihy4vilabfts8bsxruih0urusmjnglzl3iwpjinmo835dbojcrd73p56nw80v4xxrkye59ytmu5v84ysfa24d58ovv9"
+    "w1n54n0mhhf4z0mpv6oudywrp9vfoks6lrvxv3uihvbi2ihazf237kvt1nbsjn3kdvfdb";
+
+// ------------------------- Entry class tests ---------------------------------
+
+// Test constructor that takes a key-value pair
+TEST(EntryTest, KeyValueConstruction)
+{
+  opentelemetry::nostd::string_view key = "test_key";
+  opentelemetry::nostd::string_view val = "test_value";
+  TraceState::Entry e(key, val);
+
+  EXPECT_EQ(key.size(), e.GetKey().size());
+  EXPECT_EQ(strcmp(key.data(), e.GetKey().data()), 0);
+
+  EXPECT_EQ(val.size(), e.GetValue().size());
+  EXPECT_EQ(strcmp(val.data(), e.GetValue().data()), 0);
+}
+
+// Test copy constructor
+TEST(EntryTest, Copy)
+{
+  TraceState::Entry e("test_key", "test_value");
+  TraceState::Entry copy(e);
+  EXPECT_EQ(strcmp(copy.GetKey().data(), e.GetKey().data()), 0);
+  EXPECT_EQ(strcmp(copy.GetValue().data(), e.GetValue().data()), 0);
+}
+
+// Test assignment operator
+TEST(EntryTest, Assignment)
+{
+  TraceState::Entry e("test_key", "test_value");
+  TraceState::Entry empty;
+  empty = e;
+  EXPECT_EQ(strcmp(empty.GetKey().data(), e.GetKey().data()), 0);
+  EXPECT_EQ(strcmp(empty.GetValue().data(), e.GetValue().data()), 0);
+}
+
+TEST(EntryTest, SetValue)
+{
+  TraceState::Entry e("test_key", "test_value");
+  opentelemetry::nostd::string_view new_val = "new_value";
+  e.SetValue(new_val);
+
+  EXPECT_EQ(new_val.size(), e.GetValue().size());
+  EXPECT_EQ(strcmp(new_val.data(), e.GetValue().data()), 0);
+}
+
+// -------------------------- TraceState class tests ---------------------------
+
+TEST(TraceStateTest, DefaultConstruction)
+{
+  TraceState s;
+  opentelemetry::nostd::string_view return_val = "";
+  EXPECT_FALSE(s.Get("missing_key", return_val));
+  EXPECT_EQ(return_val.data(), "");
+  EXPECT_TRUE(s.Empty());
+  EXPECT_EQ(0, s.Entries().size());
+}
+
+TEST(TraceStateTest, Set)
+{
+  TraceState s;
+  opentelemetry::nostd::string_view key = "test_key";
+  opentelemetry::nostd::string_view val = "test_value";
+  s.Set(key, val);
+
+  opentelemetry::nostd::string_view bad_key = "bad_key";
+  opentelemetry::nostd::string_view null_val;
+  // Since string_view data is null by default, this should be a no-op
+  s.Set(bad_key, null_val);
+
+  opentelemetry::nostd::span<TraceState::Entry> entries = s.Entries();
+  EXPECT_EQ(entries.size(), 1);
+  EXPECT_EQ(entries[0].GetKey().data(), key);
+  EXPECT_EQ(entries[0].GetValue().data(), val);
+}
+
+TEST(TraceStateTest, Get)
+{
+  TraceState s;
+  const int kNumPairs                                 = 3;
+  opentelemetry::nostd::string_view keys[kNumPairs]   = {"test_key_1", "test_key_2", "test_key_3"};
+  opentelemetry::nostd::string_view values[kNumPairs] = {"test_val_1", "test_val_2", "test_val_3"};
+
+  for (int i = 0; i < kNumPairs; i++)
+  {
+    s.Set(keys[i], values[i]);
+  }
+
+  opentelemetry::nostd::string_view return_val = "";
+
+  for (int i = 0; i < kNumPairs; i++)
+  {
+    EXPECT_TRUE(s.Get(keys[i], return_val));
+    EXPECT_EQ(return_val.data(), values[i]);
+    return_val = "";
+  }
+
+  EXPECT_FALSE(s.Get("fake_key", return_val));
+  EXPECT_EQ(return_val.data(), "");
+}
+
+TEST(TraceStateTest, Empty)
+{
+  TraceState s;
+  EXPECT_TRUE(s.Empty());
+
+  s.Set("test_key", "test_value");
+  EXPECT_FALSE(s.Empty());
+}
+
+TEST(TraceStateTest, Entries)
+{
+  TraceState s;
+  const int kNumPairs                                 = 3;
+  opentelemetry::nostd::string_view keys[kNumPairs]   = {"test_key_1", "test_key_2", "test_key_3"};
+  opentelemetry::nostd::string_view values[kNumPairs] = {"test_val_1", "test_val_2", "test_val_3"};
+
+  for (int i = 0; i < kNumPairs; i++)
+  {
+    s.Set(keys[i], values[i]);
+  }
+
+  opentelemetry::nostd::span<TraceState::Entry> entries = s.Entries();
+  for (int i = 0; i < kNumPairs; i++)
+  {
+    EXPECT_EQ(entries[i].GetKey().data(), keys[i]);
+    EXPECT_EQ(entries[i].GetValue().data(), values[i]);
+  }
+}
+
+TEST(TraceStateTest, IsValidKey)
+{
+  EXPECT_TRUE(TraceState::IsValidKey("valid-key23/*"));
+  EXPECT_FALSE(TraceState::IsValidKey("Invalid_key"));
+  EXPECT_FALSE(TraceState::IsValidKey("invalid$Key&"));
+  EXPECT_FALSE(TraceState::IsValidKey(""));
+  EXPECT_FALSE(TraceState::IsValidKey(kLongString));
+}
+
+TEST(TraceStateTest, IsValidValue)
+{
+  EXPECT_TRUE(TraceState::IsValidValue("valid-val$%&~"));
+  EXPECT_FALSE(TraceState::IsValidValue("\tinvalid"));
+  EXPECT_FALSE(TraceState::IsValidValue("invalid="));
+  EXPECT_FALSE(TraceState::IsValidValue("invalid,val"));
+  EXPECT_FALSE(TraceState::IsValidValue(""));
+  EXPECT_FALSE(TraceState::IsValidValue(kLongString));
+}
+
+// Tests that keys and values don't depend on null terminators
+TEST(TraceStateTest, MemorySafe)
+{
+  TraceState s;
+  const int kNumPairs                               = 3;
+  opentelemetry::nostd::string_view key_string      = "test_key_1test_key_2test_key_3";
+  opentelemetry::nostd::string_view val_string      = "test_val_1test_val_2test_val_3";
+  opentelemetry::nostd::string_view keys[kNumPairs] = {
+      key_string.substr(0, 10), key_string.substr(10, 10), key_string.substr(20, 10)};
+  opentelemetry::nostd::string_view values[kNumPairs] = {
+      val_string.substr(0, 10), val_string.substr(10, 10), val_string.substr(20, 10)};
+
+  for (int i = 0; i < kNumPairs; i++)
+  {
+    s.Set(keys[i], values[i]);
+  }
+
+  opentelemetry::nostd::span<TraceState::Entry> entries = s.Entries();
+  for (int i = 0; i < kNumPairs; i++)
+  {
+    EXPECT_EQ(entries[i].GetKey(), keys[i]);
+    EXPECT_EQ(entries[i].GetValue(), values[i]);
+  }
+}
+}  // namespace