Skip to content

feat(node): pino integration #17584

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 37 commits into from
Oct 2, 2025
Merged

feat(node): pino integration #17584

merged 37 commits into from
Oct 2, 2025

Conversation

timfish
Copy link
Collaborator

@timfish timfish commented Sep 10, 2025 •
edited
Loading

This PR:

  • Adds @apm-js-collab/tracing-hooks as a dependency
  • Integrations can register instrumentations which causes the @apm-js-collab/tracing-hooks ESM hook and require patching to be initialised later
  • The @apm-js-collab/* dependencies only get included in a bundle when they are used by an integration
  • Adds a pinoIntegration that:
    • Is not enabled by default (see below)
    • Registers where it needs code injecting into the pino library
    • Hooks the tracing channel events, including the channel added to [email protected]
    • Captures Sentry logs for pino logs
    • Captures in the correct tracing context because this is all sync!
    • Captures exception/message events for the configured eventLevels

Supported Node versions

We can't enable this integration by default because TracingChannel, injected by @apm-js-collab/code-transformer requires Node >= v19.9.0 or v18.19.0.

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Sep 10, 2025 •
edited
Loading

size-limit report 📦

Path Size % Change Change
@sentry/browser 24.23 kB - -
@sentry/browser - with treeshaking flags 22.75 kB - -
@sentry/browser (incl. Tracing) 40.42 kB - -
@sentry/browser (incl. Tracing, Replay) 78.8 kB - -
@sentry/browser (incl. Tracing, Replay) - with treeshaking flags 68.45 kB - -
@sentry/browser (incl. Tracing, Replay with Canvas) 83.47 kB - -
@sentry/browser (incl. Tracing, Replay, Feedback) 95.67 kB - -
@sentry/browser (incl. Feedback) 40.95 kB - -
@sentry/browser (incl. sendFeedback) 28.89 kB - -
@sentry/browser (incl. FeedbackAsync) 33.82 kB - -
@sentry/react 25.96 kB - -
@sentry/react (incl. Tracing) 42.39 kB - -
@sentry/vue 28.75 kB - -
@sentry/vue (incl. Tracing) 42.23 kB - -
@sentry/svelte 24.26 kB - -
CDN Bundle 25.75 kB - -
CDN Bundle (incl. Tracing) 40.31 kB - -
CDN Bundle (incl. Tracing, Replay) 76.55 kB - -
CDN Bundle (incl. Tracing, Replay, Feedback) 82.06 kB - -
CDN Bundle - uncompressed 75.3 kB - -
CDN Bundle (incl. Tracing) - uncompressed 119.31 kB - -
CDN Bundle (incl. Tracing, Replay) - uncompressed 234.47 kB - -
CDN Bundle (incl. Tracing, Replay, Feedback) - uncompressed 247.23 kB - -
@sentry/nextjs (client) 44.4 kB - -
@sentry/sveltekit (client) 40.84 kB - -
@sentry/node-core 50.55 kB +0.15% +72 B 🔺
@sentry/node 154.16 kB +0.05% +68 B 🔺
@sentry/node - without tracing 92.41 kB +0.09% +80 B 🔺
@sentry/aws-serverless 106.12 kB +0.08% +84 B 🔺

View base workflow run

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Sep 10, 2025 •
edited
Loading

node-overhead report 🧳

Note: This is a synthetic benchmark with a minimal express app and does not necessarily reflect the real-world performance impact in an application.

Scenario Requests/s % of Baseline Prev. Requests/s Change %
GET Baseline 8,442 - 9,636 -12%
GET With Sentry 1,391 16% 1,444 -4%
GET With Sentry (error only) 6,076 72% 6,177 -2%
POST Baseline 1,210 - 1,179 +3%
POST With Sentry 528 44% 496 +6%
POST With Sentry (error only) 1,070 88% 1,022 +5%
MYSQL Baseline 3,334 - 3,310 +1%
MYSQL With Sentry 431 13% 375 +15%
MYSQL With Sentry (error only) 2,689 81% 2,658 +1%

View base workflow run

@timfish
Copy link
Collaborator Author

timfish commented Sep 10, 2025

This is blocked by:

@timfish
Copy link
Collaborator Author

timfish commented Sep 15, 2025 •
edited
Loading

I think these both need to be merged and released before we can use the hooks in production:

@timfish timfish mentioned this pull request Sep 18, 2025
Closed
@timfish timfish marked this pull request as ready for review September 19, 2025 20:46
@timfish timfish requested a review from AbhiPrasad September 19, 2025 20:46
cursor[bot]

This comment was marked as outdated.

Sign in to view

cursor[bot]

This comment was marked as outdated.

Sign in to view

AbhiPrasad
AbhiPrasad reviewed Sep 24, 2025
View reviewed changes
Copy link
Member

@AbhiPrasad AbhiPrasad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really happy with this. Once we lock down the options surface, we should be good to merge!

@timfish
Copy link
Collaborator Author

timfish commented Sep 24, 2025

Just realised we probably also want an integration test for the latest Pino that includes the tracing channel!

@timfish timfish force-pushed the timfish/feat/pino-integration branch from 194f02e to ac49a64 Compare October 1, 2025 23:25
cursor[bot]

This comment was marked as outdated.

Sign in to view

@timfish timfish requested a review from AbhiPrasad October 2, 2025 01:07
@AbhiPrasad AbhiPrasad merged commit e88c084 into develop Oct 2, 2025
364 of 371 checks passed
@AbhiPrasad AbhiPrasad deleted the timfish/feat/pino-integration branch October 2, 2025 13:57
@AbhiPrasad AbhiPrasad mentioned this pull request Oct 2, 2025
Merged
AbhiPrasad added a commit that referenced this pull request Oct 2, 2025
@AbhiPrasad
feat: Remove @sentry/pino-transport package (#17851)
632f0b9 
This package was added in
#16652, but with the
merge of #17584 we
can remove it. We will recommend users use the `pinoIntegration` because
it's way easier to set up and tracing works ootb.

This package was never published (no craft config) so it's safe to
remove.
@fossabot fossabot bot mentioned this pull request Oct 7, 2025
Closed
@AbhiPrasad AbhiPrasad mentioned this pull request Oct 8, 2025
Merged
6 tasks
AbhiPrasad added a commit to getsentry/sentry-docs that referenced this pull request Oct 8, 2025
@AbhiPrasad
feat: Document pino integration (#15166)
3fbc47b 
<!-- Use this checklist to make sure your PR is ready for merge. You may
delete any sections you don't need. -->

## DESCRIBE YOUR PR

Documents change in
getsentry/sentry-javascript#17584

## IS YOUR CHANGE URGENT?  

Help us prioritize incoming PRs by letting us know when the change needs
to go live.
- [ ] Urgent deadline (GA date, etc.): <!-- ENTER DATE HERE -->
- [ ] Other deadline: <!-- ENTER DATE HERE -->
- [x] None: Not urgent, can wait up to 1 week+

## SLA

- Teamwork makes the dream work, so please add a reviewer to your PRs.
- Please give the docs team up to 1 week to review your PR unless you've
added an urgent due date to it.
Thanks in advance for your help!

## PRE-MERGE CHECKLIST

*Make sure you've checked the following before merging your changes:*

- [ ] Checked Vercel preview for correctness, including links
- [ ] PR was reviewed and approved by any necessary SMEs (subject matter
experts)
- [ ] PR was reviewed and approved by a member of the [Sentry docs
team](https://github.com/orgs/getsentry/teams/docs)

## LEGAL BOILERPLATE

<!-- Sentry employees and contractors can delete or ignore this section.
-->

Look, I get it. The entity doing business as "Sentry" was incorporated
in the State of Delaware in 2015 as Functional Software, Inc. and is
gonna need some rights from me in order to utilize my contributions in
this here PR. So here's the deal: I retain all rights, title and
interest in and to my contributions, and by keeping this boilerplate
intact I confirm that Sentry can use, modify, copy, and redistribute my
contributions, under Sentry's choice of terms.

## EXTRA RESOURCES

- [Sentry Docs contributor guide](https://docs.sentry.io/contributing/)
@fossabot fossabot bot mentioned this pull request Oct 9, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cursor cursor[bot] cursor[bot] left review comments

@AbhiPrasad AbhiPrasad AbhiPrasad approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@timfish @AbhiPrasad