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

fix(instrumentation): Fix optional property types #4833

Merged
merged 1 commit into from
Aug 15, 2024

Conversation

alecmev
Copy link
Contributor

@alecmev alecmev commented Jun 30, 2024

Which problem is this PR solving?

Fix #4712, related to #3713.

TypeScript emits InstrumentationNodeModuleDefinition with | undefined for some reason, making it incompatible with InstrumentationModuleDefinition under exactOptionalPropertyTypes: true.

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

I'm using OpenTelemetry indirectly via Sentry. I applied this patch locally to the latest version of @opentelemetry/instrumentation and it resolved the issue.

Checklist:

  • Followed the style guidelines of this project
  • Unit tests have been added
  • Documentation has been updated

@alecmev alecmev requested a review from a team June 30, 2024 21:24
@alecmev
Copy link
Contributor Author

alecmev commented Jul 16, 2024

@pichlermarc @dyladan Friendly ping 😉

@alecmev
Copy link
Contributor Author

alecmev commented Aug 13, 2024

@pichlermarc @dyladan @legendecas @blumamir Friendly ping!

@legendecas
Copy link
Member

Would you mind updating the PR title and commit message according to the guide https://github.com/open-telemetry/opentelemetry-js/blob/main/CONTRIBUTING.md#conventional-commit ? thank you!

TypeScript emits InstrumentationNodeModuleDefinition with " | undefined"
for some reason, making it incompatible with
InstrumentationModuleDefinition under exactOptionalPropertyTypes.
@alecmev alecmev changed the title Fix #4712 fix(instrumentation): Fix optional property types Aug 13, 2024
@alecmev
Copy link
Contributor Author

alecmev commented Aug 13, 2024

@legendecas Sure thing, done 😉

Copy link
Member

@legendecas legendecas left a comment

Choose a reason for hiding this comment

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

Thank you!

@legendecas legendecas added this pull request to the merge queue Aug 15, 2024
Merged via the queue into open-telemetry:main with commit bb67268 Aug 15, 2024
19 checks passed
@alecmev
Copy link
Contributor Author

alecmev commented Aug 18, 2024

Thanks! 👍

@alecmev alecmev deleted the fix-4712 branch August 18, 2024 12:07
Zirak pushed a commit to Zirak/opentelemetry-js that referenced this pull request Sep 14, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

InstrumentationBase#init triggers cascading errors
2 participants