From 04c7f43b31535b2204a2dc65c09326dde65808e9 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++ | 82 +++++++++++++++++++++++++++------ src/workerd/api/kv.h | 13 ++++-- src/workerd/io/limit-enforcer.h | 2 +- 3 files changed, 77 insertions(+), 20 deletions(-) diff --git a/src/workerd/api/kv.c++ b/src/workerd/api/kv.c++ index e8593275a14..3acafa15f8d 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: @@ -95,8 +98,50 @@ kj::Own KvNamespace::getHttpClient(IoContext& context, KJ_UNREACHABLE; }(); - auto client = context.getHttpClientWithSpans( - subrequestChannel, true, kj::none, operationName, {{"db.system"_kjc, "cloudflare-kv"_kjc}}); + auto client = context.getHttpClientWithCallback( + subrequestChannel, true, kj::none, operationName, [&](TraceContext& tracing) { + auto& userSpan = tracing.userSpan; + userSpan.setTag("db.system"_kjc, kj::str("cloudflare-kv"_kj)); + userSpan.setTag("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) { + userSpan.setTag("cloudflare.kv.query.parameter.type"_kjc, kj::mv(type)); + } + KJ_CASE_ONEOF(o, GetOptions) { + KJ_IF_SOME(type, o.type) { + userSpan.setTag("cloudflare.kv.query.parameter.type"_kjc, kj::mv(type)); + } + KJ_IF_SOME(cacheTtl, o.cacheTtl) { + userSpan.setTag("cloudflare.kv.query.parameter.cacheTtl"_kjc, (long)cacheTtl); + } + } + } + } + KJ_CASE_ONEOF(o, ListOptions) { + KJ_IF_SOME(l, o.limit) { + userSpan.setTag("cloudflare.kv.query.parameter.limit"_kjc, (long)l); + } + KJ_IF_SOME(prefix, o.prefix.orDefault(kj::none)) { + userSpan.setTag("cloudflare.kv.query.parameter.prefix"_kjc, kj::mv(prefix)); + } + KJ_IF_SOME(cursor, o.cursor.orDefault(kj::none)) { + userSpan.setTag("cloudflare.kv.query.parameter.cursor"_kjc, kj::mv(cursor)); + } + } + KJ_CASE_ONEOF(o, PutOptions) { + KJ_IF_SOME(expiration, o.expiration) { + userSpan.setTag("cloudflare.kv.query.parameter.expiration"_kjc, (long)expiration); + } + KJ_IF_SOME(expirationTtl, o.expirationTtl) { + userSpan.setTag("cloudflare.kv.query.parameter.expirationTtl"_kjc, (long)expirationTtl); + } + } + } + } + }); headers.add(FLPROD_405_HEADER, urlStr); for (const auto& header: additionalHeaders) { headers.add(header.name.asPtr(), header.value.asPtr()); @@ -105,12 +150,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); }); }); @@ -118,6 +162,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(); @@ -132,11 +183,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)}); @@ -148,7 +199,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), @@ -260,7 +311,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), @@ -363,7 +415,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), @@ -419,7 +472,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;