Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

can we avoid the punycode deprecation warning stemming from resource-detector-gcp? #520

Open
trentm opened this issue Jan 9, 2025 · 2 comments

Comments

@trentm
Copy link
Member

trentm commented Jan 9, 2025

Just using EDOT Node.js with Node.js v22 or higher results in this Node.js deprecation warning about punycode:

% node --version
v22.13.0
% OTEL_LOG_LEVEL=warn node -r @elastic/opentelemetry-node -e 'console.log("hi")'
hi
(node:36413) [DEP0040] DeprecationWarning: The `punycode` module is deprecated. Please use a userland alternative instead.
(Use `node --trace-deprecation ...` to show where the warning was created)

This is coming from @opentelemetry/resource-detector-gcp:

% npm ls whatwg-url
[email protected] /Users/trentm/tmp/asdf.20250106T092423
└─┬ @elastic/[email protected]
  └─┬ @opentelemetry/[email protected]
    └─┬ [email protected]
      └─┬ [email protected]
        └─┬ [email protected]
          └── [email protected]

node-fetch@2 uses whatwg-url@5 which uses punycode (there is a tr46 module in there that uses punycode as well).

It would be nice to avoid this deprecation warning somehow.
Requiring users to do something like this in "package.json" isn't acceptable:

  "// overrides comment": "Override to avoid punycode warnings in recent versions of Node.JS",
  "overrides": {
    "[email protected]": {
      "whatwg-url": "14.x"
    }
  }
@trentm
Copy link
Member Author

trentm commented Jan 9, 2025

related issues with @opentelemetry/resource-detector-gcp

If you run some code with EDOT Node.js active, say:

% cd examples
% node -r @elastic/opentelemetry-node just-http-request.js
{"name":"elastic-otel-node","level":30,"preamble":true,"distroVersion":"0.6.0","env":{"os":"darwin 24.2.0","arch":"arm64","runtime":"Node.js v22.13.0"},"msg":"start Elastic Distribution of OpenTelemetry Node.js","time":"2025-01-09T20:39:43.938Z"}
(node:37193) [DEP0040] DeprecationWarning: The `punycode` module is deprecated. Please use a userland alternative instead.
(Use `node --trace-deprecation ...` to show where the warning was created)
client response: 200 {
  date: 'Thu, 09 Jan 2025 20:39:43 GMT',
  connection: 'keep-alive',
  'keep-alive': 'timeout=5',
  'transfer-encoding': 'chunked'
}
client response body (123 bytes): {
  "build_date": "2021-09-16T02:05:39Z"...

You get these spans:

------ trace 066f24 (1 span) ------
       span 902948 "GET" (5.0ms, STATUS_CODE_ERROR, SPAN_KIND_CLIENT, GET http://169.254.169.254/computeMetadata/v1/instance)
------ trace 3e4291 (1 span) ------
       span 186179 "GET" (5.8ms, STATUS_CODE_ERROR, SPAN_KIND_CLIENT, GET http://metadata.google.internal./computeMetadata/v1/instance)
------ trace 912aa6 (1 span) ------
       span da2715 "GET" (11.6ms, SPAN_KIND_CLIENT, GET http://127.0.0.1:8200/ -> 200)

Those first two spans are from the GCP resource detector. This is open-telemetry/opentelemetry-js-contrib#2320

Another issue with the GCP detector is this subtle, possible issue if the user application is itself using node-fetch: open-telemetry/opentelemetry-js-contrib#2440 (comment)

@trentm
Copy link
Member Author

trentm commented Jan 9, 2025

options

  1. Remove @opentelemetry/resource-detector-gcp for now? Seems a bit harsh for the possible
  2. Option 2 from shared use of node-fetch by app and resource-detector-gcp can result in https module not getting instrumented open-telemetry/opentelemetry-js-contrib#2440 (comment): Re-write resource-detector-gcp to not use gcp-metadata -> gaxios -> node-fetch.
  3. Option 3 from shared use of node-fetch by app and resource-detector-gcp can result in https module not getting instrumented open-telemetry/opentelemetry-js-contrib#2440 (comment): change resource-detector-gcp to use a private copy of node-fetch.
  4. Get the gcp-metadata module change to update to node-fetch@3 which doesn't have this problem.

Regarding option 4:

  "name": "node-fetch",
  "version": "3.1.1",
  "engines": {
    "node": "^12.20.0 || ^14.13.1 || >=16.0.0"
  },

but:

  "name": "@opentelemetry/resource-detector-gcp",
  "version": "0.32.0",
  "engines": {
    "node": ">=14"
  },

That >=14 includes earlier Node.js v14 versions than ^14.13.1 so
we'd have to bump the min supported version of detector-gcp. Given the coming
OTel JS "SDK 2.0" work that raises the min-node to v18, this might be fine.

Regarding option 2:

I'm not too opposed to trying this. elastic/apm-agent-nodejs
does GCP cloud metadata detection without gcp-metadata.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant