Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom

fix(instrumentation-fastify): add missing module export #2633

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(instrumentation-fastify): add missing module export #2633

wants to merge 1 commit into from

Conversation

tsmithhisler
Copy link

@tsmithhisler tsmithhisler commented Jan 8, 2025 •
edited
Loading

Which problem is this PR solving?

This PR adds an additional export that is part of the fastify public API to the InstrumentationNodeModuleDefinition patch function. Without this patch,

Short description of the changes

fastify v4.8+ exports an object named errorCodes as both a property of the default export and as a named export.

The export is documented at https://github.com/fastify/fastify/blob/4.x/docs/Reference/Errors.md?plain=1#L236.

This closes #2027.

@tsmithhisler tsmithhisler requested a review from a team as a code owner January 8, 2025 22:25
@linux-foundation-easycla Linux Foundation: EasyCLA
Copy link

linux-foundation-easycla bot commented Jan 8, 2025 •
edited
Loading

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: tsmithhisler / name: Tom Smithhisler (2727df2)

@github-actions github-actions bot added pkg:instrumentation-fastify pkg-status:unmaintained This package is unmaintained. Only bugfixes may be acceped until a new owner has been found. labels Jan 8, 2025
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jan 9, 2025

This package does not have an assigned component owner and is considered unmaintained. As such this package is in feature-freeze and this PR will be closed with 14 days unless a new owner or a sponsor (a member of @open-telemetry/javascript-approvers) for the feature is found. It is the responsibility of the author to find a sponsor for this feature.
Are you familiar with this package? Consider becoming a component owner.

pichlermarc
pichlermarc previously approved these changes Jan 9, 2025
View reviewed changes
Copy link
Member

@pichlermarc pichlermarc left a comment

Choose a reason for hiding this comment

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

thanks 👍

@pichlermarc pichlermarc added bug Something isn't working and removed pkg-status:unmaintained:autoclose-scheduled labels Jan 9, 2025
@codecov Codecov
Copy link

codecov bot commented Jan 9, 2025 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.79%. Comparing base (3350583) to head (2727df2).
Report is 1 commits behind head on main.

Additional details and impacted files 
@@           Coverage Diff           @@
##             main    #2633   +/-   ##
=======================================
  Coverage   90.79%   90.79%           
=======================================
  Files         169      169           
  Lines        8059     8061    +2     
  Branches     1645     1646    +1     
=======================================
+ Hits         7317     7319    +2     
  Misses        742      742
Files with missing lines Coverage Δ
...try-instrumentation-fastify/src/instrumentation.ts 95.38% <100.00%> (+0.07%) ⬆️

@pichlermarc
Copy link
Member

oh this still needs some lint fixes (npm run lint:fix)

@pichlermarc pichlermarc dismissed their stale review January 9, 2025 12:32

failing tests

@pichlermarc
Copy link
Member

Looks like tests are failing for older fastify versions (4.5.1, for instance).

@tsmithhisler
Copy link
Author

It turns out the errorCodes export was added in v4.8.0. I updated the test and code to support that. Also the lint fixes should be in as well.

@tsmithhisler
fix(instrumentation-fastify): add missing module export
2727df2 
fastify v4.8+ exports an object named `errorCodes` as both a property of the default export and as a named export.

The export is documented at https://github.com/fastify/fastify/blob/4.x/docs/Reference/Errors.md?plain=1#L236.

This closes open-telemetry#2027.
@tsmithhisler
Copy link
Author

FYI I rebased off of latest main and updated commit body to reflect v4.8 instead of general v4.x.

# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@pichlermarc pichlermarc Awaiting requested review from pichlermarc

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
bug Something isn't working pkg:instrumentation-fastify pkg-status:unmaintained This package is unmaintained. Only bugfixes may be acceped until a new owner has been found.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Fastify's _patchConstructor causing errorCodes to be inaccessible
2 participants
@tsmithhisler @pichlermarc