Skip to content

fix: type check event before treating it as a dict #233

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Jun 27, 2022
Merged

fix: type check event before treating it as a dict #233

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3006cdd
fix: type check event before treating it as a dict
astuyve Jun 27, 2022
74b106a
fix: Use doublequotes for binary string
astuyve Jun 27, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion datadog_lambda/tracing.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,7 @@ def extract_dd_trace_context(event, lambda_context, extractor=None):
parent_id,
sampling_priority,
) = extract_context_custom_extractor(extractor, event, lambda_context)
elif "headers" in event:
elif isinstance(event, (list, dict)) and "headers" in event:
Copy link
Contributor

Choose a reason for hiding this comment

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

@astuyve I'm wondering we should only check dict. Is it possible for list to contain Datadog trace context?

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 was trying to consider which data structures implement an iterable, but realize that list probably won't help us. I think I'm going to replace list with set, since another library might convert the event into a set instead of a dict.

Copy link
Contributor

@kimi-p kimi-p Jun 27, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

SG! Just for me own knowledge, do we have a doc that lists what lib might form event in what format?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It varies wildly depending on the integration (see docs). But this library seems to have been written basically only for a Dict or Set type event - however Lambda allows the type of event to be None, binary data, or a few other options (including a list/array), and since we don't have an guarantee, we have to duck-type it.

(
trace_id,
parent_id,
Expand Down
22 changes: 22 additions & 0 deletions tests/test_tracing.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,28 @@ def test_without_datadog_trace_headers(self):
{},
)

def test_with_non_object_event(self):
lambda_ctx = get_mock_context()
ctx, source = extract_dd_trace_context(b"", lambda_ctx)
self.assertEqual(source, "xray")
self.assertDictEqual(
ctx,
{
"trace-id": fake_xray_header_value_root_decimal,
"parent-id": fake_xray_header_value_parent_decimal,
"sampling-priority": "2",
},
)
self.assertDictEqual(
get_dd_trace_context(),
{
TraceHeader.TRACE_ID: fake_xray_header_value_root_decimal,
TraceHeader.PARENT_ID: fake_xray_header_value_parent_decimal,
TraceHeader.SAMPLING_PRIORITY: "2",
},
{},
)

def test_with_incomplete_datadog_trace_headers(self):
lambda_ctx = get_mock_context()
ctx, source = extract_dd_trace_context(
Expand Down