From 137599c273463368c6a9dab523bf3f459aa0b09c Mon Sep 17 00:00:00 2001 From: James M Snell Date: Tue, 11 Jun 2024 12:43:09 -0700 Subject: [PATCH] Add FeatureObserver to metrics (#2167) --- src/workerd/api/blob.c++ | 9 ++++++ src/workerd/api/blob.h | 2 +- src/workerd/io/BUILD.bazel | 7 +++++ src/workerd/io/features.capnp | 32 +++++++++++++++++++++ src/workerd/io/observer-test.c++ | 26 +++++++++++++++++ src/workerd/io/observer.c++ | 49 ++++++++++++++++++++++++++++++++ src/workerd/io/observer.h | 32 +++++++++++++++++++++ 7 files changed, 156 insertions(+), 1 deletion(-) create mode 100644 src/workerd/io/features.capnp create mode 100644 src/workerd/io/observer-test.c++ create mode 100644 src/workerd/io/observer.c++ diff --git a/src/workerd/api/blob.c++ b/src/workerd/api/blob.c++ index fe00dbad390..7fb603f01ae 100644 --- a/src/workerd/api/blob.c++ +++ b/src/workerd/api/blob.c++ @@ -5,6 +5,7 @@ #include "blob.h" #include "streams.h" #include "util.h" +#include #include namespace workerd::api { @@ -160,6 +161,11 @@ jsg::Ref Blob::constructor(jsg::Lock& js, return jsg::alloc(js, concat(js, kj::mv(bits)), kj::mv(type)); } +kj::ArrayPtr Blob::getData() const { + FeatureObserver::maybeRecordUse(FeatureObserver::Feature::BLOB_GET_DATA); + return data; +} + jsg::Ref Blob::slice(jsg::Optional maybeStart, jsg::Optional maybeEnd, jsg::Optional type) { int start = maybeStart.orDefault(0); @@ -193,6 +199,7 @@ jsg::Ref Blob::slice(jsg::Optional maybeStart, jsg::Optional may jsg::Promise> Blob::arrayBuffer(jsg::Lock& js) { // TODO(perf): Find a way to avoid the copy. + FeatureObserver::maybeRecordUse(FeatureObserver::Feature::BLOB_AS_ARRAY_BUFFER); return js.resolvedPromise(kj::heapArray(data)); } @@ -201,6 +208,7 @@ jsg::Promise Blob::bytes(jsg::Lock& js) { } jsg::Promise Blob::text(jsg::Lock& js) { + FeatureObserver::maybeRecordUse(FeatureObserver::Feature::BLOB_AS_TEXT); return js.resolvedPromise(kj::str(data.asChars())); } @@ -260,6 +268,7 @@ private: }; jsg::Ref Blob::stream() { + FeatureObserver::maybeRecordUse(FeatureObserver::Feature::BLOB_AS_STREAM); return jsg::alloc( IoContext::current(), kj::heap(JSG_THIS)); diff --git a/src/workerd/api/blob.h b/src/workerd/api/blob.h index 0b2b7d9972d..9352996c35f 100644 --- a/src/workerd/api/blob.h +++ b/src/workerd/api/blob.h @@ -17,7 +17,7 @@ class Blob: public jsg::Object { Blob(jsg::Lock& js, kj::Array data, kj::String type); Blob(jsg::Ref parent, kj::ArrayPtr data, kj::String type); - inline kj::ArrayPtr getData() const KJ_LIFETIMEBOUND { return data; } + kj::ArrayPtr getData() const KJ_LIFETIMEBOUND; // --------------------------------------------------------------------------- // JS API diff --git a/src/workerd/io/BUILD.bazel b/src/workerd/io/BUILD.bazel index fd3b579eaf1..b24089b7440 100644 --- a/src/workerd/io/BUILD.bazel +++ b/src/workerd/io/BUILD.bazel @@ -173,6 +173,7 @@ wd_cc_capnp_library( visibility = ["//visibility:public"], deps = [ ":compatibility_date_capnp", + ":features_capnp", ":outcome_capnp", ":script_version_capnp", ":trimmed-supported-compatibility-date", @@ -219,6 +220,12 @@ wd_cc_capnp_library( visibility = ["//visibility:public"], ) +wd_cc_capnp_library( + name = "features_capnp", + srcs = ["features.capnp"], + visibility = ["//visibility:public"], +) + [kj_test( src = f, deps = [ diff --git a/src/workerd/io/features.capnp b/src/workerd/io/features.capnp new file mode 100644 index 00000000000..482b7ce70f3 --- /dev/null +++ b/src/workerd/io/features.capnp @@ -0,0 +1,32 @@ +# Copyright (c) 2017-2022 Cloudflare, Inc. +# Licensed under the Apache 2.0 license found in the LICENSE file or at: +# https://opensource.org/licenses/Apache-2.0 + +@0x8b3d4aaa36221ec9; + +using Cxx = import "/capnp/c++.capnp"; +$Cxx.namespace("workerd"); +$Cxx.allowCancellation; + +enum Features { + test @0; + # A test feature that should never be used in production code. + + # Due to a number of practical limitations on the metrics collection, + # we do not really want the list of features to grow unbounded over + # time. At any given point in time we shouldn't be trying to track + # more than 50 features at a time. + # + # Features we are no longer needing to track can and should be removed, + # just be careful to adjust the index ordinals of the remaining features + # correctly. In code, be sure to never rely on the ordinal value and + # instead always use the features enum to ensure that things won't break. + + # We want to determine how users typically read the data from a Blob. + # The reason is so that we can determine how best to optimize the Blob + # implementation. + blobAsArrayBuffer @1; + blobAsText @2; + blobAsStream @3; + blobGetData @4; +} diff --git a/src/workerd/io/observer-test.c++ b/src/workerd/io/observer-test.c++ new file mode 100644 index 00000000000..941c727a124 --- /dev/null +++ b/src/workerd/io/observer-test.c++ @@ -0,0 +1,26 @@ +#include "observer.h" +#include "worker-interface.h" +#include + +namespace workerd { +namespace { + +KJ_TEST("FeatureObserver") { + FeatureObserver::init(FeatureObserver::createDefault()); + + auto& observer = KJ_ASSERT_NONNULL(FeatureObserver::get()); + + observer.use(FeatureObserver::Feature::TEST); + observer.use(FeatureObserver::Feature::TEST); + observer.use(FeatureObserver::Feature::TEST); + + uint64_t count = 0; + observer.collect([&](FeatureObserver::Feature feature, const uint64_t value) { + KJ_ASSERT(feature == FeatureObserver::Feature::TEST); + count = value; + }); + KJ_ASSERT(count == 3); +} + +} // namespace +} // namespace workerd diff --git a/src/workerd/io/observer.c++ b/src/workerd/io/observer.c++ new file mode 100644 index 00000000000..50999c62a84 --- /dev/null +++ b/src/workerd/io/observer.c++ @@ -0,0 +1,49 @@ +#include "observer.h" +#include "worker-interface.h" +#include +#include +#include + +namespace workerd { + +namespace { +kj::Maybe> featureObserver; + +class FeatureObserverImpl final: public FeatureObserver { +public: + void use(Feature feature) const override { + auto lock = counts.lockExclusive(); + lock->upsert(feature, 1, [](uint64_t& count, uint64_t value) { + count += value; + }); + } + + void collect(CollectCallback&& callback) const override { + auto lock = counts.lockShared(); + for (auto& entry: *lock) { + callback(entry.key, entry.value); + } + } + +private: + kj::MutexGuarded> counts; +}; + +} // namespace + +kj::Own FeatureObserver::createDefault() { + return kj::heap(); +} + +void FeatureObserver::init(kj::Own instance) { + KJ_ASSERT(featureObserver == kj::none); + featureObserver = kj::mv(instance); +} + +kj::Maybe FeatureObserver::get() { + KJ_IF_SOME(impl, featureObserver) { + return *impl; + } + return kj::none; +} +}; diff --git a/src/workerd/io/observer.h b/src/workerd/io/observer.h index f0ba90fe84b..5951e553e44 100644 --- a/src/workerd/io/observer.h +++ b/src/workerd/io/observer.h @@ -12,6 +12,7 @@ #include #include #include +#include #include namespace workerd { @@ -247,4 +248,35 @@ class TeardownFinishedGuard { Observer& ref; }; +// Provides counters/observers for various features. The intent is to +// make it possible to collect metrics on which runtime features are +// used and how often. +// +// There is exactly one instance of this class per worker process. +class FeatureObserver { +public: + static kj::Own createDefault(); + static void init(kj::Own instance); + static kj::Maybe get(); + + // A "Feature" is just an opaque identifier defined in the features.capnp + // file. + using Feature = workerd::Features; + + // Called to increment the usage counter for a feature. + virtual void use(Feature feature) const {} + + using CollectCallback = kj::Function; + // This method is called from the internal metrics collection mechanisn to harvest the + // current features and counts that have been recorded by the observer. + virtual void collect(CollectCallback&& callback) const {} + + // Records the use of the feature if a FeatureObserver is available. + static inline void maybeRecordUse(Feature feature) { + KJ_IF_SOME(observer, get()) { + observer.use(feature); + } + } +}; + } // namespace workerd