search

aws / aws-xray-sdk-node

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

Support AWS Serverless Express #45

Open haotianw465 opened 6 years ago

haotianw465 commented 6 years ago

The SDK is having issues when an X-Ray instrumented express web application is running in AWS Lambda by utilizing https://github.com/awslabs/aws-serverless-express, reported in a comment at https://github.com/aws/aws-xray-sdk-node/issues/30.

The SDK vended express middleware needs to work better with Lambda generated segments to provide better customer experiences.

duro commented 4 years ago

@haotianw465 and @chrisradek has any progress on this been made. This issue is almost 1.5 years old. I'm running into this issue, and cannot for the life of me figure out how to get tracing enabled in my functions that are running aws-serverless-express.

willarmiros commented 4 years ago

Hi @duro ,

This feature is still part of our backlog. Since it likely involves changing some things under the hood with how X-Ray is integrated with Lambda it is fairly complicated.

To be clear about your exact use case, what exactly are you looking for to "get tracing enabled in functions running aws-serverless-express"? What do you currently see in your trace and what would you like to see? Would you like to directly manipulate the segment in your express route logic, e.g. with AWSXRay.getSegment().setUser('john123')?

avin-kavish commented 4 years ago

Shouldn't we be able to see the lambda segment and express segment as one dot on the map?

willarmiros commented 4 years ago

@avin-kavish that is correct. For some context, we generate a facade segment in Lambda environments that does not represent the actual segment sent to the X-Ray console. This facade segment just performs no-ops for all operations done on it except for adding subsegments. You can see the implementation of the facade segment in Lambda environments here.

The facade segment will clobber any segments attempted to be generated by Middleware patchers such as our Express library. So there should always only be the AWS::Lambda::Function segment, represented as a node on the service graph, for each Lambda function, even if it uses the express middleware.

The ask here, at least to my understanding, is to have such segments represented as an express segment INSTEAD of a Lambda segment.

avin-kavish commented 4 years ago

Hmm.. I think two seperate segments are good because they are essentially two separate systems with their own complications and two distinct points of failure. I.e. a request could fail due to a lambda issue or an express issue.

I don't think the express segment can be used INSTEAD of the lambda because the metrics gathered from the two are different. For example: A disparity between the timing of the lambda and the express segment could reveal that whichever library you are using as an adapter between lambda and express is introducing latency into the system.

If they are to be represented as one, then the metrics relating to them have to be merged and presented appropriately, not overwritten.

One improvement would be to have the express segment embedded inside the lambda segment in the service map. Everything else seems fine.

willarmiros commented 4 years ago

Ok I see, and I completely agree on your point of having two separate segments. Thank you for explaining the experience you'd prefer as a user, it will be valuable when prioritizing and ultimately designing this feature. Since this involves a much larger story of modifying our integration with AWS Lambda, I cannot provide any ETAs or other plans but these comments will be taken into account.