Allow blocking on fastify cookie #5910

IlyasShabi · 2025-06-18T09:34:16Z

What does this PR do?

Allow blocking on fastify cookie @fastify/cookie library

Motivation

Plugin Checklist

Additional Notes

github-actions · 2025-06-18T09:34:46Z

Overall package size

Self size: 9.74 MB
Deduped: 106.25 MB
No deduping: 106.77 MB

Dependency sizes

| name | version | self size | total size | |------|---------|-----------|------------| | @datadog/libdatadog | 0.7.0 | 35.02 MB | 35.02 MB | | @datadog/native-appsec | 8.5.2 | 19.33 MB | 19.34 MB | | @datadog/native-iast-taint-tracking | 4.0.0 | 11.72 MB | 11.73 MB | | @datadog/pprof | 5.8.2 | 9.56 MB | 9.93 MB | | @opentelemetry/core | 1.30.1 | 908.66 kB | 7.16 MB | | protobufjs | 7.5.3 | 2.95 MB | 5.6 MB | | @datadog/wasm-js-rewriter | 4.0.1 | 2.85 MB | 3.58 MB | | @datadog/native-metrics | 3.1.1 | 1.02 MB | 1.43 MB | | @opentelemetry/api | 1.8.0 | 1.21 MB | 1.21 MB | | import-in-the-middle | 1.14.0 | 120.58 kB | 841.68 kB | | source-map | 0.7.4 | 226 kB | 226 kB | | opentracing | 0.14.7 | 194.81 kB | 194.81 kB | | lru-cache | 7.18.3 | 133.92 kB | 133.92 kB | | pprof-format | 2.1.0 | 111.69 kB | 111.69 kB | | @datadog/sketches-js | 2.1.1 | 109.9 kB | 109.9 kB | | lodash.sortby | 4.7.0 | 75.76 kB | 75.76 kB | | ignore | 5.3.2 | 53.63 kB | 53.63 kB | | istanbul-lib-coverage | 3.2.2 | 34.37 kB | 34.37 kB | | rfdc | 1.4.1 | 27.15 kB | 27.15 kB | | @isaacs/ttlcache | 1.4.1 | 25.2 kB | 25.2 kB | | dc-polyfill | 0.1.9 | 25.11 kB | 25.11 kB | | tlhunter-sorted-set | 0.1.0 | 24.94 kB | 24.94 kB | | shell-quote | 1.8.2 | 23.54 kB | 23.54 kB | | limiter | 1.1.5 | 23.17 kB | 23.17 kB | | retry | 0.13.1 | 18.85 kB | 18.85 kB | | semifies | 1.0.0 | 15.84 kB | 15.84 kB | | jest-docblock | 29.7.0 | 8.99 kB | 12.76 kB | | crypto-randomuuid | 1.0.0 | 11.18 kB | 11.18 kB | | ttl-set | 1.0.0 | 4.61 kB | 9.69 kB | | mutexify | 1.4.0 | 5.71 kB | 8.74 kB | | path-to-regexp | 0.1.12 | 6.6 kB | 6.6 kB | | koalas | 1.0.2 | 6.47 kB | 6.47 kB | | module-details-from-path | 1.0.4 | 3.96 kB | 3.96 kB |

_{🤖 This report was automatically generated by heaviest-objects-in-the-universe}

codecov · 2025-06-18T09:35:21Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 80.79%. Comparing base (a408cd7) to head (5e07eed).
Report is 15 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #5910      +/-   ##
==========================================
+ Coverage   80.75%   80.79%   +0.03%     
==========================================
  Files         462      467       +5     
  Lines       19924    20019      +95     
==========================================
+ Hits        16090    16174      +84     
- Misses       3834     3845      +11

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

datadog-datadog-prod-us1 · 2025-06-18T09:42:02Z

Datadog Report

Branch report: ishabi/fastify-cookies
Commit report: a280256
Test service: dd-trace-js-integration-tests

✅ 0 Failed, 1253 Passed, 0 Skipped, 17m 36.43s Total Time

pr-commenter · 2025-06-18T09:47:08Z

Benchmarks

Benchmark execution time: 2025-06-24 10:06:39

Comparing candidate commit 5e07eed in PR branch ishabi/fastify-cookies with baseline commit a408cd7 in branch master.

Found 0 performance improvements and 0 performance regressions! Performance is the same for 956 metrics, 32 unstable metrics.

simon-id · 2025-06-23T16:42:05Z

