-
Notifications
You must be signed in to change notification settings - Fork 45
Use single socket instance for all xray api calls. #467
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
Conversation
datadog_lambda/xray.py
Outdated
except Exception as e_send: | ||
logger.error("Error occurred submitting to xray daemon: %s", e_send) | ||
|
||
def reset(self): |
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.
reset
is used for tests.
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.
nitpick: can we move it to tests files?
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.
Yes, good call. Done.
self._host_port_tuple = self._get_xray_host_port( | ||
os.environ.get(XrayDaemon.XRAY_DAEMON_ADDRESS, "") | ||
) | ||
return self._host_port_tuple |
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.
This value is now memoized so we only need to parse the env var once.
@@ -623,7 +626,7 @@ def handler(event, context): | |||
self.assertEqual(result.span_id, aws_lambda_span.span_id) | |||
self.assertEqual(result.sampling_priority, 1) | |||
mock_send.assert_called_once() | |||
(_, raw_payload), _ = mock_send.call_args | |||
(raw_payload,), _ = mock_send.call_args |
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.
Signature of the xray.send
method changed.
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.
LGTM
datadog_lambda/xray.py
Outdated
except Exception as e_send: | ||
logger.error("Error occurred submitting to xray daemon: %s", e_send) | ||
|
||
def reset(self): |
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.
nitpick: can we move it to tests files?
What does this PR do?
Motivation
The need for speed.
Testing Guidelines
Additional Notes
Previously, we were creating a new socket connection and closing the connection for each xray api call. When xray is enabled, and the lambda has extracted trace context from an upstream event, the xray api is called twice.
When an xray api is called, this change decreases runtime duration by somewhere between 1-5%.
Types of Changes
Check all that apply