From 4ec8795d556fd3e18cbebc354063f6bb930e5664 Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Tue, 16 Nov 2021 15:16:48 -0800 Subject: [PATCH 1/6] added json_fields argument to pass in payload data --- google/cloud/logging_v2/handlers/handlers.py | 8 +- tests/system/test_system.py | 30 +++++++ tests/unit/handlers/test_handlers.py | 90 ++++++++++++++++++++ tests/unit/handlers/test_structured_log.py | 23 +++++ 4 files changed, 150 insertions(+), 1 deletion(-) diff --git a/google/cloud/logging_v2/handlers/handlers.py b/google/cloud/logging_v2/handlers/handlers.py index 8d14852e1..2736a897d 100644 --- a/google/cloud/logging_v2/handlers/handlers.py +++ b/google/cloud/logging_v2/handlers/handlers.py @@ -221,9 +221,10 @@ def _format_and_parse_message(record, formatter_handler): record (logging.LogRecord): The record object representing the log formatter_handler (logging.Handler): The handler used to format the log """ + passed_json_fields = getattr(record, "json_fields", {}) # if message is a dictionary, return as-is if isinstance(record.msg, collections.abc.Mapping): - return record.msg + return {**record.msg, **passed_json_fields} # format message string based on superclass message = formatter_handler.format(record) try: @@ -235,6 +236,11 @@ def _format_and_parse_message(record, formatter_handler): except (json.decoder.JSONDecodeError, IndexError): # log string is not valid json pass + # if json_fields was set, create a dictionary using that + if passed_json_fields and isinstance(passed_json_fields, collections.abc.Mapping): + if message != 'None': + passed_json_fields['message'] = message + return passed_json_fields # if formatted message contains no content, return None return message if message != "None" else None diff --git a/tests/system/test_system.py b/tests/system/test_system.py index cde722bd6..b6101fedf 100644 --- a/tests/system/test_system.py +++ b/tests/system/test_system.py @@ -551,6 +551,36 @@ def test_handlers_w_extras(self): ) self.assertEqual(entries[0].resource.type, extra["resource"].type) +def test_handlers_w_json_fields(self): + LOG_MESSAGE = "Testing with json_field extras." + LOGGER_NAME = "json_field_extras" + handler_name = self._logger_name(LOGGER_NAME) + + handler = CloudLoggingHandler( + Config.CLIENT, name=handler_name, transport=SyncTransport + ) + + # only create the logger to delete, hidden otherwise + logger = Config.CLIENT.logger(handler.name) + self.to_delete.append(logger) + + cloud_logger = logging.getLogger(LOGGER_NAME) + cloud_logger.addHandler(handler) + expected_request = {"requestUrl": "localhost"} + expected_source = {"file": "test.py"} + extra = { + "json_fields": {"hello", "world", "two", 2} + } + cloud_logger.warn(LOG_MESSAGE, extra=extra) + + entries = _list_entries(logger) + self.assertEqual(len(entries), 1) + payload = entries[0].payload + self.assertEqual(payload["message"], LOG_MESSAGE) + self.assertEqual(payload["hello"], "world") + self.assertEqual(payload["two"], 2) + + def test_log_root_handler(self): LOG_MESSAGE = "It was the best of times." diff --git a/tests/unit/handlers/test_handlers.py b/tests/unit/handlers/test_handlers.py index d36dc8959..1cd31e880 100644 --- a/tests/unit/handlers/test_handlers.py +++ b/tests/unit/handlers/test_handlers.py @@ -447,6 +447,40 @@ def test_emit_dict(self): ), ) + def test_emit_dict(self): + """ + User can add json_fields to the record, which should populate the payload + """ + from google.cloud.logging_v2.logger import _GLOBAL_RESOURCE + + client = _Client(self.PROJECT) + handler = self._make_one( + client, transport=_Transport, resource=_GLOBAL_RESOURCE, + ) + message = "message" + json_fields = {"hello": "world"} + logname = "logname" + expected_label = {"python_logger": logname} + record = logging.LogRecord( + logname, logging.INFO, None, None, message, None, None + ) + setattr(record, "json_fields", json_fields) + handler.handle(record) + + self.assertEqual( + handler.transport.send_called_with, + ( + record, + {"message":"message", "hello":"world"}, + _GLOBAL_RESOURCE, + expected_label, + None, + None, + None, + None, + ), + ) + def test_emit_with_encoded_json(self): """ Handler should parse json encoded as a string @@ -608,6 +642,62 @@ def test_broken_encoded_dict(self): result = _format_and_parse_message(record, handler) self.assertEqual(result, message) + def test_json_fields(self): + """ + record.json_fields should populate the json payload + """ + from google.cloud.logging_v2.handlers.handlers import _format_and_parse_message + + message = 'hello' + json_fields = {"key":"val"} + record = logging.LogRecord("logname", None, None, None, message, None, None) + setattr(record, "json_fields", json_fields) + handler = logging.StreamHandler() + result = _format_and_parse_message(record, handler) + self.assertEqual(result, {"message":message, "key":"val"}) + + def test_empty_json_fields(self): + """ + empty jsond_field dictionaries should result in a string output + """ + from google.cloud.logging_v2.handlers.handlers import _format_and_parse_message + + message = 'hello' + record = logging.LogRecord("logname", None, None, None, message, None, None) + setattr(record, "json_fields", {}) + handler = logging.StreamHandler() + result = _format_and_parse_message(record, handler) + self.assertEqual(result, message) + + def test_json_fields_empty_message(self): + """ + empty message fields should not be added to json_fields dictionaries + """ + from google.cloud.logging_v2.handlers.handlers import _format_and_parse_message + + message = None + json_fields = {"key":"val"} + record = logging.LogRecord("logname", None, None, None, message, None, None) + setattr(record, "json_fields", json_fields) + handler = logging.StreamHandler() + result = _format_and_parse_message(record, handler) + self.assertEqual(result, json_fields) + + def test_json_fields_with_json_message(self): + """ + if json_fields and message are both dicts, they should be combined + """ + from google.cloud.logging_v2.handlers.handlers import _format_and_parse_message + + message = {"key_m":"val_m"} + json_fields = {"key_j":"val_j"} + record = logging.LogRecord("logname", None, None, None, message, None, None) + setattr(record, "json_fields", json_fields) + handler = logging.StreamHandler() + result = _format_and_parse_message(record, handler) + self.assertEqual(result['key_m'], message["key_m"]) + self.assertEqual(result['key_j'], json_fields["key_j"]) + class TestSetupLogging(unittest.TestCase): def _call_fut(self, handler, excludes=None): diff --git a/tests/unit/handlers/test_structured_log.py b/tests/unit/handlers/test_structured_log.py index d9dfa2512..1d3e8f25b 100644 --- a/tests/unit/handlers/test_structured_log.py +++ b/tests/unit/handlers/test_structured_log.py @@ -321,3 +321,26 @@ def test_format_overrides(self): result = json.loads(handler.format(record)) for (key, value) in expected_payload.items(): self.assertEqual(value, result[key]) + + def test_format_with_json_fields(self): + """ + User can add json_fields to the record, which should populate the payload + """ + import logging + import json + + handler = self._make_one() + message = "name: %s" + name_arg = "Daniel" + expected_result = "name: Daniel" + json_fields = {"hello": "world", "number": 12} + record = logging.LogRecord( + None, logging.INFO, None, None, message, name_arg, None, + ) + record.created = None + setattr(record, "json_fields", json_fields) + handler.filter(record) + result = json.loads(handler.format(record)) + self.assertEqual(result['message'], expected_result) + self.assertEqual(result['hello'], 'world') + self.assertEqual(result['number'], 12) From 6ac765856dccab14c32c3739f73333c5f13e26f6 Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Tue, 16 Nov 2021 15:23:58 -0800 Subject: [PATCH 2/6] fixed merge issue --- google/cloud/logging_v2/client.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/google/cloud/logging_v2/client.py b/google/cloud/logging_v2/client.py index 92ab72a3a..2deef69e3 100644 --- a/google/cloud/logging_v2/client.py +++ b/google/cloud/logging_v2/client.py @@ -376,7 +376,7 @@ def get_default_handler(self, **kw): if monitored_resource.type == _GAE_RESOURCE_TYPE: return CloudLoggingHandler(self, resource=monitored_resource, **kw) elif monitored_resource.type == _GKE_RESOURCE_TYPE: - return ContainerEngineHandler(**kw) + return StructuredLogHandler(**kw, project_id=self.project) elif monitored_resource.type == _GCF_RESOURCE_TYPE: # __stdout__ stream required to support structured logging on Python 3.7 kw["stream"] = kw.get("stream", sys.__stdout__) From f8107cb178faa650cbd3adc1622999eed528c049 Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Tue, 16 Nov 2021 15:53:13 -0800 Subject: [PATCH 3/6] added extra type check --- google/cloud/logging_v2/handlers/handlers.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/google/cloud/logging_v2/handlers/handlers.py b/google/cloud/logging_v2/handlers/handlers.py index 2736a897d..e960f9d19 100644 --- a/google/cloud/logging_v2/handlers/handlers.py +++ b/google/cloud/logging_v2/handlers/handlers.py @@ -224,7 +224,11 @@ def _format_and_parse_message(record, formatter_handler): passed_json_fields = getattr(record, "json_fields", {}) # if message is a dictionary, return as-is if isinstance(record.msg, collections.abc.Mapping): - return {**record.msg, **passed_json_fields} + payload = record.msg + if passed_json_fields and isinstance(passed_json_fields, collections.abc.Mapping): + # append any json data in `json_fields` + payload = {**payload, **passed_json_fields} + return payload # format message string based on superclass message = formatter_handler.format(record) try: From c147da3c8a49852b8dfc4c23eb565a998022217f Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Thu, 2 Dec 2021 14:38:50 -0800 Subject: [PATCH 4/6] fixed lint/test issues --- google/cloud/logging_v2/client.py | 2 +- google/cloud/logging_v2/handlers/handlers.py | 8 ++++--- tests/system/test_system.py | 9 ++------ tests/unit/handlers/test_handlers.py | 22 ++++++++++---------- tests/unit/handlers/test_structured_log.py | 6 +++--- 5 files changed, 22 insertions(+), 25 deletions(-) diff --git a/google/cloud/logging_v2/client.py b/google/cloud/logging_v2/client.py index 2deef69e3..3d5ea24fc 100644 --- a/google/cloud/logging_v2/client.py +++ b/google/cloud/logging_v2/client.py @@ -376,7 +376,7 @@ def get_default_handler(self, **kw): if monitored_resource.type == _GAE_RESOURCE_TYPE: return CloudLoggingHandler(self, resource=monitored_resource, **kw) elif monitored_resource.type == _GKE_RESOURCE_TYPE: - return StructuredLogHandler(**kw, project_id=self.project) + return StructuredLogHandler(**kw, project_id=self.project) elif monitored_resource.type == _GCF_RESOURCE_TYPE: # __stdout__ stream required to support structured logging on Python 3.7 kw["stream"] = kw.get("stream", sys.__stdout__) diff --git a/google/cloud/logging_v2/handlers/handlers.py b/google/cloud/logging_v2/handlers/handlers.py index e960f9d19..2bff32280 100644 --- a/google/cloud/logging_v2/handlers/handlers.py +++ b/google/cloud/logging_v2/handlers/handlers.py @@ -225,7 +225,9 @@ def _format_and_parse_message(record, formatter_handler): # if message is a dictionary, return as-is if isinstance(record.msg, collections.abc.Mapping): payload = record.msg - if passed_json_fields and isinstance(passed_json_fields, collections.abc.Mapping): + if passed_json_fields and isinstance( + passed_json_fields, collections.abc.Mapping + ): # append any json data in `json_fields` payload = {**payload, **passed_json_fields} return payload @@ -242,8 +244,8 @@ def _format_and_parse_message(record, formatter_handler): pass # if json_fields was set, create a dictionary using that if passed_json_fields and isinstance(passed_json_fields, collections.abc.Mapping): - if message != 'None': - passed_json_fields['message'] = message + if message != "None": + passed_json_fields["message"] = message return passed_json_fields # if formatted message contains no content, return None return message if message != "None" else None diff --git a/tests/system/test_system.py b/tests/system/test_system.py index b6101fedf..24050e8b3 100644 --- a/tests/system/test_system.py +++ b/tests/system/test_system.py @@ -551,7 +551,7 @@ def test_handlers_w_extras(self): ) self.assertEqual(entries[0].resource.type, extra["resource"].type) -def test_handlers_w_json_fields(self): + def test_handlers_w_json_fields(self): LOG_MESSAGE = "Testing with json_field extras." LOGGER_NAME = "json_field_extras" handler_name = self._logger_name(LOGGER_NAME) @@ -566,11 +566,7 @@ def test_handlers_w_json_fields(self): cloud_logger = logging.getLogger(LOGGER_NAME) cloud_logger.addHandler(handler) - expected_request = {"requestUrl": "localhost"} - expected_source = {"file": "test.py"} - extra = { - "json_fields": {"hello", "world", "two", 2} - } + extra = {"json_fields": {"hello": "world", "two": 2}} cloud_logger.warn(LOG_MESSAGE, extra=extra) entries = _list_entries(logger) @@ -580,7 +576,6 @@ def test_handlers_w_json_fields(self): self.assertEqual(payload["hello"], "world") self.assertEqual(payload["two"], 2) - def test_log_root_handler(self): LOG_MESSAGE = "It was the best of times." diff --git a/tests/unit/handlers/test_handlers.py b/tests/unit/handlers/test_handlers.py index 1cd31e880..71a709b6a 100644 --- a/tests/unit/handlers/test_handlers.py +++ b/tests/unit/handlers/test_handlers.py @@ -447,7 +447,7 @@ def test_emit_dict(self): ), ) - def test_emit_dict(self): + def test_emit_w_json_extras(self): """ User can add json_fields to the record, which should populate the payload """ @@ -471,7 +471,7 @@ def test_emit_dict(self): handler.transport.send_called_with, ( record, - {"message":"message", "hello":"world"}, + {"message": "message", "hello": "world"}, _GLOBAL_RESOURCE, expected_label, None, @@ -648,13 +648,13 @@ def test_json_fields(self): """ from google.cloud.logging_v2.handlers.handlers import _format_and_parse_message - message = 'hello' - json_fields = {"key":"val"} + message = "hello" + json_fields = {"key": "val"} record = logging.LogRecord("logname", None, None, None, message, None, None) setattr(record, "json_fields", json_fields) handler = logging.StreamHandler() result = _format_and_parse_message(record, handler) - self.assertEqual(result, {"message":message, "key":"val"}) + self.assertEqual(result, {"message": message, "key": "val"}) def test_empty_json_fields(self): """ @@ -662,7 +662,7 @@ def test_empty_json_fields(self): """ from google.cloud.logging_v2.handlers.handlers import _format_and_parse_message - message = 'hello' + message = "hello" record = logging.LogRecord("logname", None, None, None, message, None, None) setattr(record, "json_fields", {}) handler = logging.StreamHandler() @@ -676,7 +676,7 @@ def test_json_fields_empty_message(self): from google.cloud.logging_v2.handlers.handlers import _format_and_parse_message message = None - json_fields = {"key":"val"} + json_fields = {"key": "val"} record = logging.LogRecord("logname", None, None, None, message, None, None) setattr(record, "json_fields", json_fields) handler = logging.StreamHandler() @@ -689,14 +689,14 @@ def test_json_fields_with_json_message(self): """ from google.cloud.logging_v2.handlers.handlers import _format_and_parse_message - message = {"key_m":"val_m"} - json_fields = {"key_j":"val_j"} + message = {"key_m": "val_m"} + json_fields = {"key_j": "val_j"} record = logging.LogRecord("logname", None, None, None, message, None, None) setattr(record, "json_fields", json_fields) handler = logging.StreamHandler() result = _format_and_parse_message(record, handler) - self.assertEqual(result['key_m'], message["key_m"]) - self.assertEqual(result['key_j'], json_fields["key_j"]) + self.assertEqual(result["key_m"], message["key_m"]) + self.assertEqual(result["key_j"], json_fields["key_j"]) class TestSetupLogging(unittest.TestCase): diff --git a/tests/unit/handlers/test_structured_log.py b/tests/unit/handlers/test_structured_log.py index 1d3e8f25b..08e4c2906 100644 --- a/tests/unit/handlers/test_structured_log.py +++ b/tests/unit/handlers/test_structured_log.py @@ -341,6 +341,6 @@ def test_format_with_json_fields(self): setattr(record, "json_fields", json_fields) handler.filter(record) result = json.loads(handler.format(record)) - self.assertEqual(result['message'], expected_result) - self.assertEqual(result['hello'], 'world') - self.assertEqual(result['number'], 12) + self.assertEqual(result["message"], expected_result) + self.assertEqual(result["hello"], "world") + self.assertEqual(result["number"], 12) From 538fc09737c42b6738bd943b3484b1b8978ce2a7 Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Tue, 7 Dec 2021 11:26:25 -0800 Subject: [PATCH 5/6] updated submodule --- tests/environment | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/environment b/tests/environment index a201d48f1..2ff3cb26a 160000 --- a/tests/environment +++ b/tests/environment @@ -1 +1 @@ -Subproject commit a201d48f163db4ff799305188d380f620ebf345a +Subproject commit 2ff3cb26a09d574715573bcf0a2c480eeac73311 From a7b811c86a200d68e3b5f599088a47fedc131ffc Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Tue, 7 Dec 2021 11:27:48 -0800 Subject: [PATCH 6/6] added comments --- google/cloud/logging_v2/handlers/handlers.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/google/cloud/logging_v2/handlers/handlers.py b/google/cloud/logging_v2/handlers/handlers.py index 2bff32280..39bcbca79 100644 --- a/google/cloud/logging_v2/handlers/handlers.py +++ b/google/cloud/logging_v2/handlers/handlers.py @@ -222,13 +222,13 @@ def _format_and_parse_message(record, formatter_handler): formatter_handler (logging.Handler): The handler used to format the log """ passed_json_fields = getattr(record, "json_fields", {}) - # if message is a dictionary, return as-is + # if message is a dictionary, use dictionary directly if isinstance(record.msg, collections.abc.Mapping): payload = record.msg + # attach any extra json fields if present if passed_json_fields and isinstance( passed_json_fields, collections.abc.Mapping ): - # append any json data in `json_fields` payload = {**payload, **passed_json_fields} return payload # format message string based on superclass