opentelemetry-js-contrib icon indicating copy to clipboard operation
opentelemetry-js-contrib copied to clipboard

feat(bullmq): add bullmq (>=2) instrumentation

Open unflxw opened this issue 1 year ago • 7 comments

Add an instrumentation for BullMQ. This commit adds the already existing instrumentation at appsignal/opentelemetry-instrumentation-bullmq to this repository, with some minor changes, removing under-tested functionality in order to reduce the maintenance surface.

The initial work for this instrumentation was done by @jenniferplusplus at jenniferplusplus/opentelemetry-instrumentation-bullmq, of which both appsignal/opentelemetry-instrumentation-bullmq and the instrumentation in this commit are forks.

The instrumentation aims to comply with the Messaging Semantic Conventions (v0.24.0) whenever possible. Due to the latest published version of the @opentelemetry/semantic-conventions package not fully complying with the semantic conventions, when in conflict, the instrumentation chooses to follow the implemented shared package instead of the contents of the document.

unflxw avatar Jun 21 '24 13:06 unflxw

It's good to see this happening. I'm glad the instrumentation has been useful to people, but I'm also glad to be relieved of it. It's not a stack I use anymore.

I'll be happy to mark my npm package as deprecated once this gets merged and published.

jenniferplusplus avatar Jun 21 '24 19:06 jenniferplusplus

Thank you!

The same goes for @appsignal/opentelemetry-instrumentation-bullmq, we would deprecate it and switch our dependencies over to the contrib instrumentation.

unflxw avatar Jun 21 '24 19:06 unflxw

Hi @unflxw. We've recently added new contributing guidelines for new instrumentation libraries.

Is there a specific reason why you'd want @appsignal/opentelemetry-instrumentation-bullmq here in this repo as opposed to hosting and maintaining it in a seperate repo, ideally closer to the community that knows and maintains BullMQ?

pichlermarc avatar Jun 26 '24 08:06 pichlermarc

@pichlermarc Thanks for letting me know! I had not seen these guidelines before.

Our belief is that it is valuable for this instrumentation to be visible to the OpenTelemetry community, as well as for its maintenance to be shared with the community at large. Nonetheless, I have added myself as the maintainer for this instrumentation in the component owners file.

We're not BullMQ maintainers ourselves. We run an OpenTelemetry-based APM, and we took on the task of improving this instrumentation in order to be able to offer it to our customers. So, while we can host it on a public repository of our own, as we currently do, it will not be a repository owned by the BullMQ maintainers. The BullMQ maintainer team has stated that they're not interested in maintaining the OpenTelemetry instrumentation.

The instrumentation, while popular enough to have tens of thousands of weekly downloads, was not under active maintenance. Part of why we forked it was to avoid imposing additional maintenance workload on @jenniferplusplus, as we implemented changes to it. We would hope to prevent further fragmentation of the instrumentation ecosystem and the work on it by sponsoring it under the OpenTelemetry banner.

unflxw avatar Jun 26 '24 09:06 unflxw

I have added @luismiramirez as a code owner for the instrumentation, in compliance with the guidelines.

unflxw avatar Jun 26 '24 09:06 unflxw

Codecov Report

Attention: Patch coverage is 98.59813% with 3 lines in your changes missing coverage. Please review.

Project coverage is 90.54%. Comparing base (dfb2dff) to head (cb5868e). Report is 440 commits behind head on main.

Files with missing lines Patch % Lines
...node/instrumentation-bullmq/src/instrumentation.ts 99.03% 2 Missing :warning:
...ugins/node/instrumentation-bullmq/src/.eslintrc.js 0.00% 1 Missing :warning:
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2293      +/-   ##
==========================================
- Coverage   90.97%   90.54%   -0.44%     
==========================================
  Files         146      150       +4     
  Lines        7492     7477      -15     
  Branches     1502     1556      +54     
==========================================
- Hits         6816     6770      -46     
- Misses        676      707      +31     
Files with missing lines Coverage Δ
...gins/node/instrumentation-bullmq/src/attributes.ts 100.00% <100.00%> (ø)
...ugins/node/instrumentation-bullmq/src/.eslintrc.js 0.00% <0.00%> (ø)
...node/instrumentation-bullmq/src/instrumentation.ts 99.03% <99.03%> (ø)

... and 59 files with indirect coverage changes

:rocket: New features to boost your workflow:
  • :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • :package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

codecov[bot] avatar Jun 26 '24 09:06 codecov[bot]

Please let us know if there's anything we can do to help move this forward.

unflxw avatar Jul 29 '24 14:07 unflxw