Merge pull request #5498 from cloudflare/jmp/getCurrentBookmark-traces

Add tracing for `DurableObjectStorage::getCurrentBookmark()`

Author: justin-mp

.github/workflows/_bazel.yml

@@ -129,7 +129,7 @@ jobs:
         # dependencies of header targets properly).
         if: inputs.parse_headers
         run: |
-          bazel build --config=parse_headers --remote_cache=https://bazel:${{ secrets.BAZEL_CACHE_KEY }}@bazel-remote-cache.devprod.cloudflare.dev --config=ci ${{ inputs.extra_bazel_args }} //src/workerd/util //src/workerd/io:actor
+          bazel build --config=parse_headers --remote_cache=https://bazel:${{ secrets.BAZEL_CACHE_KEY }}@bazel-remote-cache.devprod.cloudflare.dev --config=ci ${{ inputs.extra_bazel_args }} //src/workerd/util
       - name: Upload test logs
         if: inputs.upload_test_logs
         uses: actions/upload-artifact@v4

src/workerd/api/actor-state.c++

@@ -839,7 +839,10 @@ jsg::Ref<SyncKvStorage> DurableObjectStorage::getKv(jsg::Lock& js) {
 }
 
 kj::Promise<kj::String> DurableObjectStorage::getCurrentBookmark() {
-  return cache->getCurrentBookmark();
+  auto& context = IoContext::current();
+  auto span = context.makeTraceSpan("durable_object_storage_getCurrentBookmark"_kjc);
+
+  return cache->getCurrentBookmark(span).attach(kj::mv(span));
 }
 
 kj::Promise<kj::String> DurableObjectStorage::getBookmarkForTime(kj::Date timestamp) {

src/workerd/io/BUILD.bazel

@@ -199,13 +199,11 @@ wd_cc_library(
         "actor-sqlite.h",
         "actor-storage.h",
     ],
-    implementation_deps = [
-        "@sqlite3",
-    ],
     visibility = ["//visibility:public"],
     deps = [
         ":actor-storage_capnp",
         ":io-gate",
+        ":trace",
         "//src/workerd/jsg:exception",
         "//src/workerd/util:autogate",
         "//src/workerd/util:duration-exceeded-logger",

src/workerd/io/actor-cache.h

@@ -5,6 +5,7 @@
 #pragma once
 
 #include <workerd/io/actor-storage.capnp.h>
+#include <workerd/io/trace.h>
 #include <workerd/jsg/exception.h>
 
 #include <kj/async.h>
@@ -242,7 +243,7 @@ class ActorCacheInterface: public ActorCacheOps {
 
   // Implements the respective PITR API calls. The default implementations throw JSG errors saying
   // PITR is not implemented. These methods are meant to be implemented internally.
-  virtual kj::Promise<kj::String> getCurrentBookmark() {
+  virtual kj::Promise<kj::String> getCurrentBookmark(SpanParent parentSpan) {
     JSG_FAIL_REQUIRE(
         Error, "This Durable Object's storage back-end does not implement point-in-time recovery.");
   }

src/workerd/io/actor-sqlite.c++

@@ -10,8 +10,6 @@
 #include <workerd/util/autogate.h>
 #include <workerd/util/sentry.h>
 
-#include <sqlite3.h>
-
 #include <kj/exception.h>
 #include <kj/function.h>
 
@@ -826,7 +824,7 @@ kj::Maybe<kj::Promise<void>> ActorSqlite::onNoPendingFlush() {
   return kj::joinPromisesFailFast(kj::arr(lastCommit.addBranch(), outputGate.wait()));
 }
 
-kj::Promise<kj::String> ActorSqlite::getCurrentBookmark() {
+kj::Promise<kj::String> ActorSqlite::getCurrentBookmark(SpanParent parentSpan) {
   // This is an ersatz implementation that's good enough for local dev with D1's Session API.
   //
   // The returned bookmark satisfies the properties that D1 cares about:
@@ -873,6 +871,10 @@ kj::Promise<void> ActorSqlite::waitForBookmark(kj::StringPtr bookmark) {
 void ActorSqlite::TxnCommitRegulator::onError(
     kj::Maybe<int> sqliteErrorCode, kj::StringPtr message) const {
   KJ_IF_SOME(c, sqliteErrorCode) {
+    // We cannot `#include <sqlite3.h>` in the same compilation unit as `#include
+    // <workerd/io/trace.h>` because the latter includes v8 and v8 seems to conflict with sqlite.
+    // So we copy the value of SQLITE_CONSTRAINT from sqlite3.h
+    constexpr int SQLITE_CONSTRAINT = 19;
     if (c == SQLITE_CONSTRAINT) {
       JSG_ASSERT(false, Error,
           "Durable Object was reset and rolled back to its last known good state because the "

src/workerd/io/actor-sqlite.h

@@ -6,6 +6,7 @@
 
 #include "actor-cache.h"
 
+#include <workerd/io/trace.h>
 #include <workerd/util/sqlite-kv.h>
 #include <workerd/util/sqlite-metadata.h>
 
@@ -92,7 +93,7 @@ class ActorSqlite final: public ActorCacheInterface, private kj::TaskSet::ErrorH
       kj::Date scheduledTime, bool noCache = false, kj::StringPtr actorId = "") override;
   void cancelDeferredAlarmDeletion() override;
   kj::Maybe<kj::Promise<void>> onNoPendingFlush() override;
-  kj::Promise<kj::String> getCurrentBookmark() override;
+  kj::Promise<kj::String> getCurrentBookmark(SpanParent parentSpan) override;
   kj::Promise<void> waitForBookmark(kj::StringPtr bookmark) override;
   // See ActorCacheInterface