feat(cloudflare): export the init method

[marbemac]

I'm creating little helpers to handle trace propagation on regular durable object rpc calls. To do this I needed access to the init method to initialize a new sdk instance (just like sentry/cloudflare does for the methods it auto-instruments). So, if you guys are open to it, hoping to get access to the init method.

Example of how I'm using it here -> https://github.com/marbemac/cloudflare-sentry-effect-tracing/blob/main/worker/rpc-tracing-helpers.ts#L112.

[Lms24]

Hey @marbemac thanks for opening this PR! Before we merge this, can you elaborate a bit on your use case? The reason I'm a bit hesitant still is because afaik the SDK must be initialized in each request handler (as opposed to once globally like in Node or other platforms). Though admittedly my knowledge on Cloudflare is a bit limited. I asked some other team members to take a look as well.

[marbemac]

Sure - tldr I am initializing the sdk on every request (this is why I needed access to the init method :)).

@sentry/cloudflare durable object automatically instruments/wraps known methods (fetch, alarm websocketMessage, etc). But we make extensive use of durable object rpc calls, not the fetch handler. So I established a basic pattern to instrument our rpc calls w trace propagation (this is key and why the existing instrumentPrototypeMethods option isn't viable). It works like this:

1. Export the callTraceableRPC helper from the durable object file - things that make rpc calls to this durable object should use this helper.

https://github.com/marbemac/cloudflare-sentry-effect-tracing/blob/main/worker/my-durable-object.ts#L9

const { callTraceableRPC, continueTraceableRPC } = makeTraceableRPCHelpers({ serviceName: 'MyDurableObject' });

export { callTraceableRPC };

2. The public rpc methods, that are meant to be called from the outside, must pass their props to continueTraceableRPC. This is the function that inits the sentry sdk and wraps everything in a continueTrace + span (and is linked in original pr description).

https://github.com/marbemac/cloudflare-sentry-effect-tracing/blob/main/worker/my-durable-object.ts#L14-L21

export class MyDurableObject extends DurableObject<Env> {
  async sayHello(props: WithTrace<ExampleProps>) {
    return continueTraceableRPC('sayHello', this.#sayHello, this.ctx.waitUntil.bind(this.ctx), props);
  }

  #sayHello = async (_: ExampleProps) => {
    return { hello: 'from durable object' };
  };
}

3. Consumers call the durable object rpc method via callTraceableRPC (imported from that export in step 1).

https://github.com/marbemac/cloudflare-sentry-effect-tracing/blob/main/worker/router.ts#L126-L128

const stub = env.MY_DURABLE_OBJECT.getByName('xyz');

const res = await callTraceableRPC(stub, 'sayHello');

It's all nice and type safe, and you can't do things like accidentally call a durable object method w callTraceableRPC that wasn't set up with continueTraceableRPC. Could probably make this even nicer with decorators.

[marbemac]

In general if this is a pattern you guys think could be interesting as first class in sentry sdk, I can certainly clean it up and open a PR. It's just overall a little awkward to get the DO rpc calls instrumented correctly - I'm sure there are many ways, so I wasn't sure if this kind of thing belonged in the sdk itself.

[Lms24]

Thanks for outlining! IMHO we should, instead of exporting init and handing a potential footgun to users, rather support RPC methods on DOs. Without too much insights, I think this can be achieved in two ways:

1. we find a way that instrumentDurableObjectWithSentry can automatically wrap RPC methods. Most likely we need users to pass in an array of method names that are RPC methods but I think this would be a nice to use API. This instrumentation should then automatically take care of trace continuation.

2. the less involved version is to provide wrappers for your DO RPC methods, similarly to what you already showed in your reply. Maybe we could start with this and use the same wrapper in instrumentDurableObjectWithSentry eventually.

I guess the tricky part is that the RPC consumer apparently has to forward the trace data to the RPC method, so we likely need a counter part here as well. Before we do any of this, I'll tag @AbhiPrasad, @s1gr1d and @andreiborza -- thoughts? (if anyone of you thinks we should rather go with exporting init as step 1, I'm also happy to go with that!)

Last thought: I'm wondering if there's a lower-level primitive we could instrument for trace propagation/continuation on DOs so that this is taken care of for requests in general.

[s1gr1d]

This is mostly about continuing the trace, right?

In your code example, the traceableRPC starts a span and adds the trace context. Within (or probably outside) the startSpan of Sentry's DO wrapper, we could call continueTrace.

We could get the needed baggage and sentry-trace from the args (when they are defined in a specific way) or people provide a callback so the SDK can get the needed data.

SDK users would need to call it like this (with trace or other key - by adding a callback to the wrapper options, people can define their own "extracting" key):

const { ['sentry-trace']: sentryTrace, baggage } = Sentry.getTraceData();
await stub.sayHello({ name: 'World', trace: { sentryTrace, baggage } });

[andreiborza]

I think we can expose a wrapper function like @Lms24 said that does what @s1gr1d suggests under the hood.

[marbemac]

Hi folks - yes that's exactly what this util is doing - https://github.com/marbemac/cloudflare-sentry-effect-tracing/blob/main/worker/rpc-tracing-helpers.ts.

It returns a type safe callTraceableRPC fn that consumers are expected to use when calling traceable rpc functions. And a continueTraceableRPC fn that the rpc target is expected to wrap the rpc entrypoint with.

[Lms24]

Hey @marbemac thanks for sticking with us! We discussed this internally and decided that for the time being, we'll not export the init function, primarily to avoid handing users an API they'd expect to work differently than it actually does.
Therefore I'm going to close this PR for now.

However, if you're still up for it, we'd happily review a PR that enables wrapping RPC calls (on both ends) similarly to what you showed us in https://github.com/marbemac/cloudflare-sentry-effect-tracing/blob/main/worker/rpc-tracing-helpers.ts.

Some thoughts around this:

• It would be great if the wrappers can be used in both Durable objects and regular workers (as discussed in Instrument Cloudflare Worker RPC methods #16898, RPC methods don't have to be on DOs).
• Ideally they're designed in a way so that we can eventually automatically add them to workers and DOs at some point. Manual usage first and automatic application later seems like a good step-by-step approach to us.
• We'll probably need to adjust the naming and the functionality to make it a bit more general. Tracing is not the only thing these wrappers do but they should also capture exceptions and handle request isolation (which they already seem to do). So maybe something along the lines of callRpcWithSentry and wrapRpcWithSentry? No guarantees that these will be the final names but take it as food for thought.

Again, thanks for contributing and for starting the conversation!

[marbemac]

Np! Using a patch file for the moment, and the patch is tiny (just adding that init to the index export), so should do fine for us for a while.

Adding to my todo list - can't promise anything but I'd love to help out here.

[marbemac]

We'll probably need to adjust the naming and the functionality to make it a bit more general.

Agree on this btw - we already have some extra stuff in our version that handles things like serializing exceptions over the rpc call (for cases where the exception reporter is in the worker making the call to the DO - like in router middleware). otherwise things like the original stack trace get lost