Limit streaming trace metrics struct to float

cloudflare · Oct 16, 2024 · 374ad5f · 374ad5f
1 parent e434684
commit 374ad5f
Show file tree

Hide file tree

Showing 5 changed files with 16 additions and 65 deletions.
diff --git a/src/workerd/io/trace-common-test.c++ b/src/workerd/io/trace-common-test.c++
@@ -499,7 +499,7 @@ KJ_TEST("Metric works") {
   trace::Metric metric(trace::Metric::Type::COUNTER, kj::str("foo"), 1.0);
   KJ_EXPECT(metric.type == trace::Metric::Type::COUNTER);
   KJ_EXPECT(KJ_ASSERT_NONNULL(metric.key.tryGet<kj::String>()) == "foo"_kj);
-  KJ_EXPECT(KJ_ASSERT_NONNULL(metric.value.tryGet<double>()) == 1.0);
+  KJ_EXPECT(metric.value == 1.0);
   KJ_EXPECT(metric.keyMatches("foo"_kj));
 
   enum class Foo { A };
@@ -513,12 +513,12 @@ KJ_TEST("Metric works") {
   trace::Metric metric2(reader);
   KJ_EXPECT(metric2.type == trace::Metric::Type::COUNTER);
   KJ_EXPECT(KJ_ASSERT_NONNULL(metric2.key.tryGet<kj::String>()) == "foo"_kj);
-  KJ_EXPECT(KJ_ASSERT_NONNULL(metric2.value.tryGet<double>()) == 1.0);
+  KJ_EXPECT(metric2.value == 1.0);
 
   auto metric3 = metric.clone();
   KJ_EXPECT(metric3.type == trace::Metric::Type::COUNTER);
   KJ_EXPECT(KJ_ASSERT_NONNULL(metric3.key.tryGet<kj::String>()) == "foo"_kj);
-  KJ_EXPECT(KJ_ASSERT_NONNULL(metric3.value.tryGet<double>()) == 1.0);
+  KJ_EXPECT(metric3.value == 1.0);
 }
 
 KJ_TEST("Dropped works") {

diff --git a/src/workerd/io/trace-common.c++ b/src/workerd/io/trace-common.c++
@@ -1093,33 +1093,17 @@ Metric::Key getMetricKey(const rpc::Trace::Metric::Reader& reader) {
   }
   KJ_UNREACHABLE;
 }
-
-Metric::Value getMetricValue(const rpc::Trace::Metric::Reader& reader) {
-  auto value = reader.getValue();
-  switch (value.which()) {
-    case rpc::Trace::Metric::Value::Which::FLOAT64: {
-      return value.getFloat64();
-    }
-    case rpc::Trace::Metric::Value::Which::INT64: {
-      return value.getInt64();
-    }
-    case rpc::Trace::Metric::Value::Which::UINT64: {
-      return value.getUint64();
-    }
-  }
-  KJ_UNREACHABLE;
-}
 }  // namespace
 
-Metric::Metric(Type type, Key key, Value value)
+Metric::Metric(Type type, Key key, double value)
     : type(type),
       key(kj::mv(key)),
       value(kj::mv(value)) {}
 
 Metric::Metric(rpc::Trace::Metric::Reader reader)
     : type(reader.getType()),
       key(getMetricKey(reader)),
-      value(getMetricValue(reader)) {}
+      value(reader.getValue()) {}
 
 void Metric::copyTo(rpc::Trace::Metric::Builder builder) const {
   builder.setType(type);
@@ -1131,17 +1115,7 @@ void Metric::copyTo(rpc::Trace::Metric::Builder builder) const {
       builder.getKey().setId(id);
     }
   }
-  KJ_SWITCH_ONEOF(value) {
-    KJ_CASE_ONEOF(d, double) {
-      builder.getValue().setFloat64(d);
-    }
-    KJ_CASE_ONEOF(i, int64_t) {
-      builder.getValue().setInt64(i);
-    }
-    KJ_CASE_ONEOF(u, uint64_t) {
-      builder.getValue().setUint64(u);
-    }
-  }
+  builder.setValue(value);
 }
 
 Metric Metric::clone() const {
@@ -1156,21 +1130,7 @@ Metric Metric::clone() const {
     }
     KJ_UNREACHABLE;
   })();
