Skip to content

feat(node): Migrate to @fastify/otel #15542

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

Open
wants to merge 30 commits into
base: develop
Choose a base branch
from

Conversation

onurtemizkan
Copy link
Collaborator

@onurtemizkan onurtemizkan commented Feb 28, 2025

Resolves: #15130

Example events:

@onurtemizkan onurtemizkan force-pushed the onur/fastify-otel-migration branch 2 times, most recently from 38047fd to 6c347ee Compare February 28, 2025 15:20
@@ -101,7 +101,8 @@
"import-in-the-middle": "^1.13.0"
},
"devDependencies": {
"@types/node": "^18.19.1"
"@types/node": "^18.19.1",
"fastify": "4.18.0"
Copy link
Member

Choose a reason for hiding this comment

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

Can we avoid this and just vendor in the minimal interface we need to satisfy the types?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sure 👍

Copy link
Contributor

github-actions bot commented Feb 28, 2025

size-limit report 📦

Path Size % Change Change
@sentry/browser 23.28 KB - -
@sentry/browser - with treeshaking flags 23.12 KB - -
@sentry/browser (incl. Tracing) 36.99 KB - -
@sentry/browser (incl. Tracing, Replay) 74.17 KB - -
@sentry/browser (incl. Tracing, Replay) - with treeshaking flags 67.55 KB - -
@sentry/browser (incl. Tracing, Replay with Canvas) 78.83 KB - -
@sentry/browser (incl. Tracing, Replay, Feedback) 90.65 KB - -
@sentry/browser (incl. Feedback) 39.68 KB - -
@sentry/browser (incl. sendFeedback) 27.9 KB - -
@sentry/browser (incl. FeedbackAsync) 32.67 KB - -
@sentry/react 25.09 KB - -
@sentry/react (incl. Tracing) 38.91 KB - -
@sentry/vue 27.51 KB - -
@sentry/vue (incl. Tracing) 38.71 KB - -
@sentry/svelte 23.32 KB - -
CDN Bundle 24.5 KB - -
CDN Bundle (incl. Tracing) 36.98 KB - -
CDN Bundle (incl. Tracing, Replay) 72 KB - -
CDN Bundle (incl. Tracing, Replay, Feedback) 77.16 KB - -
CDN Bundle - uncompressed 71.49 KB - -
CDN Bundle (incl. Tracing) - uncompressed 109.34 KB - -
CDN Bundle (incl. Tracing, Replay) - uncompressed 220.63 KB - -
CDN Bundle (incl. Tracing, Replay, Feedback) - uncompressed 233.16 KB - -
@sentry/nextjs (client) 40.54 KB - -
@sentry/sveltekit (client) 37.44 KB - -
@sentry/node 150.83 KB +5.2% +7.45 KB 🔺
@sentry/node - without tracing 96.51 KB - -
@sentry/aws-serverless 120.84 KB +0.01% +8 B 🔺

View base workflow run

@onurtemizkan onurtemizkan force-pushed the onur/fastify-otel-migration branch 2 times, most recently from 88d946e to 112c80a Compare March 5, 2025 18:27
@onurtemizkan onurtemizkan marked this pull request as ready for review March 5, 2025 18:33
@andreiborza andreiborza self-assigned this Mar 10, 2025
@@ -0,0 +1,32 @@
/*
Copy link
Member

Choose a reason for hiding this comment

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

For the files we vendored in from OTEL, we should also link to the git sha we used of the versioned files.

Copy link
Member

@andreiborza andreiborza left a comment

Choose a reason for hiding this comment

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

Great job! This is looking nice.

@@ -65,6 +65,7 @@
"access": "public"
},
"dependencies": {
"@fastify/otel": "0.5.0",
Copy link
Member

Choose a reason for hiding this comment

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

l: We can bump to 0.5.1

Comment on lines 19 to 52
// /**
// * Minimal request type containing properties around route information.
// * Works for Fastify 3, 4 and presumably 5.
// *
// * Based on https://github.com/fastify/fastify/blob/ce3811f5f718be278bbcd4392c615d64230065a6/types/request.d.ts
// */
// // eslint-disable-next-line @typescript-eslint/no-explicit-any
// interface MinimalFastifyRequest extends Record<string, any> {
// method?: string;
// // since fastify@4.10.0
// routeOptions?: {
// url?: string;
// };
// routerPath?: string;
// }

// /**
// * Minimal reply type containing properties needed for error handling.
// *
// * Based on https://github.com/fastify/fastify/blob/ce3811f5f718be278bbcd4392c615d64230065a6/types/reply.d.ts
// */
// // eslint-disable-next-line @typescript-eslint/no-explicit-any
// interface MinimalFastifyReply extends Record<string, any> {
// statusCode: number;
// }

// // We inline the types we care about here
// interface Fastify {
// // eslint-disable-next-line @typescript-eslint/no-explicit-any
// version: string;
// register: (plugin: any) => Fastify;
// after: (listener?: (err: Error) => void) => Fastify;
// addHook: (name: string, handler: (...params: unknown[]) => void) => Fastify;
// }
Copy link
Member

Choose a reason for hiding this comment

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

l: Do we want to keep this around?

@onurtemizkan onurtemizkan requested review from a team as code owners April 3, 2025 14:40
@onurtemizkan onurtemizkan force-pushed the onur/fastify-otel-migration branch 2 times, most recently from 2bd1c74 to 0308803 Compare April 3, 2025 14:54
@onurtemizkan
Copy link
Collaborator Author

@andreiborza - I updated the PR. Waiting for fastify/otel#35 to be released.

@mydea
Copy link
Member

mydea commented Apr 8, 2025

I would assume this also closes #12844?

@andreiborza
Copy link
Member

andreiborza commented Apr 14, 2025

@andreiborza - I updated the PR. Waiting for fastify/otel#35 to be released.

looks like it's been released with 0.5.2 🚀

Make sure to update the lock file when you bump.

@onurtemizkan onurtemizkan force-pushed the onur/fastify-otel-migration branch 3 times, most recently from b1847dc to 275a14d Compare April 14, 2025 14:23
@onurtemizkan
Copy link
Collaborator Author

@andreiborza, @mydea. I bumped the version. I think it's ready to go now.

@onurtemizkan onurtemizkan force-pushed the onur/fastify-otel-migration branch from 43c201f to 964cf0c Compare April 14, 2025 20:31
@onurtemizkan onurtemizkan force-pushed the onur/fastify-otel-migration branch from 047460d to fc6f51f Compare April 24, 2025 10:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
4 participants