-
Notifications
You must be signed in to change notification settings - Fork 935
Add tracing channels #2281
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
Add tracing channels #2281
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.
Great!
mcollina
left a comment
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.
+1
Can you add some docs and check the bechmarks?
|
@bengl might this help DataDog as well? |
bizob2828
left a comment
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 can't see the entire function, is there a point where an error needs to be emitted?
|
+1 from the Sentry side. For reference, here's the existing OTEL instrumentation: https://www.npmjs.com/package/@opentelemetry/instrumentation-pino |
|
I published the PR and then went to lunch. As I was having lunch, I realized that subscribers should not need to utilize the exported channels. So I reworked the test in 52c6862 to only use the event names. Question to the crowd: should Pino even export its internal channels? |
@AbhiPrasad I am not very familiar with the OTEL instrumentation. Does this PR address what OTEL would need in order to utilize tracing channels instead of monkey patching Pino? |
This comment was marked as outdated.
This comment was marked as outdated.
|
Our instrumentation in |
lib/tools.js
Outdated
| } | ||
|
|
||
| function asJson (obj, msg, num, time) { | ||
| asJsonChan.start.publish({ instance: this, arguments }) |
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.
Could you add a test verifying that the "active span" (or just dummy object in AsyncLocalStorage) is exactly the same as when the logging function is called?
|
Just had a call with @bengl on this. My current implementation is a bit flawed in understanding how tracing channel is meant to be used by libraries intending to support tracing channel. I need to refactor some things, so I have converted this back to draft. |
d832231 to
0d556fa
Compare
|
Basic benchmarks from my system are as follows: BranchMainThese results line up with the ones in our automated PR benchmarks. I find it strange that they are lower for the branch, but they are consistent in this. There should be a bit of a hit when there are subscribers attached, but our baseline seems unaffected. |
|
Using |
mcollina
left a comment
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.
lgtm
|
Awesome! We got it merged. Thank you for helping get this thing in the right state, everyone. It'll go out in Pino's next release. |
 <h3>Snyk has created this PR to upgrade pino from 9.9.5 to 9.11.0.</h3> :information_source: Keep your dependencies up-to-date. This makes it easier to fix existing vulnerabilities and to more quickly identify and fix newly disclosed vulnerabilities when they affect your project. <hr/> - The recommended version is **2 versions** ahead of your current version. - The recommended version was released **24 days ago**. <details> <summary><b>Release notes</b></summary> <br/> <details> <summary>Package name: <b>pino</b></summary> <ul> <li> <b>9.11.0</b> - <a href="https://redirect.github.com/pinojs/pino/releases/tag/v9.11.0">2025-09-20</a></br><h2>What's Changed</h2> <ul> <li>feat: added timestamp rfc3339 format with nanoseconds by <a class="user-mention notranslate" data-hovercard-type="user" data-hovercard-url="/users/edge33/hovercard" data-octo-click="hovercard-link-click" data-octo-dimensions="link_type:self" href="https://redirect.github.com/edge33">@ edge33</a> in <a class="issue-link js-issue-link" data-error-text="Failed to load title" data-id="3288586845" data-permission-text="Title is private" data-url="pinojs/pino#2251" data-hovercard-type="pull_request" data-hovercard-url="/pinojs/pino/pull/2251/hovercard" href="https://redirect.github.com/pinojs/pino/pull/2251">#2251</a></li> <li>fix: gracefully handle missing diagChannel.tracingChannel on Node < 18.19 by <a class="user-mention notranslate" data-hovercard-type="user" data-hovercard-url="/users/aryamohanan/hovercard" data-octo-click="hovercard-link-click" data-octo-dimensions="link_type:self" href="https://redirect.github.com/aryamohanan">@ aryamohanan</a> in <a class="issue-link js-issue-link" data-error-text="Failed to load title" data-id="3433786084" data-permission-text="Title is private" data-url="pinojs/pino#2290" data-hovercard-type="pull_request" data-hovercard-url="/pinojs/pino/pull/2290/hovercard" href="https://redirect.github.com/pinojs/pino/pull/2290">#2290</a></li> </ul> <h2>New Contributors</h2> <ul> <li><a class="user-mention notranslate" data-hovercard-type="user" data-hovercard-url="/users/edge33/hovercard" data-octo-click="hovercard-link-click" data-octo-dimensions="link_type:self" href="https://redirect.github.com/edge33">@ edge33</a> made their first contribution in <a class="issue-link js-issue-link" data-error-text="Failed to load title" data-id="3288586845" data-permission-text="Title is private" data-url="pinojs/pino#2251" data-hovercard-type="pull_request" data-hovercard-url="/pinojs/pino/pull/2251/hovercard" href="https://redirect.github.com/pinojs/pino/pull/2251">#2251</a></li> <li><a class="user-mention notranslate" data-hovercard-type="user" data-hovercard-url="/users/aryamohanan/hovercard" data-octo-click="hovercard-link-click" data-octo-dimensions="link_type:self" href="https://redirect.github.com/aryamohanan">@ aryamohanan</a> made their first contribution in <a class="issue-link js-issue-link" data-error-text="Failed to load title" data-id="3433786084" data-permission-text="Title is private" data-url="pinojs/pino#2290" data-hovercard-type="pull_request" data-hovercard-url="/pinojs/pino/pull/2290/hovercard" href="https://redirect.github.com/pinojs/pino/pull/2290">#2290</a></li> </ul> <p><strong>Full Changelog</strong>: <a class="commit-link" href="https://redirect.github.com/pinojs/pino/compare/v9.10.0...v9.11.0"><tt>v9.10.0...v9.11.0</tt></a></p> </li> <li> <b>9.10.0</b> - <a href="https://redirect.github.com/pinojs/pino/releases/tag/v9.10.0">2025-09-17</a></br><h2>What's Changed</h2> <ul> <li>docs: Move pino-logflare out of legacy transports list by <a class="user-mention notranslate" data-hovercard-type="user" data-hovercard-url="/users/kamilogorek/hovercard" data-octo-click="hovercard-link-click" data-octo-dimensions="link_type:self" href="https://redirect.github.com/kamilogorek">@ kamilogorek</a> in <a class="issue-link js-issue-link" data-error-text="Failed to load title" data-id="3406229630" data-permission-text="Title is private" data-url="pinojs/pino#2283" data-hovercard-type="pull_request" data-hovercard-url="/pinojs/pino/pull/2283/hovercard" href="https://redirect.github.com/pinojs/pino/pull/2283">#2283</a></li> <li>Add support for <code>Pear</code> and <code>Bare</code> runtimes by <a class="user-mention notranslate" data-hovercard-type="user" data-hovercard-url="/users/yassernasc/hovercard" data-octo-click="hovercard-link-click" data-octo-dimensions="link_type:self" href="https://redirect.github.com/yassernasc">@ yassernasc</a> in <a class="issue-link js-issue-link" data-error-text="Failed to load title" data-id="3388452783" data-permission-text="Title is private" data-url="pinojs/pino#2278" data-hovercard-type="pull_request" data-hovercard-url="/pinojs/pino/pull/2278/hovercard" href="https://redirect.github.com/pinojs/pino/pull/2278">#2278</a></li> <li>Add tracing channels by <a class="user-mention notranslate" data-hovercard-type="user" data-hovercard-url="/users/jsumners-nr/hovercard" data-octo-click="hovercard-link-click" data-octo-dimensions="link_type:self" href="https://redirect.github.com/jsumners-nr">@ jsumners-nr</a> in <a class="issue-link js-issue-link" data-error-text="Failed to load title" data-id="3402802217" data-permission-text="Title is private" data-url="pinojs/pino#2281" data-hovercard-type="pull_request" data-hovercard-url="/pinojs/pino/pull/2281/hovercard" href="https://redirect.github.com/pinojs/pino/pull/2281">#2281</a></li> <li>Add pino-console to ecoystem page by <a class="user-mention notranslate" data-hovercard-type="user" data-hovercard-url="/users/mcollina/hovercard" data-octo-click="hovercard-link-click" data-octo-dimensions="link_type:self" href="https://redirect.github.com/mcollina">@ mcollina</a> in <a class="issue-link js-issue-link" data-error-text="Failed to load title" data-id="3426535226" data-permission-text="Title is private" data-url="pinojs/pino#2288" data-hovercard-type="pull_request" data-hovercard-url="/pinojs/pino/pull/2288/hovercard" href="https://redirect.github.com/pinojs/pino/pull/2288">#2288</a></li> </ul> <h2>New Contributors</h2> <ul> <li><a class="user-mention notranslate" data-hovercard-type="user" data-hovercard-url="/users/kamilogorek/hovercard" data-octo-click="hovercard-link-click" data-octo-dimensions="link_type:self" href="https://redirect.github.com/kamilogorek">@ kamilogorek</a> made their first contribution in <a class="issue-link js-issue-link" data-error-text="Failed to load title" data-id="3406229630" data-permission-text="Title is private" data-url="pinojs/pino#2283" data-hovercard-type="pull_request" data-hovercard-url="/pinojs/pino/pull/2283/hovercard" href="https://redirect.github.com/pinojs/pino/pull/2283">#2283</a></li> <li><a class="user-mention notranslate" data-hovercard-type="user" data-hovercard-url="/users/yassernasc/hovercard" data-octo-click="hovercard-link-click" data-octo-dimensions="link_type:self" href="https://redirect.github.com/yassernasc">@ yassernasc</a> made their first contribution in <a class="issue-link js-issue-link" data-error-text="Failed to load title" data-id="3388452783" data-permission-text="Title is private" data-url="pinojs/pino#2278" data-hovercard-type="pull_request" data-hovercard-url="/pinojs/pino/pull/2278/hovercard" href="https://redirect.github.com/pinojs/pino/pull/2278">#2278</a></li> <li><a class="user-mention notranslate" data-hovercard-type="user" data-hovercard-url="/users/jsumners-nr/hovercard" data-octo-click="hovercard-link-click" data-octo-dimensions="link_type:self" href="https://redirect.github.com/jsumners-nr">@ jsumners-nr</a> made their first contribution in <a class="issue-link js-issue-link" data-error-text="Failed to load title" data-id="3402802217" data-permission-text="Title is private" data-url="pinojs/pino#2281" data-hovercard-type="pull_request" data-hovercard-url="/pinojs/pino/pull/2281/hovercard" href="https://redirect.github.com/pinojs/pino/pull/2281">#2281</a></li> </ul> <p><strong>Full Changelog</strong>: <a class="commit-link" href="https://redirect.github.com/pinojs/pino/compare/v9.9.5...v9.10.0"><tt>v9.9.5...v9.10.0</tt></a></p> </li> <li> <b>9.9.5</b> - <a href="https://redirect.github.com/pinojs/pino/releases/tag/v9.9.5">2025-09-10</a></br><h2>What's Changed</h2> <ul> <li>fix: allow object type in %s placeholder by <a class="user-mention notranslate" data-hovercard-type="user" data-hovercard-url="/users/mcollina/hovercard" data-octo-click="hovercard-link-click" data-octo-dimensions="link_type:self" href="https://redirect.github.com/mcollina">@ mcollina</a> in <a class="issue-link js-issue-link" data-error-text="Failed to load title" data-id="3388093159" data-permission-text="Title is private" data-url="pinojs/pino#2277" data-hovercard-type="pull_request" data-hovercard-url="/pinojs/pino/pull/2277/hovercard" href="https://redirect.github.com/pinojs/pino/pull/2277">#2277</a></li> </ul> <p><strong>Full Changelog</strong>: <a class="commit-link" href="https://redirect.github.com/pinojs/pino/compare/v9.9.4...v9.9.5"><tt>v9.9.4...v9.9.5</tt></a></p> </li> </ul> from <a href="https://redirect.github.com/pinojs/pino/releases">pino GitHub release notes</a> </details> </details> --- > [!IMPORTANT] > > - Check the changes in this PR to ensure they won't cause issues with your project. > - This PR was automatically created by Snyk using the credentials of a real user. --- **Note:** _You are seeing this because you or someone else with access to this repository has authorized Snyk to open upgrade PRs._ **For more information:** <img src="https://api.segment.io/v1/pixel/track?data=eyJ3cml0ZUtleSI6InJyWmxZcEdHY2RyTHZsb0lYd0dUcVg4WkFRTnNCOUEwIiwiYW5vbnltb3VzSWQiOiJlMGNmZDdhOS0yNzM2LTQ3ZDUtOTY5Ny1iYWZjYzg0MDk5ZDgiLCJldmVudCI6IlBSIHZpZXdlZCIsInByb3BlcnRpZXMiOnsicHJJZCI6ImUwY2ZkN2E5LTI3MzYtNDdkNS05Njk3LWJhZmNjODQwOTlkOCJ9fQ==" width="0" height="0"/> > - 🧐 [View latest project report](https://app.snyk.io/org/bugbaredrums/project/cf9c5b42-a7d0-446f-a41c-275c56edf456?utm_source=github&utm_medium=referral&page=upgrade-pr) > - 📜 [Customise PR templates](https://docs.snyk.io/scan-using-snyk/pull-requests/snyk-fix-pull-or-merge-requests/customize-pr-templates?utm_source=&utm_content=fix-pr-template) > - 🛠 [Adjust upgrade PR settings](https://app.snyk.io/org/bugbaredrums/project/cf9c5b42-a7d0-446f-a41c-275c56edf456/settings/integration?utm_source=github&utm_medium=referral&page=upgrade-pr) > - 🔕 [Ignore this dependency or unsubscribe from future upgrade PRs](https://app.snyk.io/org/bugbaredrums/project/cf9c5b42-a7d0-446f-a41c-275c56edf456/settings/integration?pkg=pino&utm_source=github&utm_medium=referral&page=upgrade-pr#auto-dep-upgrades) [//]: # 'snyk:metadata:{"breakingChangeRiskLevel":null,"FF_showPullRequestBreakingChanges":null,"FF_showPullRequestBreakingChangesWebSearch":null,"customTemplate":{"variablesUsed":[],"fieldsUsed":[]},"dependencies":[{"name":"pino","from":"9.9.5","to":"9.11.0"}],"env":"prod","hasFixes":false,"isBreakingChange":false,"isMajorUpgrade":false,"issuesToFix":[],"prId":"e0cfd7a9-2736-47d5-9697-bafcc84099d8","prPublicId":"e0cfd7a9-2736-47d5-9697-bafcc84099d8","packageManager":"npm","priorityScoreList":[],"projectPublicId":"cf9c5b42-a7d0-446f-a41c-275c56edf456","projectUrl":"https://app.snyk.io/org/bugbaredrums/project/cf9c5b42-a7d0-446f-a41c-275c56edf456?utm_source=github&utm_medium=referral&page=upgrade-pr","prType":"upgrade","templateFieldSources":{"branchName":"default","commitMessage":"default","description":"default","title":"default"},"templateVariants":[],"type":"auto","upgrade":[],"upgradeInfo":{"versionsDiff":2,"publishedDate":"2025-09-20T13:03:07.987Z"},"vulns":[]}'
This PR adds a tracing channel for the
asJsonfunction. APM tools can utilize this channel for their tracing needs. At New Relic, this is the only channel we need. Other vendors may like a bit more, so I'm tagging them here to get their input.I'm opening this in draft mode while the feedback is collected. Once we have "yeah, this is good" from enough people, I will add documentation and convert the PR to ready.
attn: @AbhiPrasad @bengl @bizob2828 @timfish