Feature Request: support BigInt serialization in segment metadata

[misterjoshua]

Expected Behaviour

When I add metadata to my trace, I expect the tracer to handle any data I send to it without causing the lambda to crash.

Current Behaviour

It crashes the lambda when I use putMetadata to add a data structure that contains a BigInt or any other format that JSON.stringify cannot encode.

Code snippet

import middy from '@middy/core';
import { Tracer, captureLambdaHandler } from '@aws-lambda-powertools/tracer';

const tracer = new Tracer({ serviceName: 'stream-handler-test' });

export const handler = middy(async (_event) => {
  console.log('1. Call someLogic.doSomething()');
  const response = await someLogic.doSomething();

  console.log('2. Add the response to the trace metadata');

  // Add the response to the trace, so we can see it in the observability
  // platform in case anything goes wrong.
  //
  // This is a very common pattern for me. I don't usually think about
  // what's in the response unless the data is likely to be giant or
  // contains sensitive information in it. It just so happens that
  // this response contains a BigInt, and that's enough to cause the
  // lambda to crash.
  tracer.putMetadata('response', response);

  console.log('3. Return a response from the handler');

  return `Here is a number: ${response.number}`;
}).use(captureLambdaHandler(tracer));

class SomeLogic {
  async doSomething() {
    return {
      number: 1234n,
    };
  }
}

const someLogic = new SomeLogic();

Steps to Reproduce

1. Bundle and install the script above in an AWS Lambda function with tracing enabled.
2. Run the function from the AWS Lambda console.

Possible Solution

I think there are two factors to consider.

1. We should make an effort to encode the types putMetadata receives when JSON.stringify cannot handle them - perhaps we can use a replacer function to encode exotic types.
2. Even if the tracer fails to encode, I don't think it should ever crash the lambda. Perhaps the tracer should be swallowing all exceptions so it doesn't affect the user's code when something goes wrong.

Powertools for AWS Lambda (TypeScript) version

latest

AWS Lambda function runtime

16.x

Packaging format used

npm

Execution logs

START RequestId: c879e6a8-5a2f-4e1b-9240-7dfce8c41719 Version: $LATEST
2023-09-26T17:29:22.684Z	c879e6a8-5a2f-4e1b-9240-7dfce8c41719	INFO	1. Call someLogic.doSomething()
2023-09-26T17:29:22.704Z	c879e6a8-5a2f-4e1b-9240-7dfce8c41719	INFO	2. Add the response to the trace metadata
2023-09-26T17:29:22.704Z	c879e6a8-5a2f-4e1b-9240-7dfce8c41719	INFO	3. Return a response from the handler
2023-09-26T17:29:22.765Z	c879e6a8-5a2f-4e1b-9240-7dfce8c41719	ERROR	Invoke Error 	{"errorType":"TypeError","errorMessage":"Do not know how to serialize a BigInt","originalError":{"errorType":"TypeError","errorMessage":"Do not know how to serialize a BigInt","stack":["TypeError: Do not know how to serialize a BigInt","    at JSON.stringify (<anonymous>)","    at Subsegment.format (/var/task/index.js:2103:19)","    at Object.send (/var/task/index.js:1434:33)","    at Subsegment.flush (/var/task/index.js:2073:26)","    at Subsegment.streamSubsegments (/var/task/index.js:2083:14)","    at Subsegment.close (/var/task/index.js:2046:18)","    at close (/var/task/index.js:9565:24)","    at captureLambdaHandlerAfter (/var/task/index.js:9581:11)","    at runMiddlewares (/var/task/index.js:9799:23)","    at runRequest (/var/task/index.js:9775:13)"]},"stack":["TypeError: Do not know how to serialize a BigInt","    at JSON.stringify (<anonymous>)","    at Subsegment.format (/var/task/index.js:2103:19)","    at Object.send (/var/task/index.js:1434:33)","    at Subsegment.flush (/var/task/index.js:2073:26)","    at Subsegment.streamSubsegments (/var/task/index.js:2083:14)","    at Subsegment.close (/var/task/index.js:2046:18)","    at close (/var/task/index.js:9565:24)","    at captureLambdaHandlerError (/var/task/index.js:9587:11)","    at runMiddlewares (/var/task/index.js:9799:23)","    at runRequest (/var/task/index.js:9782:13)"]}
END RequestId: c879e6a8-5a2f-4e1b-9240-7dfce8c41719
REPORT RequestId: c879e6a8-5a2f-4e1b-9240-7dfce8c41719	Duration: 139.59 ms	Billed Duration: 140 ms	Memory Size: 128 MB	Max Memory Used: 63 MB	Init Duration: 199.81 ms	
XRAY TraceId: 1-651314f2-6e1cb4753036f8091d8a133e	SegmentId: 1937588641412fd8	Sampled: true	

This issue came from Twitter. https://twitter.com/heitor_lessa/status/1706563622694981916

[dreamorosi]

Hi @misterjoshua thanks for taking the time to open the issue.

As you have correctly pointed out , the JSON.stringify() function doesn't know how to serialize the BigInt and so it crashes. Based on this I'd be more inclined in considering this a feature request given that the behavior observed is idiomatic to the language/runtime.

With that said, I do agree that we should do something about this, so I propose a two steps approach:

Step 1

Add an internal try/catch logic to the putMetadata() method so that in case the SDK throws an error, we catch it and log a warning.

This is a change that we can do right away and that we should do regardless of the serialization discussion.

Step 2

Add support for BigInt objects. The change is relatively simple if made on the X-Ray SDK side, however if made on the Powertools side it requires a more roundabout way given that the SDK implementation doesn't extract the serialization in its own method. For this reason we would have to essentially provide a new implementation for two methods of the Segment object.

I'd like to avoid this option if possible since it'll mean we'll have to make sure our implementation doesn't go out of sync with theirs, so for this reason I have opened an issue on the X-Ray SDK repo and offered to contribute a fix: aws/aws-xray-sdk-node#616.

To PoC the fix, I was able write this passing test:

it('handles BigInt types when adding metadata to a segment', () => {
  const segment = new Subsegment('## foo.bar');

  segment.addMetadata('foo', {
    bar: BigInt(123),
  });

  (
    segment as Subsegment & {
      type: string;
      parent_id: string;
      trace_id: string;
    }
  ).format = function format() {
    this.type = 'subsegment';
    if (this.parent) {
      this.parent_id = this.parent.id;
    }
    if (this.segment) {
      this.trace_id = this.segment.trace_id;
    }

    return JSON.stringify(this, (_, value) => {
      if (typeof value === 'bigint') {
        return value.toString();
      }

      return value;
    });
  };
  segment.toString = function toString() {
    return JSON.stringify(this, (_, value) => {
      if (typeof value === 'bigint') {
        return value.toString();
      }

      return value;
    });
  };

  expect(segment).toEqual(
    expect.objectContaining({
      metadata: {
        default: {
          foo: {
            bar: 123n,
          },
        },
      },
    })
  );
});

As mentioned I'll open a PR for the first step and try to include it in the next release that should go out today/tomorrow at the latest. In the meanwhile, we'll give some time to the X-Ray SDK folks to review my issue and reply.

We'll keep monitoring the progress of that issue and hopefully contribute a PR; however if things take too long we'll reconsider implementing the change on our side either temporarily or permanently.