add addHook handler to fastify instrumentation #936
Conversation
| t.test('should wrap request/response hooks', async (t) => { | ||
| let ok = false | ||
|
|
||
| fastify.addHook('onRequest', (request, reply, done) => { |
There was a problem hiding this comment.
personally I would put these kind of tests in the naming suites. I'd abstract away the creation of the fastify server and add hooks for every single request lifecycle event. Then assert the segment names which means it's prob best to name each plugin function instead of arrow functions, but we can chat IRL just ping me
There was a problem hiding this comment.
I ended up placing them in their own test file because it seems the hook isn't affected by the differences between v2 and v3. LMK if this is sub-optimal!
|
OK! Hook wrapper updated, tests added. The tests pass but only because there are some lifecycle events that don't get caught, which I then commented out. Specifically:
I'll dive into this tomorrow but I thought getting this far was a nice milestone for the day. |
|
Sooooo as of this morning, those lifecycle events that mysteriously weren't firing yesterday... totally do fire this morning! 🤷♀️ I'll push what I have momentarily and then we'll let the CI make its call. |
|
Looks like we're good to go for review! Tests pass :D |
…to properly make requests and register/assert segments
Proposed Release Notes
addHook.Links
Details
As of this writing, this PR does not add additional tests. (I'm working on it...!) Any pointers about testing this would be appreciated, cc @bizob2828