APMSVLS-65 Extract Trace Context For AppSync Events #657
Conversation
…extract_dd_trace_context
…t['request']['headers']
| span_id=67890, | ||
| sampling_priority=1, | ||
| ), | ||
| ), |
There was a problem hiding this comment.
There's a lot more testing that we're going to need. When thinking about test coverage, I think about two things.
-
Coverage. If I were to run a coverage report on our tests, would I see that we've hit every line of code? In the current case, the answer is no.
-
Logic. If I were to make a change to any line of your code, would a test fail? For example, if I change a
intonot in, I would expect to see a failing test. In the current case, this could mean testing thatrequestis a dict butheadersis not in it and vice versa.
We also would need a test that makes sure that the authorizer context is never decoded.
| decode_authorizer_context=False, | ||
| ) | ||
| else: | ||
| context = extract_context_from_lambda_context(lambda_context) |
There was a problem hiding this comment.
Looks like we still need a test to cover this portion of the logic. We know that the function does not error when the request is not a dict or that there are no headers in it. But we have not confirmed that we'll instead attempt to extract from the lambda context.
|
/merge |
|
View all feedbacks in Devflow UI.
The expected merge time in
The merge request has been interrupted because the build 78722712 took longer than expected. The current limit for the base branch 'main' is 120 minutes. |
|
/merge |
|
View all feedbacks in Devflow UI.
The expected merge time in
|
dd-devflow
bot
added
mergequeue-status: queued
mergequeue-status: in_progress
and removed
mergequeue-status: rejected
mergequeue-status: queued
labels
dd-mergequeue
bot
deleted the
rithika.narayan/APMSVLS-65/extract-context-from-appsync
branch
3 participants
What does this PR do?
This PR adds logic to extract the trace context of an event that comes from a traced service through AppSync to a Python lambda. When a lambda is invoked from an AppSync API which was called in a RUM-instrumented front-end, the datadog trace context is located under event["request"]["headers"] rather than other locations from different event types. The extraction logic is placed directly in the
extract_dd_trace_contextfunction in tracing.py, indexing into the request key in the event, after which theextract_context_from_http_event_or_contextfunction will get the header field and extract datadog context properly.Motivation
Currently, if a customer has a setup where they use RUM to start a trace that goes through AppSync and triggers a Lambda function, we ask them to write a custom function to extract the trace context from the AppSync event that is passed to the Lambda. See an example of such a function here. It would be simpler for customers and for ourselves to extract the trace context in the lambda tracer layers ourselves, as we do for invocations from other sources (ex. SQS, API Gateway, etc.). This PR does so for the Python tracer.
Based on this ticket, will reduce amount of code changes a customer has to make in order connect traces between RUM and a Lambda function when there is an AWS AppSync API in between them.
Testing Guidelines
Unit tested in
test_tracing.pyby adding events in tests/event_samples and including the tests in _test_extract_dd_trace_context. The events added,rum-appsync.json,rum-appsync-no-headers.json, andrum-appsync-request-not-dict.json, are based on a sample request from the Datadog APM page of a trace that followed RUM -> AppSync -> Lambda. Some of the sample events are malformed/formatted differently than expected to ensure that exceptions are not raised if we encounter an event with a different format than anticipated. I ran a coverage test using pytest-cov to ensure that all new lines of code from this PR were tested.Ran integration tests using scripts/run_integration_tests.sh. Added a new input event called
appsync.jsonand updated the snapshot so that integration tests also cover this new supported case.Uploaded my changes as a layer to AWS and tested whether a trace that followed RUM -> AppSync -> Lambda was shown as connected in the Datadog UI without needing a custom extractor, which is the goal of this PR. A successfully connected trace can be found here. The ARN for the testing version of the Python Lambda layer is
arn:aws:lambda:us-east-1:425362996713:layer:Python39-RITHIKA:3Also used this layer to check distributed tracing in an API Gateway -> Lambda -> SQS -> Lambda setup to ensure that other tracing functionality was not broken by change.Additional Notes
Types of Changes
Check all that apply