Extract span tags from triggering event #140
Conversation
Codecov Report
@@ Coverage Diff @@
## main #140 +/- ##
==========================================
- Coverage 87.33% 86.05% -1.29%
==========================================
Files 29 31 +2
Lines 1011 1169 +158
Branches 179 232 +53
==========================================
+ Hits 883 1006 +123
- Misses 89 110 +21
- Partials 39 53 +14
Continue to review full report at Codecov.
|
|
@DarcyRaynerDD Can you help review as well and give @DylanLovesCoffee some feedback, as I'm not super sure about a few key changes. Hope you can share some thoughts. |
| @@ -16,6 +16,7 @@ export const samplingPriorityHeader = "x-datadog-sampling-priority"; | |||
| export const xraySubsegmentName = "datadog-metadata"; | |||
| export const xraySubsegmentKey = "trace"; | |||
| export const xrayBaggageSubsegmentKey = "root_span_metadata"; | |||
| export const xrayLambdaFunctionTagsKey = "lambda_function_tags"; | |||
There was a problem hiding this comment.
I'm still a little confused by this change. What was the reason for xrayBaggageSubsegmentKey not being sufficient?
There was a problem hiding this comment.
This is mainly just to differentiate the behavior in the X-Ray converter. Only the first root_span_metadata subsegment that's found is added to the root span, while all lambda_function_tags subsegments found are added to their respective parent Lambda execution span(s). On top of that, if X-Ray for API Gateway is enabled, the API Gateway spans are considered the root span.
src/trace/trigger.ts
Outdated
| if (result === undefined) { | ||
| statusCode = "502"; | ||
| } else if (result.statusCode) { | ||
| // Use the statusCode if available |
There was a problem hiding this comment.
Using any for the result type is fine for this method, but we should be stricter with the type checking, especially since this method is called during a finally block and should never throw.
const statusCode = result?.statusCode; // Will be undefined if statusCode doesn't exist or result isn't an object
if (typeof statusCode === 'number') {
return statusCode.toString();
} else if (typeof statusCode ==="string") {
return statusCode;
}There was a problem hiding this comment.
TIL about optional chaining! I kept the check for result === undefined here since we would still need to return a 502 status when the Lambda doesn't receive a response.
|
|
||
| // e.g. arn:aws:s3:::lambda-xyz123-abc890 | ||
| if (source === "s3") { | ||
| eventSourceARN = extractS3EventARN(event); |
There was a problem hiding this comment.
The event structure may still be malformed, even after we apply our typeguard. We should add a try catch around this method
There was a problem hiding this comment.
I was curious how you knew that the S3 event structure might be malformed versus others? I wasn't sure if I should try wrapping the other similar methods with a try...catch
There was a problem hiding this comment.
I should have clarified, they could all be malformed, I meant a generic try catch around the entire method.
What does this PR do?
Parses the event invoking the function for tags to be added to the span. Currently only adds the following services to be detected: API Gateway, ALB, SQS, SNS, DynamoDB, Kinesis, Cloudfront, CW Logs, CW Events. API Gateway and ALB trigger events will also add the HTTP url, method, path, referer and status code response if available.
For the case of X-Ray <> Datadog hybrid tracing, we create an additional dummy Datadog subsegment for the trigger tags.
Motivation
SLS-790 - Display function trigger span tags
Testing Guidelines
Unit tests
Additional Notes
Types of Changes
Check all that apply