search

autotelic / fastify-opentelemetry

A Fastify plugin that utilizes the OpenTelemetry API to provide request tracing.
MIT License
76 stars 12 forks source link

chore(fastify): upgrade fastify to 4.x #47

Closed gjuchault closed 2 years ago

gjuchault commented 2 years ago

Hello!

Fastify just released their version 4, so here's the update in this package.

I also upgraded engines.node to v14 (in Github Actions as well) as I think fastify started to use Optional chaining operator, introduced in node 14.

3 tests are failing with wrapRoutes, so I'm investigating them at the moment

gjuchault commented 2 years ago

The issue with tests seems to come from the fact that onRoute is only called with the HEAD method, and not with the GET one. What's weird is this happens even when giving exposeHeadRoutes: false to fastify' settings

HW13 commented 2 years ago

Thanks for taking this on @gjuchault!

It looks like the issues with the tests can be resolved by adding await to the fastify.register and fastify.get calls in both the failing tests (here and here).

gjuchault commented 2 years ago

@HW13 Nice catch. How did you get the idea? Is it a breaking change in fastify v4?

HW13 commented 2 years ago

While I can't seem to find an actual list of breaking changes, this seems to be one. This post regarding the v4 release mentions route registration becoming synchronous - looks like only fastify.register needs the await (but you've already implemented that way 👍)