Skip to content

Conversation

@Czechh
Copy link
Contributor

@Czechh Czechh commented Nov 16, 2020

What does this PR do?

Handles AWS Lambdas triggered from SQS events, and attempts to extract context from it.

Motivation

Improving distributed tracing experience.

Testing Guidelines

  • Unit tests.

Additional Notes

Types of Changes

  • Bug fix
  • New feature
  • Breaking change
  • Misc (docs, refactoring, dependency upgrade, etc.)

Check all that apply

  • This PR's description is comprehensive
  • This PR contains breaking changes that are documented in the description
  • This PR introduces new APIs or parameters that are documented and unlikely to change in the foreseeable future
  • This PR impacts documentation, and it has been updated (or a ticket has been logged)
  • This PR's changes are covered by the automated tests
  • This PR collects user input/sensitive content into Datadog
  • This PR passes the integration tests (ask a Datadog member to run the tests)

@Czechh Czechh requested a review from a team as a code owner November 16, 2020 17:10
Copy link
Contributor

@DarcyRaynerDD DarcyRaynerDD left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left some comments and nits

{"e":XXXX,"m":"serverless.integration_test.execution","t":["function:http-request","dd_lambda_layer:datadog-nodev12.XX.X"],"v":1}
XXXX-XX-XX XX:XX:XX.XXX XXXX-XXXX-XXXX-XXXX-XXXX INFO [dd.trace_id=XXXX dd.span_id=XXXX] Snapshot test http requests successfully made to URLs: https://ip-ranges.datadoghq.com,https://ip-ranges.datadoghq.eu
{"traces":[[{"trace_id":"XXXX","span_id":"XXXX","parent_id":"XXXX","name":"aws.lambda","resource":"integration-plugin-dev-http-requests_node12_with_plugin","error":0,"meta":{"_dd.origin":"lambda","service":"integration-plugin-dev-http-requests_node12_with_plugin","cold_start":"false","function_arn":"XXXX_node12_with_plugin","function_version":"$LATEST","request_id":"XXXX","resource_names":"integration-plugin-dev-http-requests_node12_with_plugin","datadog_lambda":"XXXX","dd_trace":"XXXX","_dd.parent_source":"xray"},"metrics":{"_sample_rate":1,"_sampling_priority_v1":2},"start":XXXX,"duration":XXXX,"service":"aws.lambda","type":"serverless"}]]}
{"traces":[[{"trace_id":"XXXX","span_id":"XXXX","parent_id":"XXXX","name":"http.request","resource":"GET","error":0,"meta":{"_dd.origin":"lambda","service":"integration-plugin-dev-http-requests_node12_with_plugin","span.kind":"client","http.method":"GET","http.url":"https://ip-ranges.datadoghq.com/","http.status_code":"200"},"metrics":{"_sample_rate":1,"_sampling_priority_v1":2},"start":XXXX,"duration":XXXX,"service":"integration-plugin-dev-http-requests_node12_with_plugin-http-client","type":"http"},{"trace_id":"XXXX","span_id":"XXXX","parent_id":"XXXX","name":"http.request","resource":"GET","error":0,"meta":{"_dd.origin":"lambda","service":"integration-plugin-dev-http-requests_node12_with_plugin","span.kind":"client","http.method":"GET","http.url":"https://ip-ranges.datadoghq.eu/","http.status_code":"200"},"metrics":{"_sample_rate":1,"_sampling_priority_v1":2},"start":XXXX,"duration":XXXX,"service":"integration-plugin-dev-http-requests_node12_with_plugin-http-client","type":"http"},{"trace_id":"XXXX","span_id":"XXXX","parent_id":"XXXX","name":"aws.lambda","resource":"integration-plugin-dev-http-requests_node12_with_plugin","error":0,"meta":{"_dd.origin":"lambda","service":"integration-plugin-dev-http-requests_node12_with_plugin","cold_start":"false","function_arn":"XXXX_node12_with_plugin","function_version":"$LATEST","request_id":"XXXX","resource_names":"integration-plugin-dev-http-requests_node12_with_plugin","datadog_lambda":"XXXX","dd_trace":"XXXX","_dd.parent_source":"xray"},"metrics":{"_sample_rate":1,"_sampling_priority_v1":2},"start":XXXX,"duration":XXXX,"service":"aws.lambda","type":"serverless"}]]}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These traces changed kind of dramatically? I'm guessing this is more from flakiness and not this PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think some of these changes reflect the request to check for the extension.

Czechh and others added 3 commits November 16, 2020 18:15
Co-authored-by: Darcy Rayner <50632605+DarcyRaynerDD@users.noreply.github.com>
@codecov-io
Copy link

codecov-io commented Nov 17, 2020

Codecov Report

Merging #128 (0a9c6a0) into master (e170dc5) will decrease coverage by 0.52%.
The diff coverage is 75.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #128      +/-   ##
==========================================
- Coverage   88.21%   87.68%   -0.53%     
==========================================
  Files          29       29              
  Lines         933      958      +25     
  Branches      162      169       +7     
==========================================
+ Hits          823      840      +17     
- Misses         76       80       +4     
- Partials       34       38       +4     
Impacted Files Coverage Δ
src/trace/context.ts 90.81% <75.00%> (-3.34%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e170dc5...0a9c6a0. Read the comment docs.

@Czechh Czechh merged commit f2558a5 into master Nov 17, 2020
@Czechh Czechh deleted the sergio.prada/extract-trace-context-from-sqs-event branch November 17, 2020 15:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants