appsignal / opentelemetry-instrumentation-bullmq

OTel Auto-instrumentation for BullMQ
Apache License 2.0
4 stars 3 forks source link

Built-in Observability interface in BullMQ #9

Open manast opened 2 months ago

manast commented 2 months ago

Hello,

this is not an issue, it's just to inform that we have finally creating a generic observability interface for BullMQ, which can be used to implement an OpenTelemetry version of it, which we will actually also provide very soon. You are welcome to give any feedback to the PR as we are still ironing out and learning about observability ourselves:

https://github.com/taskforcesh/bullmq/pull/2721

unflxw commented 1 month ago

Hi @manast, thank you! This is fantastic news, thank you for letting us know.

My two cents: while I understand the desire to avoid pulling in an additional dependency, I would recommend leveraging OpenTelemetry's own @opentelemetry/api package when possible, rather than, going by the PR linked, re-implementing a subset of it in your own code. Perhaps it could be an optional dependency, with a thin wrapper around it to account for it not being present. You may want to look at the Next.js or Prisma's internal OpenTelemery tracer wrappers for examples of this.

If the intention is for the library to be agnostic towards which telemetry system is used, and therefore tying it to @opentelemetry/api is not on the table, then I would still suggest that building these OpenTelemetry-esque semantics into it, as done in the PR, will not be friendly to other telemetry systems.

Instead, in that scenario, I would suggest providing "instrumentation hooks" at key functions worth instrumenting. The instrumentation hook is responsible for calling the actual function that performs the task being instrumented, and receives the same contextual information as the original function would have. This allows the actual instrumentation that uses the hooks to make use of them in whichever way it needs.

I've mocked up what I mean by this in a TypeScript playground. Please feel free to let me know if you have any questions.

manast commented 1 month ago

I appreciate the interface that we are providing may not be implementable by other telemetry systems, I guess we will figure that out later on. However I do not think it is possible to simplify it just as in the code you provide, as there are important semantic relations between when jobs are produced vs when they are consumed, and how the context data is actually stored and propagated in the jobs options, etc. Overall I think our current solution is a good trade-off. For users not caring about telemetry no new dependencies are needed and the overhead is negligible, for users caring about Otel telemetry they just need to import the Otel plugin and they are all set. Worst case scenario only Otel will work with this interface, I think that's still a good enough solution.