From 66bc5c4fca9a217ca80a1ae6ccca0f12421fe583 Mon Sep 17 00:00:00 2001 From: thoward Date: Thu, 21 Jul 2022 15:20:55 -0700 Subject: [PATCH 1/5] add log_format argument --- src/cloudformation_cli_python_lib/hook.py | 8 ++++++-- .../log_delivery.py | 18 ++++++++++++++++-- src/cloudformation_cli_python_lib/resource.py | 4 +++- src/setup.py | 2 +- 4 files changed, 26 insertions(+), 6 deletions(-) diff --git a/src/cloudformation_cli_python_lib/hook.py b/src/cloudformation_cli_python_lib/hook.py index f92f445..5504d6a 100644 --- a/src/cloudformation_cli_python_lib/hook.py +++ b/src/cloudformation_cli_python_lib/hook.py @@ -65,13 +65,17 @@ def wrapper(self: Any, event: MutableMapping[str, Any], context: Any) -> Any: class Hook: def __init__( - self, type_name: str, type_configuration_model_cls: Type[BaseModel] + self, + type_name: str, + type_configuration_model_cls: Type[BaseModel], + log_format: Optional[logging.Formatter] = None, ) -> None: self.type_name = type_name self._type_configuration_model_cls: Type[ BaseModel ] = type_configuration_model_cls self._handlers: MutableMapping[HookInvocationPoint, HandlerSignature] = {} + self.log_format = log_format def handler( self, invocation_point: HookInvocationPoint @@ -247,7 +251,7 @@ def print_or_log(message: str) -> None: metrics = MetricsPublisherProxy() if event.requestData.providerLogGroupName and provider_sess: - HookProviderLogHandler.setup(event, provider_sess) + HookProviderLogHandler.setup(event, provider_sess, self.log_format) logs_setup = True metrics.add_hook_metrics_publisher( provider_sess, event.hookTypeName, event.awsAccountId diff --git a/src/cloudformation_cli_python_lib/log_delivery.py b/src/cloudformation_cli_python_lib/log_delivery.py index d620899..f230325 100644 --- a/src/cloudformation_cli_python_lib/log_delivery.py +++ b/src/cloudformation_cli_python_lib/log_delivery.py @@ -35,7 +35,10 @@ def _get_existing_logger(cls) -> Optional["ProviderLogHandler"]: @classmethod def setup( - cls, request: HandlerRequest, provider_sess: Optional[SessionProxy] + cls, + request: HandlerRequest, + provider_sess: Optional[SessionProxy], + log_format: Optional[logging.Formatter] = None, ) -> None: log_group = request.requestData.providerLogGroupName if request.stackId and request.requestData.logicalResourceId: @@ -50,6 +53,10 @@ def setup( # we just refresh the client with new creds log_handler.client = provider_sess.client("logs") return + + if log_handler and log_format: + log_handler.setFormatter(log_format) + # filter provider messages from platform provider = request.resourceType.replace("::", "_").lower() log_handler = cls( @@ -114,7 +121,10 @@ def _get_existing_logger(cls) -> Optional["HookProviderLogHandler"]: @classmethod def setup( # type: ignore - cls, request: HookInvocationRequest, provider_sess: Optional[SessionProxy] + cls, + request: HookInvocationRequest, + provider_sess: Optional[SessionProxy], + log_format: Optional[logging.Formatter] = None, ) -> None: log_group = request.requestData.providerLogGroupName if request.stackId and request.requestData.targetLogicalId: @@ -129,6 +139,10 @@ def setup( # type: ignore # we just refresh the client with new creds log_handler.client = provider_sess.client("logs") return + + if log_handler and log_format: + log_handler.setFormatter(log_format) + # filter provider messages from platform provider = request.hookTypeName.replace("::", "_").lower() logging.getLogger().handlers[0].addFilter(ProviderFilter(provider)) diff --git a/src/cloudformation_cli_python_lib/resource.py b/src/cloudformation_cli_python_lib/resource.py index ace3116..c4d8cd1 100644 --- a/src/cloudformation_cli_python_lib/resource.py +++ b/src/cloudformation_cli_python_lib/resource.py @@ -61,6 +61,7 @@ def __init__( type_name: str, resouce_model_cls: Type[BaseModel], type_configuration_model_cls: Optional[Type[BaseModel]] = None, + log_format: Optional[logging.Formatter] = None, ) -> None: self.type_name = type_name self._model_cls: Type[BaseModel] = resouce_model_cls @@ -68,6 +69,7 @@ def __init__( Type[BaseModel] ] = type_configuration_model_cls self._handlers: MutableMapping[Action, HandlerSignature] = {} + self.log_format = log_format def handler(self, action: Action) -> Callable[[HandlerSignature], HandlerSignature]: def _add_handler(f: HandlerSignature) -> HandlerSignature: @@ -200,7 +202,7 @@ def print_or_log(message: str) -> None: metrics = MetricsPublisherProxy() if event.requestData.providerLogGroupName and provider_sess: - ProviderLogHandler.setup(event, provider_sess) + ProviderLogHandler.setup(event, provider_sess, self.log_format) logs_setup = True metrics.add_metrics_publisher(provider_sess, event.resourceType) diff --git a/src/setup.py b/src/setup.py index 1a98285..39d75fa 100644 --- a/src/setup.py +++ b/src/setup.py @@ -3,7 +3,7 @@ setup( name="cloudformation-cli-python-lib", - version="2.1.12", + version="2.1.13a0", description=__doc__, author="Amazon Web Services", author_email="aws-cloudformation-developers@amazon.com", From 8e9be7ba3c614bb7ab24418eeccaa3b0670a8ef7 Mon Sep 17 00:00:00 2001 From: thoward Date: Thu, 21 Jul 2022 15:38:41 -0700 Subject: [PATCH 2/5] move set format condition --- src/cloudformation_cli_python_lib/log_delivery.py | 14 ++++++++------ src/setup.py | 2 +- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/src/cloudformation_cli_python_lib/log_delivery.py b/src/cloudformation_cli_python_lib/log_delivery.py index f230325..3a38a0c 100644 --- a/src/cloudformation_cli_python_lib/log_delivery.py +++ b/src/cloudformation_cli_python_lib/log_delivery.py @@ -54,14 +54,15 @@ def setup( log_handler.client = provider_sess.client("logs") return - if log_handler and log_format: - log_handler.setFormatter(log_format) - # filter provider messages from platform provider = request.resourceType.replace("::", "_").lower() log_handler = cls( group=log_group, stream=stream_name, session=provider_sess ) + + if log_format: + log_handler.setFormatter(log_format) + # add log handler to root, so that provider gets plugin logs too logging.getLogger().addHandler(log_handler) logging.getLogger().handlers[0].addFilter(ProviderFilter(provider)) @@ -140,14 +141,15 @@ def setup( # type: ignore log_handler.client = provider_sess.client("logs") return - if log_handler and log_format: - log_handler.setFormatter(log_format) - # filter provider messages from platform provider = request.hookTypeName.replace("::", "_").lower() logging.getLogger().handlers[0].addFilter(ProviderFilter(provider)) log_handler = cls( group=log_group, stream=stream_name, session=provider_sess ) + + if log_format: + log_handler.setFormatter(log_format) + # add log handler to root, so that provider gets plugin logs too logging.getLogger().addHandler(log_handler) diff --git a/src/setup.py b/src/setup.py index 39d75fa..0efe6aa 100644 --- a/src/setup.py +++ b/src/setup.py @@ -3,7 +3,7 @@ setup( name="cloudformation-cli-python-lib", - version="2.1.13a0", + version="2.1.13a1", description=__doc__, author="Amazon Web Services", author_email="aws-cloudformation-developers@amazon.com", From c34164f931a78321f666bca48a2915dff5bc83a3 Mon Sep 17 00:00:00 2001 From: thoward Date: Thu, 21 Jul 2022 16:17:13 -0700 Subject: [PATCH 3/5] test test --- tests/lib/log_delivery_test.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/tests/lib/log_delivery_test.py b/tests/lib/log_delivery_test.py index 8cfeaff..ca9eb69 100644 --- a/tests/lib/log_delivery_test.py +++ b/tests/lib/log_delivery_test.py @@ -158,6 +158,24 @@ def test_setup_with_provider_creds_and_stack_id_and_logical_resource_id( assert payload.requestData.logicalResourceId in plh.stream +def test_setup_with_formatter( + setup_patches, mock_session +): + payload, _hook_payload, p_logger, p__get_logger, _p__get_hook_logger = setup_patches + with p_logger as mock_log, p__get_logger as mock_get: + mock_get.return_value = None + ProviderLogHandler.setup(payload, mock_session, logging.Formatter()) + mock_session.client.assert_called_once_with("logs") + mock_log.return_value.addHandler.assert_called_once() + print(mock_log.handlers[0]) + + assert 0 == 1 + + plh = mock_log.return_value.addHandler.call_args[0][0] + assert payload.stackId in plh.stream + assert payload.requestData.logicalResourceId in plh.stream + + def test_setup_with_provider_creds_without_stack_id(setup_patches, mock_session): payload, _hook_payload, p_logger, p__get_logger, _p__get_hook_logger = setup_patches payload.stackId = None From 7a08392a610408fec6fe1b5b7baf71dc423e8c0e Mon Sep 17 00:00:00 2001 From: thoward Date: Thu, 21 Jul 2022 16:48:49 -0700 Subject: [PATCH 4/5] add tests --- tests/lib/log_delivery_test.py | 51 +++++++++++++++++++++------------- 1 file changed, 32 insertions(+), 19 deletions(-) diff --git a/tests/lib/log_delivery_test.py b/tests/lib/log_delivery_test.py index ca9eb69..bd54d74 100644 --- a/tests/lib/log_delivery_test.py +++ b/tests/lib/log_delivery_test.py @@ -86,10 +86,17 @@ def setup_patches(mock_logger): make_hook_payload(), patch_logger, patch__get_logger, - patch__get_hook_logger, + patch__get_hook_logger ) +@pytest.fixture +def mock_handler_set_formatter(): + patch__set_handler_formatter = patch.object(ProviderLogHandler, "setFormatter") + patch__set_hook_handler_formatter = patch.object(HookProviderLogHandler, "setFormatter") + return patch__set_handler_formatter, patch__set_hook_handler_formatter + + @pytest.fixture def mock_provider_handler(): plh = ProviderLogHandler( @@ -158,24 +165,6 @@ def test_setup_with_provider_creds_and_stack_id_and_logical_resource_id( assert payload.requestData.logicalResourceId in plh.stream -def test_setup_with_formatter( - setup_patches, mock_session -): - payload, _hook_payload, p_logger, p__get_logger, _p__get_hook_logger = setup_patches - with p_logger as mock_log, p__get_logger as mock_get: - mock_get.return_value = None - ProviderLogHandler.setup(payload, mock_session, logging.Formatter()) - mock_session.client.assert_called_once_with("logs") - mock_log.return_value.addHandler.assert_called_once() - print(mock_log.handlers[0]) - - assert 0 == 1 - - plh = mock_log.return_value.addHandler.call_args[0][0] - assert payload.stackId in plh.stream - assert payload.requestData.logicalResourceId in plh.stream - - def test_setup_with_provider_creds_without_stack_id(setup_patches, mock_session): payload, _hook_payload, p_logger, p__get_logger, _p__get_hook_logger = setup_patches payload.stackId = None @@ -215,6 +204,18 @@ def test_setup_existing_logger(setup_patches, mock_session): mock_log.return_value.addHandler.assert_not_called() +def test_setup_with_formatter(setup_patches, mock_session, mock_handler_set_formatter): + payload, _hook_payload, p_logger, p__get_logger, _p__get_hook_logger = setup_patches + p__set_handler_formatter, _p__set_hook_handler_formatter = mock_handler_set_formatter + formatter = logging.Formatter() + with p_logger as mock_log, p__get_logger as mock_get, p__set_handler_formatter as mock_set_formatter: + mock_get.return_value = None + ProviderLogHandler.setup(payload, mock_session, formatter) + mock_session.client.assert_called_once_with("logs") + mock_log.return_value.addHandler.assert_called_once() + mock_set_formatter.assert_called_once_with(formatter) + + def test_setup_without_log_group_should_not_set_up(mock_logger, mock_session): patch_logger = patch( "cloudformation_cli_python_lib.log_delivery.logging.getLogger", @@ -405,6 +406,18 @@ def test_setup_existing_hook_logger(setup_patches, mock_session): mock_log.return_value.addHandler.assert_not_called() +def test_setup_with_hook_formatter(setup_patches, mock_session, mock_handler_set_formatter): + _payload, hook_payload, p_logger, p__get_logger, _p__get_hook_logger = setup_patches + _p__set_handler_formatter, p__set_hook_handler_formatter = mock_handler_set_formatter + formatter = logging.Formatter() + with p_logger as mock_log, p__get_logger as mock_get, p__set_hook_handler_formatter as mock_set_formatter: + mock_get.return_value = None + HookProviderLogHandler.setup(hook_payload, mock_session, formatter) + mock_session.client.assert_called_once_with("logs") + mock_log.return_value.addHandler.assert_called_once() + mock_set_formatter.assert_called_once_with(formatter) + + def test_setup_without_hook_log_group_should_not_set_up(mock_logger, mock_session): patch_logger = patch( "cloudformation_cli_python_lib.log_delivery.logging.getLogger", From 42a4192a2d6188c10bb84aef574a74feb1f0e6ab Mon Sep 17 00:00:00 2001 From: thoward Date: Thu, 21 Jul 2022 16:56:20 -0700 Subject: [PATCH 5/5] lint pass --- tests/lib/log_delivery_test.py | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/tests/lib/log_delivery_test.py b/tests/lib/log_delivery_test.py index bd54d74..002437f 100644 --- a/tests/lib/log_delivery_test.py +++ b/tests/lib/log_delivery_test.py @@ -86,14 +86,16 @@ def setup_patches(mock_logger): make_hook_payload(), patch_logger, patch__get_logger, - patch__get_hook_logger + patch__get_hook_logger, ) @pytest.fixture def mock_handler_set_formatter(): patch__set_handler_formatter = patch.object(ProviderLogHandler, "setFormatter") - patch__set_hook_handler_formatter = patch.object(HookProviderLogHandler, "setFormatter") + patch__set_hook_handler_formatter = patch.object( + HookProviderLogHandler, "setFormatter" + ) return patch__set_handler_formatter, patch__set_hook_handler_formatter @@ -206,9 +208,12 @@ def test_setup_existing_logger(setup_patches, mock_session): def test_setup_with_formatter(setup_patches, mock_session, mock_handler_set_formatter): payload, _hook_payload, p_logger, p__get_logger, _p__get_hook_logger = setup_patches - p__set_handler_formatter, _p__set_hook_handler_formatter = mock_handler_set_formatter + ( + p__set_handler_formatter, + _p__set_hook_handler_formatter, + ) = mock_handler_set_formatter formatter = logging.Formatter() - with p_logger as mock_log, p__get_logger as mock_get, p__set_handler_formatter as mock_set_formatter: + with p_logger as mock_log, p__get_logger as mock_get, p__set_handler_formatter as mock_set_formatter: # pylint: disable=C0301 # noqa: B950 mock_get.return_value = None ProviderLogHandler.setup(payload, mock_session, formatter) mock_session.client.assert_called_once_with("logs") @@ -406,11 +411,16 @@ def test_setup_existing_hook_logger(setup_patches, mock_session): mock_log.return_value.addHandler.assert_not_called() -def test_setup_with_hook_formatter(setup_patches, mock_session, mock_handler_set_formatter): +def test_setup_with_hook_formatter( + setup_patches, mock_session, mock_handler_set_formatter +): _payload, hook_payload, p_logger, p__get_logger, _p__get_hook_logger = setup_patches - _p__set_handler_formatter, p__set_hook_handler_formatter = mock_handler_set_formatter + ( + _p__set_handler_formatter, + p__set_hook_handler_formatter, + ) = mock_handler_set_formatter formatter = logging.Formatter() - with p_logger as mock_log, p__get_logger as mock_get, p__set_hook_handler_formatter as mock_set_formatter: + with p_logger as mock_log, p__get_logger as mock_get, p__set_hook_handler_formatter as mock_set_formatter: # pylint: disable=C0301 # noqa: B950 mock_get.return_value = None HookProviderLogHandler.setup(hook_payload, mock_session, formatter) mock_session.client.assert_called_once_with("logs")