-
Notifications
You must be signed in to change notification settings - Fork 45
Feat: Add ability to tag Lambda request and response payloads for 2.7… #180
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
Changes from all commits
cf87545
8472d84
adcdfaf
cf6b075
132a3c1
4a2cab7
94a91e1
199886c
8f55826
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,4 @@ | ||
{ | ||
"python.pythonPath": "/usr/local/bin/python3" | ||
} | ||
"python.pythonPath": "/usr/local/bin/python3", | ||
"python.formatting.provider": "black" | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,59 @@ | ||
# Unless explicitly stated otherwise all files in this repository are licensed | ||
# under the Apache License Version 2.0. | ||
# This product includes software developed at Datadog (https://www.datadoghq.com/). | ||
# Copyright 2021 Datadog, Inc. | ||
|
||
import json | ||
import logging | ||
|
||
redactable_keys = ["authorization", "x-authorization", "password", "token"] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How did we decide on these four keys for redaction? Should we provide a way to redact other keys? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We provide |
||
max_depth = 10 | ||
logger = logging.getLogger(__name__) | ||
|
||
|
||
def tag_object(span, key, obj, depth=0): | ||
if depth >= max_depth: | ||
return | ||
else: | ||
depth += 1 | ||
if obj is None: | ||
return span.set_tag(key, obj) | ||
if _should_try_string(obj): | ||
parsed = None | ||
try: | ||
parsed = json.loads(obj) | ||
return tag_object(span, key, parsed, depth) | ||
except ValueError: | ||
redacted = _redact_val(key, obj[0:5000]) | ||
return span.set_tag(key, redacted) | ||
if isinstance(obj, int) or isinstance(obj, float): | ||
return span.set_tag(key, obj) | ||
if isinstance(obj, list): | ||
for k, v in enumerate(obj): | ||
formatted_key = "{}.{}".format(key, k) | ||
tag_object(span, formatted_key, v, depth) | ||
return | ||
if isinstance(obj, object): | ||
for k in obj: | ||
v = obj.get(k) | ||
formatted_key = "{}.{}".format(key, k) | ||
tag_object(span, formatted_key, v, depth) | ||
return | ||
|
||
|
||
def _should_try_string(obj): | ||
try: | ||
if isinstance(obj, str) or isinstance(obj, unicode): | ||
return True | ||
except NameError: | ||
if isinstance(obj, bytes): | ||
return True | ||
|
||
return False | ||
|
||
|
||
def _redact_val(k, v): | ||
split_key = k.split(".").pop() or k | ||
if split_key in redactable_keys: | ||
return "redacted" | ||
return v | ||
astuyve marked this conversation as resolved.
Show resolved
Hide resolved
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -28,9 +28,13 @@ | |
create_function_execution_span, | ||
) | ||
from datadog_lambda.trigger import extract_trigger_tags, extract_http_status_code_tag | ||
from datadog_lambda.tag_object import tag_object | ||
|
||
logger = logging.getLogger(__name__) | ||
|
||
dd_capture_lambda_payload_enabled = ( | ||
os.environ.get("DD_CAPTURE_LAMBDA_PAYLOAD", "false").lower() == "true" | ||
) | ||
|
||
""" | ||
Usage: | ||
|
@@ -186,6 +190,10 @@ def _after(self, event, context): | |
flush_extension() | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we enable this feature in the integration tests as well for improved test coverage? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, great idea. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It appears that we don't care about span tags in our integration tests, because the integration tests are unaffected by this ENV variable (off or on). We can do this during an engineering sprint, perhaps? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm, that's weird, the integration tests are supposed to capture span tags. Let's dig into this a bit more before merging to make sure nothing is wrong. Happy to chat on Zoom about this if you want. |
||
if self.span: | ||
if dd_capture_lambda_payload_enabled: | ||
tag_object(self.span, "function.request", event) | ||
tag_object(self.span, "function.response", self.response) | ||
|
||
if status_code: | ||
self.span.set_tag("http.status_code", status_code) | ||
self.span.finish() | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for cleaning this up! 🙏