-  auto newValue = ([&]() -> Value {
-    KJ_SWITCH_ONEOF(value) {
-      KJ_CASE_ONEOF(d, double) {
-        return d;
-      }
-      KJ_CASE_ONEOF(i, int64_t) {
-        return i;
-      }
-      KJ_CASE_ONEOF(u, uint64_t) {
-        return u;
-      }
-    }
-    KJ_UNREACHABLE;
-  })();
-  return Metric(type, kj::mv(newKey), kj::mv(newValue));
+  return Metric(type, kj::mv(newKey), value);
 }
 
 bool Metric::keyMatches(kj::OneOf<kj::StringPtr, uint32_t> check) {

diff --git a/src/workerd/io/trace-common.h b/src/workerd/io/trace-common.h
@@ -568,19 +568,18 @@ struct Mark final {
 // metrics to be emitted. This is a new feature in the streaming trace model.
 struct Metric final {
   using Key = kj::OneOf<kj::String, uint32_t>;
-  using Value = kj::OneOf<double, int64_t, uint64_t>;
   using Type = rpc::Trace::Metric::Type;
 
   enum class Common {
     CPU_TIME,
     WALL_TIME,
   };
 
-  explicit Metric(Type type, Key key, Value value);
+  explicit Metric(Type type, Key key, double value);
 
   template <IsEnum K>
-  explicit Metric(Type type, K key, Value value)
-      : Metric(type, static_cast<uint32_t>(key), kj::mv(value)) {}
+  explicit Metric(Type type, K key, double value)
+      : Metric(type, static_cast<uint32_t>(key), value) {}
 
   Metric(rpc::Trace::Metric::Reader reader);
   Metric(Metric&&) = default;
@@ -589,7 +588,7 @@ struct Metric final {
 
   Type type;
   Key key;
-  Value value;
+  double value;
 
   bool keyMatches(kj::OneOf<kj::StringPtr, uint32_t> key);
 

diff --git a/src/workerd/io/trace-legacy.c++ b/src/workerd/io/trace-legacy.c++
@@ -375,15 +375,11 @@ void Trace::addMetrics(trace::Metrics&& metrics) {
     if (metric.keyMatches(trace::Metric::Common::CPU_TIME)) {
       // The CPU_TIME metric will always be a int64_t converted from a kj::Duration
       // If it's not, we'll ignore it.
-      KJ_IF_SOME(i, metric.value.tryGet<int64_t>()) {
-        cpuTime = i * kj::MILLISECONDS;
-      }
+      cpuTime = static_cast<int64_t>(metric.value) * kj::MILLISECONDS;
     } else if (metric.keyMatches(trace::Metric::Common::WALL_TIME)) {
       // The WALL_TIME metric will always be a int64_t converted from a kj::Duration
       // If it's not, we'll ignore it.
-      KJ_IF_SOME(i, metric.value.tryGet<int64_t>()) {
-        wallTime = i * kj::MILLISECONDS;
-      }
+      wallTime = static_cast<int64_t>(metric.value) * kj::MILLISECONDS;
     }
   }
 }

diff --git a/src/workerd/io/worker-interface.capnp b/src/workerd/io/worker-interface.capnp
@@ -349,13 +349,9 @@ struct Trace @0x8e8d911203762d34 {
       # a snapshot of a value at a given point in time and therefore may increase or decrease.
     }
     type @2 :Type;
-    value :union {
-      float64 @3 :Float64;
-      int64 @4 :Int64;
-      uint64 @5 :UInt64;
-    }
-    unit @6 :Text;
-    tags @7 :List(Tag);
+    value @3 :Float64;
+    unit @4 :Text;
+    tags @5 :List(Tag);
   }
 
   struct Dropped {