aws / aws-xray-sdk-node

The official AWS X-Ray SDK for Node.js.
Apache License 2.0
267 stars 155 forks source link

node 18 fetch capture #531

Open danilobuerger opened 2 years ago

danilobuerger commented 2 years ago

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.

carolabadeer commented 2 years ago

Hello! It looks like opentelemetry-js introduced instrumentation for the new native fetch in Node a few days ago. The npm package opentelemetry/instrumentation-fetch was released 13 days ago, and the documentation on the opentelemetry-js repo shows an example of how to use this instrumentation! Please keep in mind that this package is experimental and still under active development.

dreamorosi commented 1 year ago

@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?

willarmiros commented 1 year ago

@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 sdk_contrib.

JamesKyburz commented 1 year ago

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)
  }
}
mstykow commented 1 year ago

Any progress? Node 20 is entering LTS in October 2023 and fetch is no longer experimental.

srprash commented 1 year ago

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.

WtfJoke commented 10 months ago

Any progress? Node 20 is entering LTS in October 2023 and fetch is no longer experimental.

To be fair, it is still considered as experimental in Node 20.

Introduced with Node 17 (optin behind an experimental flag) Then in Node 18, still experimental, but optout behind a no-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.

aripalo commented 4 months ago

Considering:

I think it might be good to start the work on supporting fetch with X-Ray SDK sooner rather than later.

dreamorosi commented 4 months ago

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