Skip to content

Conversation

@secondfry
Copy link

@secondfry secondfry commented Aug 28, 2024

Which problem is this PR solving?

I'm observing that instrumentation-undici does not work in Node.js 18.16.0 if code was started with no Internet access. I nailed it down to usage of channel.subscribe(onMessage) which was deprecated in Node.js 18.7.0.

Interestingly enough new API does not pass tests on all Node.js versions between 18.7.0 and 18.18.2. I think this is related to nodejs/node#47520. Thus I'm setting minimal engine version to 18.19.0.

Short description of the changes

Use diagnostics_channel.subscribe(name, onMessage) instead of channel.subscribe(onMessage).

@david-luna
Copy link
Contributor

Hi @secondfry

thanks for contributing to opentelemetry :)

IMHO the change you propose is the way to go since the API is deprecated but I would prefer not to drop support for node versions if possible. Have you thought to use the diagnostics_channel.subscribe API if available and fallback to channel.subscribe if not? this logic on which API should be used can also check for specific node versions if there is a known issue as you mention.

Cheers

@david-luna
Copy link
Contributor

@secondfry did you have time to think about my suggestion on nodejs support?

@david-luna
Copy link
Contributor

Hi @secondfry

If you have no bandwidth I can take this one although I'd go for not dropping support of Node.js versions

@david-luna
Copy link
Contributor

@secondfry this PR includes your proposal along a few changes to keep compatibility
#2457

@secondfry
Copy link
Author

@david-luna I'm terribly sorry for not following through with requested changes and thank you so much for your PR

@secondfry secondfry closed this Oct 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants