From 24d60b13a8512922d4c0a96c62d739d53f524bb6 Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Wed, 11 Jan 2023 17:14:21 -0800 Subject: [PATCH 1/4] fix: instrumentation entries should not contain user labels --- google/cloud/logging_v2/_instrumentation.py | 8 ++++++-- tests/unit/test__instrumentation.py | 10 ++++++++++ 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/google/cloud/logging_v2/_instrumentation.py b/google/cloud/logging_v2/_instrumentation.py index 0d9de76d3..9d071b98e 100644 --- a/google/cloud/logging_v2/_instrumentation.py +++ b/google/cloud/logging_v2/_instrumentation.py @@ -67,8 +67,12 @@ def _create_diagnostic_entry(name=_PYTHON_LIBRARY_NAME, version=_LIBRARY_VERSION _INSTRUMENTATION_SOURCE_KEY: [_get_instrumentation_source(name, version)] } } - kw["severity"] = "INFO" - entry = StructEntry(payload=payload, **kw) + active_kws = {"severity": "INFO"} + # only keep the log_name and resource from the parent log + for key in ["log_name", "resource"]: + if key in kw.keys(): + active_kws[key] = kw[key] + entry = StructEntry(payload=payload, **active_kws) return entry diff --git a/tests/unit/test__instrumentation.py b/tests/unit/test__instrumentation.py index 501301c34..2b89361b5 100644 --- a/tests/unit/test__instrumentation.py +++ b/tests/unit/test__instrumentation.py @@ -63,3 +63,13 @@ def test_truncate_long_values(self): self.assertEqual(expected_name, self._get_diagonstic_value(entry, "name")) self.assertEqual(expected_version, self._get_diagonstic_value(entry, "version")) + + def test_drop_labels(self): + """Labels should not be copied in instrumentation log""" + test_logname = "test-name" + test_labels = {"hello": "world"} + entry = i._create_diagnostic_entry( + name=self.LONG_NAME, version=self.LONG_VERSION, log_name=test_logname, labels=test_labels, + ) + self.assertEqual(entry.log_name, test_logname) + self.assertIsNone(entry.labels) From f8da662bee5ca3f879550074141098fb87dd2dc6 Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Thu, 12 Jan 2023 08:26:19 -0800 Subject: [PATCH 2/4] fixed lint issue --- tests/unit/test__instrumentation.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/unit/test__instrumentation.py b/tests/unit/test__instrumentation.py index 2b89361b5..0cee828c3 100644 --- a/tests/unit/test__instrumentation.py +++ b/tests/unit/test__instrumentation.py @@ -69,7 +69,10 @@ def test_drop_labels(self): test_logname = "test-name" test_labels = {"hello": "world"} entry = i._create_diagnostic_entry( - name=self.LONG_NAME, version=self.LONG_VERSION, log_name=test_logname, labels=test_labels, + name=self.LONG_NAME, + version=self.LONG_VERSION, + log_name=test_logname, + labels=test_labels, ) self.assertEqual(entry.log_name, test_logname) self.assertIsNone(entry.labels) From 4237eff6a21430295c28b47d9d4ed4c8a7544b42 Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Tue, 17 Jan 2023 13:59:56 -0800 Subject: [PATCH 3/4] remove severity from instrumentation log --- google/cloud/logging_v2/_instrumentation.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/google/cloud/logging_v2/_instrumentation.py b/google/cloud/logging_v2/_instrumentation.py index 9d071b98e..553b3f94c 100644 --- a/google/cloud/logging_v2/_instrumentation.py +++ b/google/cloud/logging_v2/_instrumentation.py @@ -67,11 +67,9 @@ def _create_diagnostic_entry(name=_PYTHON_LIBRARY_NAME, version=_LIBRARY_VERSION _INSTRUMENTATION_SOURCE_KEY: [_get_instrumentation_source(name, version)] } } - active_kws = {"severity": "INFO"} # only keep the log_name and resource from the parent log - for key in ["log_name", "resource"]: - if key in kw.keys(): - active_kws[key] = kw[key] + allow_list = ("log_name", "resource") + active_kws = {k: v for k, v in kw.items() if k in allow_list} entry = StructEntry(payload=payload, **active_kws) return entry From 3a1e97f23fc9b04fe260f3ff33b04c1522e451e7 Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Tue, 17 Jan 2023 14:00:54 -0800 Subject: [PATCH 4/4] added test against instrumentation fields --- tests/unit/test__instrumentation.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/unit/test__instrumentation.py b/tests/unit/test__instrumentation.py index 0cee828c3..dc330b0ca 100644 --- a/tests/unit/test__instrumentation.py +++ b/tests/unit/test__instrumentation.py @@ -76,3 +76,6 @@ def test_drop_labels(self): ) self.assertEqual(entry.log_name, test_logname) self.assertIsNone(entry.labels) + # ensure only expected fields exist in entry + expected_keys = set(["logName", "resource", "jsonPayload"]) + self.assertEqual(set(entry.to_api_repr().keys()), expected_keys)