-
Notifications
You must be signed in to change notification settings - Fork 48
Log delivery to CloudWatch Logs in provider account #36
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
Merged
Merged
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
0033852
log delivery to cwl in provider account
jaymccon 9f553bd
filter provider generated logs from platform logger
jaymccon 47268c5
use try/except KeyError for log setup
jaymccon 3dca8fe
use super
jaymccon 17655e3
remove redundant typecast
jaymccon f001fed
cleaner use of patch context manager
jaymccon 1b1397e
less DRY, more readable
jaymccon c7c2e11
add comment about why only some boto client methods are mocked
jaymccon File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,108 @@ | ||
import logging | ||
import time | ||
from typing import Any, Mapping | ||
|
||
# boto3 doesn't have stub files | ||
import boto3 # type: ignore | ||
|
||
|
||
class ProviderFilter(logging.Filter): | ||
PROVIDER = "" | ||
|
||
def filter(self, record: logging.LogRecord) -> bool: | ||
return not record.name.startswith(self.PROVIDER) | ||
|
||
|
||
class ProviderLogHandler(logging.Handler): | ||
def __init__( | ||
self, | ||
group: str, | ||
stream: str, | ||
creds: Mapping[str, str], | ||
*args: Any, | ||
**kwargs: Any, | ||
): | ||
super(ProviderLogHandler, self).__init__(*args, **kwargs) | ||
self.group = group | ||
self.stream = stream.replace(":", "__") | ||
self.client = boto3.client("logs", **creds) | ||
self.sequence_token = "" | ||
|
||
@classmethod | ||
def setup(cls, event_data: Mapping[str, Any]) -> None: | ||
try: | ||
log_creds = event_data["requestData"]["providerCredentials"] | ||
except KeyError: | ||
log_creds = {} | ||
try: | ||
log_group = event_data["requestData"]["providerLogGroupName"] | ||
except KeyError: | ||
log_group = "" | ||
Comment on lines
+33
to
+40
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. if any of these |
||
try: | ||
stream_name = ( | ||
f'{event_data["stackId"]}/' | ||
f'{event_data["requestData"]["logicalResourceId"]}' | ||
) | ||
except KeyError: | ||
stream_name = f'{event_data["awsAccountId"]}-{event_data["region"]}' | ||
|
||
# filter provider messages from platform | ||
ProviderFilter.PROVIDER = event_data["resourceType"].replace("::", "_").lower() | ||
logging.getLogger().handlers[0].addFilter(ProviderFilter()) | ||
|
||
# add log handler to root, so that provider gets plugin logs too | ||
if log_creds and log_group: | ||
log_handler = cls( | ||
group=log_group, | ||
stream=stream_name, | ||
creds={ | ||
"aws_access_key_id": log_creds["accessKeyId"], | ||
"aws_secret_access_key": log_creds["secretAccessKey"], | ||
"aws_session_token": log_creds["sessionToken"], | ||
}, | ||
) | ||
logging.getLogger().addHandler(log_handler) | ||
|
||
def _create_log_group(self) -> None: | ||
try: | ||
self.client.create_log_group(logGroupName=self.group) | ||
except self.client.exceptions.ResourceAlreadyExistsException: | ||
pass | ||
|
||
def _create_log_stream(self) -> None: | ||
try: | ||
self.client.create_log_stream( | ||
logGroupName=self.group, logStreamName=self.stream | ||
) | ||
except self.client.exceptions.ResourceAlreadyExistsException: | ||
pass | ||
|
||
def _put_log_event(self, msg: logging.LogRecord) -> None: | ||
kwargs = { | ||
"logGroupName": self.group, | ||
"logStreamName": self.stream, | ||
"logEvents": [ | ||
{"timestamp": round(time.time() * 1000), "message": self.format(msg)} | ||
], | ||
} | ||
if self.sequence_token: | ||
kwargs["sequenceToken"] = self.sequence_token | ||
try: | ||
self.sequence_token = self.client.put_log_events(**kwargs)[ | ||
"nextSequenceToken" | ||
] | ||
except ( | ||
self.client.exceptions.DataAlreadyAcceptedException, | ||
self.client.exceptions.InvalidSequenceTokenException, | ||
) as e: | ||
self.sequence_token = str(e).split(" ")[-1] | ||
self._put_log_event(msg) | ||
|
||
def emit(self, record: logging.LogRecord) -> None: | ||
try: | ||
self._put_log_event(record) | ||
except self.client.exceptions.ResourceNotFoundException as e: | ||
if "log group does not exist" in str(e): | ||
self._create_log_group() | ||
self._create_log_stream() | ||
self._put_log_event(record) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,203 @@ | ||
# pylint: disable=redefined-outer-name,protected-access | ||
import logging | ||
from unittest.mock import DEFAULT, Mock, create_autospec, patch | ||
|
||
import pytest | ||
from aws_cloudformation_rpdk_python_lib.log_delivery import ( | ||
ProviderFilter, | ||
ProviderLogHandler, | ||
) | ||
|
||
|
||
@pytest.fixture | ||
def mock_logger(): | ||
return create_autospec(logging.getLogger()) | ||
|
||
|
||
@pytest.fixture | ||
def mock_provider_handler(): | ||
patch("aws_cloudformation_rpdk_python_lib.log_delivery.boto3.client", autospec=True) | ||
plh = ProviderLogHandler( | ||
group="test-group", | ||
stream="test-stream", | ||
creds={ | ||
"aws_access_key_id": "", | ||
"aws_secret_access_key": "", | ||
"aws_session_token": "", | ||
}, | ||
) | ||
# not mocking the whole client because that replaces generated exception classes to | ||
# be replaced with mocks | ||
for method in ["create_log_group", "create_log_stream", "put_log_events"]: | ||
setattr(plh.client, method, Mock(auto_spec=True)) | ||
tobywf marked this conversation as resolved.
Show resolved
Hide resolved
|
||
return plh | ||
|
||
|
||
@pytest.mark.parametrize( | ||
"logger", [("aa_bb_cc", False), ("aws_cloudformation_rpdk_python_lib", True)] | ||
) | ||
def test_provider_filter(logger): | ||
log_name, expected = logger | ||
ProviderFilter.PROVIDER = "aa_bb_cc" | ||
log_filter = ProviderFilter() | ||
record = logging.LogRecord( | ||
name=log_name, | ||
level=123, | ||
pathname="abc", | ||
lineno=123, | ||
msg="test", | ||
args=[], | ||
exc_info=False, | ||
) | ||
assert log_filter.filter(record) == expected | ||
|
||
|
||
def test_setup_with_provider_creds(mock_logger): | ||
payload = { | ||
"resourceType": "Foo::Bar::Baz", | ||
"stackId": "an-arn", | ||
"requestData": { | ||
"logicalResourceId": "MyResourceId", | ||
"providerCredentials": { | ||
"accessKeyId": "AKI", | ||
"secretAccessKey": "SAK", | ||
"sessionToken": "ST", | ||
}, | ||
"providerLogGroupName": "test_group", | ||
}, | ||
} | ||
patch_logger = patch( | ||
"aws_cloudformation_rpdk_python_lib.log_delivery.logging.getLogger", | ||
return_value=mock_logger, | ||
) | ||
patch_client = patch( | ||
"aws_cloudformation_rpdk_python_lib.log_delivery.boto3.client", autospec=True | ||
) | ||
|
||
with patch_logger as mock_log, patch_client as mock_client: | ||
ProviderLogHandler.setup(payload) | ||
mock_client.assert_called_once_with( | ||
"logs", | ||
aws_access_key_id="AKI", | ||
aws_secret_access_key="SAK", | ||
aws_session_token="ST", | ||
) | ||
mock_log.return_value.addHandler.assert_called_once() | ||
|
||
|
||
def test_setup_without_provider_creds(mock_logger): | ||
patch_logger = patch( | ||
"aws_cloudformation_rpdk_python_lib.log_delivery.logging.getLogger", | ||
return_value=mock_logger, | ||
) | ||
patch___init__ = patch( | ||
"aws_cloudformation_rpdk_python_lib.log_delivery.ProviderLogHandler" | ||
".__init__", | ||
autospec=True, | ||
) | ||
with patch_logger as mock_log, patch___init__ as mock___init__: | ||
payload = { | ||
"resourceType": "Foo::Bar::Baz", | ||
"region": "us-east-1", | ||
"awsAccountId": "123123123123", | ||
} | ||
ProviderLogHandler.setup(payload) | ||
payload["requestData"] = {} | ||
ProviderLogHandler.setup(payload) | ||
payload["requestData"] = {"providerLogGroupName": "test"} | ||
ProviderLogHandler.setup(payload) | ||
payload["requestData"] = { | ||
"providerCredentials": { | ||
"accessKeyId": "AKI", | ||
"secretAccessKey": "SAK", | ||
"sessionToken": "ST", | ||
} | ||
} | ||
ProviderLogHandler.setup(payload) | ||
mock___init__.assert_not_called() | ||
mock_log.return_value.addHandler.assert_not_called() | ||
|
||
|
||
def test_log_group_create_success(mock_provider_handler): | ||
mock_provider_handler._create_log_group() | ||
mock_provider_handler.client.create_log_group.assert_called_once() | ||
|
||
|
||
def test_log_stream_create_success(mock_provider_handler): | ||
mock_provider_handler._create_log_stream() | ||
mock_provider_handler.client.create_log_stream.assert_called_once() | ||
|
||
|
||
@pytest.mark.parametrize("create_method", ["_create_log_group", "_create_log_stream"]) | ||
def test__create_already_exists(mock_provider_handler, create_method): | ||
mock_logs_method = getattr(mock_provider_handler.client, create_method[1:]) | ||
exc = mock_provider_handler.client.exceptions.ResourceAlreadyExistsException | ||
mock_logs_method.side_effect = exc({}, operation_name="Test") | ||
# should not raise an exception if the log group already exists | ||
getattr(mock_provider_handler, create_method)() | ||
mock_logs_method.assert_called_once() | ||
|
||
|
||
@pytest.mark.parametrize("sequence_token", [None, "some-seq"]) | ||
def test__put_log_event_success(mock_provider_handler, sequence_token): | ||
mock_provider_handler.sequence_token = sequence_token | ||
mock_put = mock_provider_handler.client.put_log_events | ||
mock_put.return_value = {"nextSequenceToken": "some-other-seq"} | ||
mock_provider_handler._put_log_event( | ||
logging.LogRecord("a", 123, "/", 234, "log-msg", [], False) | ||
) | ||
mock_put.assert_called_once() | ||
|
||
|
||
def test__put_log_event_invalid_token(mock_provider_handler): | ||
exc = mock_provider_handler.client.exceptions | ||
mock_put = mock_provider_handler.client.put_log_events | ||
mock_put.return_value = {"nextSequenceToken": "some-other-seq"} | ||
mock_put.side_effect = [ | ||
exc.InvalidSequenceTokenException({}, operation_name="Test"), | ||
exc.DataAlreadyAcceptedException({}, operation_name="Test"), | ||
DEFAULT, | ||
] | ||
mock_provider_handler._put_log_event( | ||
logging.LogRecord("a", 123, "/", 234, "log-msg", [], False) | ||
) | ||
assert mock_put.call_count == 3 | ||
|
||
|
||
def test_emit_existing_cwl_group_stream(mock_provider_handler): | ||
mock_provider_handler._put_log_event = Mock() | ||
mock_provider_handler.emit( | ||
logging.LogRecord("a", 123, "/", 234, "log-msg", [], False) | ||
) | ||
mock_provider_handler._put_log_event.assert_called_once() | ||
|
||
|
||
def test_emit_no_group_stream(mock_provider_handler): | ||
exc = mock_provider_handler.client.exceptions.ResourceNotFoundException | ||
group_exc = exc( | ||
{"Error": {"Message": "log group does not exist"}}, | ||
operation_name="PutLogRecords", | ||
) | ||
mock_provider_handler._put_log_event = Mock() | ||
mock_provider_handler._put_log_event.side_effect = [group_exc, DEFAULT] | ||
mock_provider_handler._create_log_group = Mock() | ||
mock_provider_handler._create_log_stream = Mock() | ||
mock_provider_handler.emit( | ||
logging.LogRecord("a", 123, "/", 234, "log-msg", [], False) | ||
) | ||
assert mock_provider_handler._put_log_event.call_count == 2 | ||
mock_provider_handler._create_log_group.assert_called_once() | ||
mock_provider_handler._create_log_stream.assert_called_once() | ||
|
||
# create_group should not be called again if the group already exists | ||
stream_exc = exc( | ||
{"Error": {"Message": "log stream does not exist"}}, | ||
operation_name="PutLogRecords", | ||
) | ||
mock_provider_handler._put_log_event.side_effect = [stream_exc, DEFAULT] | ||
mock_provider_handler.emit( | ||
logging.LogRecord("a", 123, "/", 234, "log-msg", [], False) | ||
) | ||
assert mock_provider_handler._put_log_event.call_count == 4 | ||
mock_provider_handler._create_log_group.assert_called_once() | ||
assert mock_provider_handler._create_log_stream.call_count == 2 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
dependency injection of the client would make testing easier i think
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.
the
mock_provider_handler
fixture is 14 lines long, which covers patching the boto client for all tests, unless you feel strongly on this, I'm inclined to leave well enough alone.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.
i only really considered it because it would mean you wouldn't have to pass credentials around. the client could be instantiated in
setup
directly