Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cover string/b64 msg attribute trace extraction #211

Merged
merged 30 commits into from
Mar 25, 2022

Conversation

zARODz11z
Copy link
Contributor

@zARODz11z zARODz11z commented Mar 9, 2022

What does this PR do?

Ports Datadog/datadog-lambda-js#269 to Python Lambda Library and Tracer.

Motivation

FixesDatadog/serverless-plugin-datadog#232 for Python lambda library and tracer. Message attributes sent as json strings break filter policies and make it so messages don't make it to subscribers with a filter policy.

Testing Guidelines

Linking my local versions of the tracer and lambda library to my sample app I had a lambda function that published to an SNS topic that has an SQS subscription with a filter policy. After b64 encoding only SQS messageAttribute trace data in ddtrace-py the trace context is successfully passed through subscribers with a filter policy. I left SNS messageAttributes as strings. Check out my DD TRACE PY PR.

Publisher Lambda->SNS->SQS-> Receiver Lambda (dd link)

lambdaSnsSqsLambda

I also manually tested

Publisher Lambda->SNS-> Receiver Lambda dd link

lambdaSnsLambda

Publisher Lambda->SQS-> Receiver Lambda dd link

Screen Shot 2022-03-11 at 4 07 03 PM

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)

@zARODz11z
Copy link
Contributor Author

zARODz11z commented Mar 9, 2022

TODO: Update ddtrace version in pyproject.toml and poetry lock file once DataDog/dd-trace-py#3404 is merged and released.

@zARODz11z zARODz11z marked this pull request as ready for review March 11, 2022 21:28
@zARODz11z zARODz11z requested a review from a team as a code owner March 11, 2022 21:28
…eption if message attributes are not string or binary
…aDog/datadog-lambda-python into ar/sns-trace-context-from-b64-binary
@zARODz11z
Copy link
Contributor Author

As a sanity check I manually tested these scenarios again:

API GW -> Publisher Lambda->SNS->SQS-> Receiver Lambda dd link
Screen Shot 2022-03-16 at 10 43 12 AM

API GW -> Publisher Lambda->SNS-> Receiver Lambda dd link
Screen Shot 2022-03-16 at 11 17 05 AM

API GW -> Publisher Lambda->SQS-> Receiver Lambda dd link
Screen Shot 2022-03-16 at 11 03 32 AM

zARODz11z and others added 3 commits March 16, 2022 15:26
@zARODz11z
Copy link
Contributor Author

waiting on DataDog/dd-trace-py#3404

mergify bot pushed a commit to DataDog/dd-trace-py that referenced this pull request Mar 22, 2022
…#3404)

Making sure msg attributes are encoded by the tracer (by changing the Type) allows our lambda library to decode the message attribute and create inferred spans for sns and sqs.
Checkout this dd-lambda-python [PR](DataDog/datadog-lambda-python#211) for more context.
## Checklist
- [ ] Added to the correct milestone.
- [ ] Tests provided or description of manual testing performed is included in the code or PR.
- [ ] Library documentation is updated.
- [ ] [Corp site](https://github.com/DataDog/documentation/) documentation is updated (link to the PR).
mergify bot pushed a commit to DataDog/dd-trace-py that referenced this pull request Mar 22, 2022
…#3404)

Making sure msg attributes are encoded by the tracer (by changing the Type) allows our lambda library to decode the message attribute and create inferred spans for sns and sqs.
Checkout this dd-lambda-python [PR](DataDog/datadog-lambda-python#211) for more context.
## Checklist
- [ ] Added to the correct milestone.
- [ ] Tests provided or description of manual testing performed is included in the code or PR.
- [ ] Library documentation is updated.
- [ ] [Corp site](https://github.com/DataDog/documentation/) documentation is updated (link to the PR).

(cherry picked from commit bd8e176)
mergify bot pushed a commit to DataDog/dd-trace-py that referenced this pull request Mar 22, 2022
…#3404)

Making sure msg attributes are encoded by the tracer (by changing the Type) allows our lambda library to decode the message attribute and create inferred spans for sns and sqs.
Checkout this dd-lambda-python [PR](DataDog/datadog-lambda-python#211) for more context.
## Checklist
- [ ] Added to the correct milestone.
- [ ] Tests provided or description of manual testing performed is included in the code or PR.
- [ ] Library documentation is updated.
- [ ] [Corp site](https://github.com/DataDog/documentation/) documentation is updated (link to the PR).

(cherry picked from commit bd8e176)
mergify bot pushed a commit to DataDog/dd-trace-py that referenced this pull request Mar 22, 2022
…#3404)

Making sure msg attributes are encoded by the tracer (by changing the Type) allows our lambda library to decode the message attribute and create inferred spans for sns and sqs.
Checkout this dd-lambda-python [PR](DataDog/datadog-lambda-python#211) for more context.
## Checklist
- [ ] Added to the correct milestone.
- [ ] Tests provided or description of manual testing performed is included in the code or PR.
- [ ] Library documentation is updated.
- [ ] [Corp site](https://github.com/DataDog/documentation/) documentation is updated (link to the PR).

(cherry picked from commit bd8e176)
mergify bot pushed a commit to DataDog/dd-trace-py that referenced this pull request Mar 22, 2022
…#3404)

Making sure msg attributes are encoded by the tracer (by changing the Type) allows our lambda library to decode the message attribute and create inferred spans for sns and sqs.
Checkout this dd-lambda-python [PR](DataDog/datadog-lambda-python#211) for more context.
## Checklist
- [ ] Added to the correct milestone.
- [ ] Tests provided or description of manual testing performed is included in the code or PR.
- [ ] Library documentation is updated.
- [ ] [Corp site](https://github.com/DataDog/documentation/) documentation is updated (link to the PR).

(cherry picked from commit bd8e176)
mergify bot pushed a commit to DataDog/dd-trace-py that referenced this pull request Mar 22, 2022
…#3404)

Making sure msg attributes are encoded by the tracer (by changing the Type) allows our lambda library to decode the message attribute and create inferred spans for sns and sqs.
Checkout this dd-lambda-python [PR](DataDog/datadog-lambda-python#211) for more context.
## Checklist
- [ ] Added to the correct milestone.
- [ ] Tests provided or description of manual testing performed is included in the code or PR.
- [ ] Library documentation is updated.
- [ ] [Corp site](https://github.com/DataDog/documentation/) documentation is updated (link to the PR).

(cherry picked from commit bd8e176)
poetry.lock Show resolved Hide resolved
@zARODz11z
Copy link
Contributor Author

@zARODz11z zARODz11z requested a review from astuyve March 24, 2022 21:18
dd_payload.get("Value", r"{}"),
)
else:
raise Exception(
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this error caught anywhere? Why raise here when we previously didn't?

If for some reason the _datadog attribute isn't string or binary, we don't want to throw an error and halt the customer function execution.

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 now see why we wouldn't want to stop the execution if its not a string or binary. I opted for making this exception a logger.warn as I think it would still be useful for the customer to know that their message attribute type is not supported by our library.

Copy link
Contributor

Choose a reason for hiding this comment

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

Nice, I think that's a good call

@zARODz11z zARODz11z requested a review from astuyve March 25, 2022 14:46
@zARODz11z zARODz11z merged commit 0e6e9dc into main Mar 25, 2022
@zARODz11z zARODz11z deleted the ar/sns-trace-context-from-b64-binary branch March 25, 2022 17:10
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.

2 participants