aws/aws-xray-sdk-node

captureHTTPsGlobal now results in `TypeError: Cannot redefine property: request`

mrowles opened this issue ยท 13 comments

When packaging and running my AWS Lambda function using serverless + webpack with new versions of aws-sdk and aws-xray-sdk-core, I receive this error: TypeError: Cannot redefine property: request

import * as AWS from 'aws-sdk';
// import * as http from 'http';
import * as https from 'https';
import { ObjectType, PermissionRole, PrivacyRating } from './gql-types';
import axios from 'axios';
import { decrypt } from './utils/kms.service';

captureAWS(AWS);
// captureHTTPsGlobal(http, true) ; <-- having this uncommented causes the issue
// captureHTTPsGlobal(https, true); <-- having this uncommented causes the issue

AWS.config.update({
  httpOptions: {
    agent: new https.Agent({
      keepAlive: true,
      maxSockets: Infinity,
    }),
  },
});

Package versions:

"node": "14",
"aws-xray-sdk-core": "3.3.4",
"aws-sdk": "2.1074.0"

I think reverting to "3.2.0" fixes this? I'm going to do more testing and confirm.

Edit: yes, can confirm, downgrading to 3.2.0 fixes this.

Hi @mrowles,

Thanks for raising this. Definitely seems like a problem because we added TypeScript support, which changed the way we bundle our final package in 3.3.x of this SDK. It's possible that this relates to Webpack, but probably more likely that this is similar to #450. Could you try the ts-ignore workarounds suggested in that post to see if that helps with the issue?

This way we can see if the same fix would apply.

I am seeing the same issue with CDK 2.15.0 (which uses esbuild), when the bundling output format is set to OutputFormat.ESM. Reverting to CommonJS (OutputFormat.CJS) resolve it, at the expense of much larger bundle sizes.

Lambda code:

import { DynamoDBClient } from '@aws-sdk/client-dynamodb';
import { DynamoDBDocumentClient } from '@aws-sdk/lib-dynamodb';
import { captureAWSv3Client, captureHTTPsGlobal } from 'aws-xray-sdk';
import * as https from 'https';

const dynamoDbClient = new DynamoDBClient({ });
const dynamoDb = DynamoDBDocumentClient.from(dynamoDbClient);
captureAWSv3Client(dynamoDbClient);
captureHTTPsGlobal(https); // <-- fails here at runtime

CDK resource:

const apiHandlerFn = new njs.NodejsFunction(this, 'AccountDomainApiHandler', {
  timeout: Duration.seconds(10),
  runtime: lambda.Runtime.NODEJS_14_X,
  entry: path.join(__dirname, '../src/account-domain-api/index.ts'),
  bundling: {
    sourceMap: true,
    target: 'es2020',
    format: njs.OutputFormat.ESM, // <-- `njs.OutputFormat.CJS` works
    mainFields: [ 'module', 'main' ]
  },
  environment: {
    NODE_OPTIONS: '--enable-source-maps',
  },
  logRetention: logs.RetentionDays.TWO_WEEKS,
  tracing: lambda.Tracing.ACTIVE
});
willm commented

I'm also hitting this issue when bundling using esbuild, I'm not using cdk but my tscofnig.json is using commonjs already so the suggested fix above doesn't help

I also use commonjs bundles an run into this issue. How can I resolve this?

I migrated to aws-sdk v3, thus I had to upgrade aws-xray-sdk-node because of AWSXRay.captureAWSv3Client(...). I had to hack around and finally ended up with a modified copy of http_p.js file (see gist https://gist.github.com/jepetko/cde1eb40493c6a19dd9ce29de39cbdd8) which I use instead of the original one.

I don't recommend to use it because is hacky & ugly. I also intent to remove it asap.

Is there a plan how to tackle this issue?

Thank you.

it turned out that the redefinition of properties (https://github.com/aws/aws-xray-sdk-node/blob/master/packages/core/lib/patchers/http_p.js#L212) works on default export (while it doesn't when importing the entire module using import * as ... syntax).

I'm seeing the same issue after moving to esbuild from webpack. Reverting to 3.2.0 for the time being.

Also just hit this. I have not been able to get the ts-ignore workaround to work yet (it compiles/deploys, but no tracing, but that might be an issue our side).

Any news about this issue ? having the same issue after moving to esbuild from webpack !

Hi all, I created a backlog item for us to investigate this issue. If you are able to share any reproduction code with us and include details about your environments, that would be very helpful!

it's difficult for me to share all the code of my client but maybe I can extract some part. Can you tell me how I can help you and where to share some parts of the code ?

This seems to be an issue related with migrating to ESM from CJS. More specifically, the import isn't compatible with the patching instrumentation that is currently done via require like AWSXRay.captureHTTPsGlobal(require('http'));

The following summary in the OTel JS issue describes the problem clearly:
open-telemetry/opentelemetry-js#1946 (comment)
The XRay SDKs currently tries to update the HTTP/HTTPS modules via captureHTTPsGlobal to enable tracing on all HTTP clients, but it seems this is not possible to do at runtime via import dependencies. I suppose rather than captureHTTPsGlobal, captureHTTPs could still work although this will enable tracing only on the returned and newly created http module.

There is no near-term plan for full ESM support in XRay SDK. However, in the linked OTel SDK discussion, an experimental feature was developed to support this ESM/import use case. OTel SDK is an alternative tracing instrumentation that can be used to send trace data to AWS XRay, and will need to be used alongside the ADOT Collector rather than the X-Ray Daemon.