packages/datadog-instrumentations/src/fastify.js

          arguments[arguments.length - 1] = function (err) {
            publishError(err, req)

+            const hasCookies = request.cookies && Object.keys(request.cookies).length > 0


eeeeh, usually we check that in the listener instead, not sure it makes a big difference, but when debugging it's nice to still have the DC event fired, even if it is with empty object

I don't want to create an AbortController as it's an expensive operation

I talked with Ruben in slack about this last time, and from what I understand, it's really not THAT expensive, it's just a new object, nothing crazy

I think his definition of "expensive" wasn't what we understood from it

simon-id · 2025-06-23T16:43:02Z

packages/datadog-instrumentations/src/fastify.js

+        // done callback is always the last argument
+        const done = arguments[arguments.length - 1]



why add this ? there can be more arguments between reply and done ?
if that's the case, there was a bug before no ?

Hook could have 3 or 4 args https://fastify.dev/docs/latest/Reference/Hooks
Could you check now

simon-id · 2025-06-23T16:44:00Z

packages/datadog-instrumentations/src/fastify.js

@@ -45,16 +46,35 @@ function wrapAddHook (addHook) {

    if (typeof fn !== 'function') return addHook.apply(this, arguments)

-    arguments[arguments.length - 1] = shimmer.wrapFunction(fn, fn => function (request, reply, done) {
+    arguments[arguments.length - 1] = shimmer.wrapFunction(fn, fn => function () {


if you remove the arguments from the function signature, you potentially break any behavior in the framework, that might be checking "func.length" to detect the type of callback it is and what arguments to pass it

it is checked 2 lines above

what ? I think you're misunderstanding my comment

he removed the request, reply, done arguments from the function signature, thus if code in fastify checks for this callback length, it will be wrong no ?

yes sorry, I misunderstood it. you're right.

simon-id · 2025-06-23T16:47:52Z

packages/dd-trace/test/appsec/index.fastify.plugin.spec.js

 const zlib = require('zlib')
 const fs = require('node:fs')
 const agent = require('../plugins/agent')
 const appsec = require('../../src/appsec')
 const Config = require('../../src/config')
 const { json } = require('../../src/appsec/blocked_templates')

-withVersions('fastify', 'fastify', version => {
+withVersions('fastify', 'fastify', '>=2', version => {


aren't you removing coverage for fastify v1 by doing this ?

We do not support fastify v1, I will add this to the documentation

packages/dd-trace/test/plugins/externals.json

IlyasShabi added semver-patch appsec labels Jun 18, 2025

IlyasShabi force-pushed the ishabi/fastify-cookies branch from bdfd56f to d4fcd49 Compare June 18, 2025 09:45

Allow blocking on fastify cookie

cc4df74

IlyasShabi force-pushed the ishabi/fastify-cookies branch from d4fcd49 to cc4df74 Compare June 18, 2025 09:48

IlyasShabi added 3 commits June 18, 2025 17:07

Merge branch 'master' into ishabi/fastify-cookies

194d1ca

remove duplicate fastifyBodyParser

40618aa

Merge branch 'master' into ishabi/fastify-cookies

ba54d6b

IlyasShabi added the do-not-merge/WIP label Jun 19, 2025

IlyasShabi added 3 commits June 23, 2025 16:16

support parsing cookies in all hooks

fc4342b

linter

bd41db1

skip preparsing hook with fastify 2

0aa385a

IlyasShabi removed the do-not-merge/WIP label Jun 23, 2025

linter

f70bf62

IlyasShabi marked this pull request as ready for review June 23, 2025 15:43

IlyasShabi requested review from a team as code owners June 23, 2025 15:43

simon-id reviewed Jun 23, 2025

View reviewed changes

packages/dd-trace/test/plugins/externals.json Show resolved Hide resolved

do not publish cookies on each hook

5e07eed

		// done callback is always the last argument
		const done = arguments[arguments.length - 1]

Allow blocking on fastify cookie #5910

Are you sure you want to change the base?

Allow blocking on fastify cookie #5910

Conversation

IlyasShabi commented Jun 18, 2025

What does this PR do?

Motivation

Plugin Checklist

Additional Notes

Uh oh!

github-actions bot commented Jun 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Overall package size

Uh oh!

codecov bot commented Jun 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

datadog-datadog-prod-us1 bot commented Jun 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Datadog Report

Uh oh!

pr-commenter bot commented Jun 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Benchmarks

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Jun 18, 2025 •

edited

Loading

codecov bot commented Jun 18, 2025 •

edited

Loading

datadog-datadog-prod-us1 bot commented Jun 18, 2025 •

edited

Loading

pr-commenter bot commented Jun 18, 2025 •

edited

Loading