From b48a8479bc267682bdd2f61ca077b4f0fbaf308f Mon Sep 17 00:00:00 2001 From: Felix Hanau Date: Wed, 25 Sep 2024 14:58:10 +0000 Subject: [PATCH] [o11y] Add KV span tags --- src/workerd/api/kv.c++ | 81 ++++++++++++++++++++++++++++----- src/workerd/api/kv.h | 13 ++++-- src/workerd/io/limit-enforcer.h | 2 +- 3 files changed, 78 insertions(+), 18 deletions(-) diff --git a/src/workerd/api/kv.c++ b/src/workerd/api/kv.c++ index 12f0d7afd15..41112d29d8e 100644 --- a/src/workerd/api/kv.c++ +++ b/src/workerd/api/kv.c++ @@ -69,7 +69,8 @@ constexpr auto FLPROD_405_HEADER = "CF-KV-FLPROD-405"_kj; kj::Own KvNamespace::getHttpClient(IoContext& context, kj::HttpHeaders& headers, kj::OneOf opTypeOrUnknown, - kj::StringPtr urlStr) { + kj::StringPtr urlStr, + kj::Maybe, PutOptions>> options) { const auto operationName = [&] { KJ_SWITCH_ONEOF(opTypeOrUnknown) { KJ_CASE_ONEOF(name, kj::LiteralStringConst) { @@ -82,6 +83,8 @@ kj::Own KvNamespace::getHttpClient(IoContext& context, switch (opType) { case LimitEnforcer::KvOpType::GET: return "kv_get"_kjc; + case LimitEnforcer::KvOpType::GET_WITH: + return "kv_getWithMetadata"_kjc; case LimitEnforcer::KvOpType::PUT: return "kv_put"_kjc; case LimitEnforcer::KvOpType::LIST: @@ -97,8 +100,53 @@ kj::Own KvNamespace::getHttpClient(IoContext& context, kj::Vector tags; tags.add("db.system"_kjc, kj::str("cloudflare-kv"_kjc)); + tags.add("cloudflare.kv.operation.name"_kjc, kj::str(operationName.slice(3))); + + KJ_IF_SOME(_options, options) { + KJ_SWITCH_ONEOF(_options) { + KJ_CASE_ONEOF(o2, kj::OneOf) { + KJ_SWITCH_ONEOF(o2) { + KJ_CASE_ONEOF(type, kj::String) { + tags.add("cloudflare.kv.query.parameter.type"_kjc, kj::mv(type)); + } + KJ_CASE_ONEOF(o, GetOptions) { + KJ_IF_SOME(type, o.type) { + tags.add("cloudflare.kv.query.parameter.type"_kjc, kj::mv(type)); + } + KJ_IF_SOME(cacheTtl, o.cacheTtl) { + tags.add("cloudflare.kv.query.parameter.cacheTtl"_kjc, (int64_t)cacheTtl); + } + } + } + } + KJ_CASE_ONEOF(o, ListOptions) { + KJ_IF_SOME(l, o.limit) { + tags.add("cloudflare.kv.query.parameter.limit"_kjc, (int64_t)l); + } + KJ_IF_SOME(prefix, o.prefix) { + KJ_IF_SOME(p, prefix) { + tags.add("cloudflare.kv.query.parameter.prefix"_kjc, kj::mv(p)); + } + } + KJ_IF_SOME(cursor, o.cursor) { + KJ_IF_SOME(c, cursor) { + tags.add("cloudflare.kv.query.parameter.cursor"_kjc, kj::mv(c)); + } + } + } + KJ_CASE_ONEOF(o, PutOptions) { + KJ_IF_SOME(expiration, o.expiration) { + tags.add("cloudflare.kv.query.parameter.expiration"_kjc, (int64_t)expiration); + } + KJ_IF_SOME(expirationTtl, o.expirationTtl) { + tags.add("cloudflare.kv.query.parameter.expirationTtl"_kjc, (int64_t)expirationTtl); + } + } + } + } auto client = context.getHttpClientWithSpans( subrequestChannel, true, kj::none, operationName, kj::mv(tags)); + headers.add(FLPROD_405_HEADER, urlStr); for (const auto& header: additionalHeaders) { headers.add(header.name.asPtr(), header.value.asPtr()); @@ -107,12 +155,11 @@ kj::Own KvNamespace::getHttpClient(IoContext& context, return client; } -jsg::Promise KvNamespace::get(jsg::Lock& js, - kj::String name, - jsg::Optional> options, - CompatibilityFlags::Reader flags) { +jsg::Promise KvNamespace::get( + jsg::Lock& js, kj::String name, jsg::Optional> options) { return js.evalNow([&] { - auto resp = getWithMetadata(js, kj::mv(name), kj::mv(options)); + auto resp = + getWithMetadataImpl(js, kj::mv(name), kj::mv(options), LimitEnforcer::KvOpType::GET); return resp.then(js, [](jsg::Lock&, KvNamespace::GetWithMetadataResult result) { return kj::mv(result.value); }); }); @@ -120,6 +167,13 @@ jsg::Promise KvNamespace::get(jsg::Lock& js, jsg::Promise KvNamespace::getWithMetadata( jsg::Lock& js, kj::String name, jsg::Optional> options) { + return getWithMetadataImpl(js, kj::mv(name), kj::mv(options), LimitEnforcer::KvOpType::GET_WITH); +} + +jsg::Promise KvNamespace::getWithMetadataImpl(jsg::Lock& js, + kj::String name, + jsg::Optional> options, + LimitEnforcer::KvOpType op) { validateKeyName("GET", name); auto& context = IoContext::current(); @@ -134,11 +188,11 @@ jsg::Promise KvNamespace::getWithMetadata( KJ_IF_SOME(oneOfOptions, options) { KJ_SWITCH_ONEOF(oneOfOptions) { KJ_CASE_ONEOF(t, kj::String) { - type = kj::mv(t); + type = kj::str(t); } KJ_CASE_ONEOF(options, GetOptions) { KJ_IF_SOME(t, options.type) { - type = kj::mv(t); + type = kj::str(t); } KJ_IF_SOME(cacheTtl, options.cacheTtl) { url.query.add(kj::Url::QueryParam{kj::str("cache_ttl"), kj::str(cacheTtl)}); @@ -150,7 +204,7 @@ jsg::Promise KvNamespace::getWithMetadata( auto urlStr = url.toString(kj::Url::Context::HTTP_PROXY_REQUEST); auto headers = kj::HttpHeaders(context.getHeaderTable()); - auto client = getHttpClient(context, headers, LimitEnforcer::KvOpType::GET, urlStr); + auto client = getHttpClient(context, headers, op, urlStr, kj::mv(options)); auto request = client->request(kj::HttpMethod::GET, urlStr, headers); return context.awaitIo(js, kj::mv(request.response), @@ -262,7 +316,8 @@ jsg::Promise> KvNamespace::list( auto urlStr = url.toString(kj::Url::Context::HTTP_PROXY_REQUEST); auto headers = kj::HttpHeaders(context.getHeaderTable()); - auto client = getHttpClient(context, headers, LimitEnforcer::KvOpType::LIST, urlStr); + auto client = + getHttpClient(context, headers, LimitEnforcer::KvOpType::LIST, urlStr, kj::mv(options)); auto request = client->request(kj::HttpMethod::GET, urlStr, headers); return context.awaitIo(js, kj::mv(request.response), @@ -365,7 +420,8 @@ jsg::Promise KvNamespace::put(jsg::Lock& js, auto urlStr = url.toString(kj::Url::Context::HTTP_PROXY_REQUEST); - auto client = getHttpClient(context, headers, LimitEnforcer::KvOpType::PUT, urlStr); + auto client = + getHttpClient(context, headers, LimitEnforcer::KvOpType::PUT, urlStr, kj::mv(options)); auto promise = context.waitForOutputLocks().then( [&context, client = kj::mv(client), urlStr = kj::mv(urlStr), headers = kj::mv(headers), @@ -421,7 +477,8 @@ jsg::Promise KvNamespace::delete_(jsg::Lock& js, kj::String name) { kj::HttpHeaders headers(context.getHeaderTable()); - auto client = getHttpClient(context, headers, LimitEnforcer::KvOpType::DELETE, urlStr); + auto client = + getHttpClient(context, headers, LimitEnforcer::KvOpType::DELETE, urlStr, kj::none); auto promise = context.waitForOutputLocks().then( [headers = kj::mv(headers), client = kj::mv(client), urlStr = kj::mv(urlStr)]() mutable { diff --git a/src/workerd/api/kv.h b/src/workerd/api/kv.h index 0060991d462..491199029b7 100644 --- a/src/workerd/api/kv.h +++ b/src/workerd/api/kv.h @@ -50,10 +50,8 @@ class KvNamespace: public jsg::Object { using GetResult = kj::Maybe< kj::OneOf, kj::Array, kj::String, jsg::JsRef>>; - jsg::Promise get(jsg::Lock& js, - kj::String name, - jsg::Optional> options, - CompatibilityFlags::Reader flags); + jsg::Promise get( + jsg::Lock& js, kj::String name, jsg::Optional> options); struct GetWithMetadataResult { GetResult value; @@ -68,6 +66,10 @@ class KvNamespace: public jsg::Object { }); }; + jsg::Promise getWithMetadataImpl(jsg::Lock& js, + kj::String name, + jsg::Optional> options, + LimitEnforcer::KvOpType op); jsg::Promise getWithMetadata( jsg::Lock& js, kj::String name, jsg::Optional> options); @@ -173,7 +175,8 @@ class KvNamespace: public jsg::Object { kj::Own getHttpClient(IoContext& context, kj::HttpHeaders& headers, kj::OneOf opTypeOrName, - kj::StringPtr urlStr); + kj::StringPtr urlStr, + kj::Maybe, PutOptions>> options); private: kj::Array additionalHeaders; diff --git a/src/workerd/io/limit-enforcer.h b/src/workerd/io/limit-enforcer.h index 3287be9b5ee..4525f889ce2 100644 --- a/src/workerd/io/limit-enforcer.h +++ b/src/workerd/io/limit-enforcer.h @@ -109,7 +109,7 @@ class LimitEnforcer { // external subrequests. virtual void newSubrequest(bool isInHouse) = 0; - enum class KvOpType { GET, PUT, LIST, DELETE }; + enum class KvOpType { GET, GET_WITH, PUT, LIST, DELETE }; // Called before starting a KV operation. Throws a JSG exception if the operation should be // blocked due to exceeding limits, such as the free tier daily operation limit. virtual void newKvRequest(KvOpType op) = 0;