-
Notifications
You must be signed in to change notification settings - Fork 157
node 18 fetch capture #531
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
Comments
Hello! |
@carolabadeer, what about supporting it in this X-Ray SDK? At least for now there's no deprecation date and as you pointed out the Otel package is experimental, so what should customer use once Nodejs 18 runtime is released? |
@dreamorosi to be fair the Node v18 fetch is itself experimental :) At this time we do not have plans to support instrumenting fetch in the X-Ray SDK, and continue to recommend using OTel for instrumenting Node fetch. We would be open to a PR for this instrumentation with sufficient testing in |
Not pretty, and ignoring manual mode. // reverse engineered from https://github.com/aws/aws-xray-sdk-node/blob/93a4f31de2974c10b25ec317bfadd07aabcc9015/packages/core/lib/patchers/http_p.js
// we don't need to take care of manual mode
function traceFetch() {
const fetch = globalThis.fetch
globalThis.fetch = async function (resource, options) {
const traceHeader =
resource.headers?.get('X-Amzn-Trace-Id') ?? options?.['X-Amzn-Trace-Id']
if (!traceHeader) {
const parent = AWSXRay.resolveSegment()
if (parent) {
const url = resource?.url ?? resource
const method = resource?.method ?? options?.method ?? 'GET'
const { hostname } = new URL(url)
const subsegment = parent.notTraced
? parent.addNewSubsegmentWithoutSampling(hostname)
: parent.addNewSubsegment(hostname)
const root = parent.segment ? parent.segment : parent
subsegment.namespace = 'remote'
if (!options) {
options = {}
}
if (!options.headers) {
options.headers = {}
}
options.headers['X-Amzn-Trace-Id'] =
'Root=' +
root.trace_id +
';Parent=' +
subsegment.id +
';Sampled=' +
(subsegment.notTraced ? '0' : '1')
subsegment.http = {
request: {
url,
method
}
}
try {
const res = await fetch.call(globalThis, resource, options)
if (res.status === 429) {
subsegment.addThrottleFlag()
} else if (!res.ok) {
subsegment.addErrorFlag()
}
const cause = AWSXRay.utils.getCauseTypeFromHttpStatus(res.status)
if (cause) {
subsegment[cause] = true
}
const contentLength = res.headers.get('content-length')
subsegment.http.response = {
status: res.status,
...(contentLength && { content_length: contentLength })
}
subsegment.close()
return res
} catch (err) {
subsegment.close(err)
throw err
}
}
}
return await fetch.call(globalThis, resource, options)
}
} |
Any progress? Node 20 is entering LTS in October 2023 and |
Hello. We still don't have any near term plans to support fetch instrumentation in the X-Ray SDK. We recommend to use OpenTelemetry instrumentation if possible. |
To be fair, it is still considered as experimental in Node 20. Introduced with Node 17 (optin behind an experimental flag) Stable in Node 21 onwards (therefore not yet in Node 20 LTS) On a side note (my opinion only): Its easy to read it wrong.. and I guess its probably pretty stable on Node 20 when its released as stable in Node 21. |
Considering:
I think it might be good to start the work on supporting |
They actually already added it as a separate package in the latest release: https://github.com/aws/aws-xray-sdk-node/blob/master/CHANGELOG.md |
The documentation is located here: https://github.com/aws/aws-xray-sdk-node/tree/master/sdk_contrib/fetch const { captureFetchGlobal } = require('aws-xray-sdk-fetch');
// To use globally defined fetch (available in NodeJS 18+)
const fetch = captureFetchGlobal();
const result = await fetch('https://foo.com'); |
node 18 introduced a native fetch: https://nodejs.org/de/blog/announcements/v18-release-announce/#fetch-experimental
how can we capture this? It seems like captureHTTPsGlobal does not work.
The text was updated successfully, but these errors were encountered: