-
Notifications
You must be signed in to change notification settings - Fork 23
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Add distributed tracing example #279
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
overall looks fine. one issue with README, and a few suggestions(up to you if you want to address)
* attributes include/exclude lists. | ||
*/ | ||
allow_all_headers: true, | ||
distributed_tracing: { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it doesn't really matter but DT is on by default and has been for several years now
Please link this in the parent README |
}) | ||
|
||
// since BullMQ is not auto instrumented by the newrelic node agent, we have to manually start a transaction | ||
return newrelic.startBackgroundTransaction('Message queue - consumer', function innerHandler() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so this creates only 1 transaction. you could wrap every consumption in a transaction because right now it's 1. I know in standard messages queues we create 1 transaction for every consumption
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would wrap it in the job processor, right?
Proposed Release Notes
Added a message queue example to showcase distributed tracing.
Details
Created a message queue using BullMQ and Redis.
Closes #268