dd-trace-py icon indicating copy to clipboard operation
dd-trace-py copied to clipboard

feat(aiobotocore): add context propagation in Lambda calls

Open Neki opened this issue 3 years ago • 5 comments

Description

Unlike the botocore integration, the aiobotocore integration does not propagate the tracing context when a Lambda function is called. As I need this feature, I'm opening this PR, hoping that I can get this upstreamed :slightly_smiling_face:

The botocore integration also supports context propagation in SNS and SQS, but since I don't have a need for them yet, I didn't include them in this PR.

I implemented this by adapting the code in the botocore integration (https://github.com/DataDog/dd-trace-py/blob/master/ddtrace/contrib/botocore/patch.py) and the corresponding tests (https://github.com/DataDog/dd-trace-py/blob/master/tests/contrib/botocore/test.py). I kept the "musical" function names too :smile:

There is one noteworthy difference: for some reason, during the aiobotocore tests, the lambda API returns HTTP 202 (instead of HTTP 200) in API calls. That's the expected return code for an 'Event' invocation (rather than 'RequestResponse'). I changed the tests accordingly; if there is a chance that this PR might be merged and you give me some pointers as to where to look, I'm OK with spending some more time investigating this.

There is some duplicated code between the botocore and aiobotocore integrations. I don't know if you'd rather keep the implementations for these two libraries completely decoupled, or if you'd rather extract the helper methods in a common module. Here again, I'm OK with spending some more time on this if it means that this PR will be merged :slightly_smiling_face:

Fixes https://github.com/DataDog/dd-trace-py/issues/2039.

Checklist

  • [ ] Added to the correct milestone.
  • [x] Tests provided or description of manual testing performed is included in the code or PR.
  • [x] Library documentation is updated.
  • [ ] Corp site documentation is updated (link to the PR).

Neki avatar Apr 16 '21 08:04 Neki

@Neki this pull request is now in conflict 😩

mergify[bot] avatar Apr 27 '21 03:04 mergify[bot]

Hi, I'm not sure if something is expected of me here :) fixing the merge conflict seems doable but is not direct, should I rebase this branch on top of the latest master?

Neki avatar May 03 '21 08:05 Neki

Is there a plan to review this PR in the near future?

Padwicker avatar Sep 22 '21 16:09 Padwicker

@Kyle-Verhoog will this change be released soon?

theQuazz avatar Nov 18 '21 04:11 theQuazz

@Neki this pull request is now in conflict 😩

mergify[bot] avatar Sep 26 '22 07:09 mergify[bot]

Given the low activity and conflicts I am going to close this PR. Please feel free to resolve the conflicts and re-open!

brettlangdon avatar Mar 28 '23 23:03 brettlangdon