From 55383da07cab1a9b68bb8bd11737d4b0dbbc8685 Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Tue, 8 Dec 2020 16:19:33 -0800 Subject: [PATCH 01/35] added http request to transports --- .../handlers/transports/background_thread.py | 12 ++++++++++-- google/cloud/logging_v2/handlers/transports/base.py | 3 ++- google/cloud/logging_v2/handlers/transports/sync.py | 6 +++++- 3 files changed, 17 insertions(+), 4 deletions(-) diff --git a/google/cloud/logging_v2/handlers/transports/background_thread.py b/google/cloud/logging_v2/handlers/transports/background_thread.py index 873fa452..7cea98ef 100644 --- a/google/cloud/logging_v2/handlers/transports/background_thread.py +++ b/google/cloud/logging_v2/handlers/transports/background_thread.py @@ -223,7 +223,8 @@ def _main_thread_terminated(self): ) def enqueue( - self, record, message, *, resource=None, labels=None, trace=None, span_id=None + self, record, message, *, resource=None, labels=None, trace=None, + span_id=None, http_request=None ): """Queues a log entry to be written by the background thread. @@ -237,6 +238,8 @@ def enqueue( trace (Optional[str]): TraceID to apply to the logging entry. span_id (Optional[str]): Span_id within the trace for the log entry. Specify the trace parameter if span_id is set. + http_request (Optional[dict]): Info about HTTP request associated + with the entry. """ queue_entry = { "info": {"message": message, "python_logger": record.name}, @@ -246,6 +249,7 @@ def enqueue( "trace": trace, "span_id": span_id, "timestamp": datetime.datetime.utcfromtimestamp(record.created), + "http_request": http_request } self._queue.put_nowait(queue_entry) @@ -292,7 +296,8 @@ def __init__( self.worker.start() def send( - self, record, message, resource=None, labels=None, trace=None, span_id=None + self, record, message, resource=None, labels=None, trace=None, + span_id=None, , http_request=None ): """Overrides Transport.send(). @@ -306,6 +311,8 @@ def send( trace (Optional[str]): TraceID to apply to the logging entry. span_id (Optional[str]): span_id within the trace for the log entry. Specify the trace parameter if span_id is set. + http_request (Optional[dict]): Info about HTTP request associated + with the entry. """ self.worker.enqueue( record, @@ -314,6 +321,7 @@ def send( labels=labels, trace=trace, span_id=span_id, + http_request=http_request, ) def flush(self): diff --git a/google/cloud/logging_v2/handlers/transports/base.py b/google/cloud/logging_v2/handlers/transports/base.py index c94c7ad7..9904b5ae 100644 --- a/google/cloud/logging_v2/handlers/transports/base.py +++ b/google/cloud/logging_v2/handlers/transports/base.py @@ -23,7 +23,8 @@ class Transport(object): """ def send( - self, record, message, *, resource=None, labels=None, trace=None, span_id=None + self, record, message, *, resource=None, labels=None, trace=None, + span_id=None, http_request=None ): """Transport send to be implemented by subclasses. diff --git a/google/cloud/logging_v2/handlers/transports/sync.py b/google/cloud/logging_v2/handlers/transports/sync.py index 550c2939..ae91c68c 100644 --- a/google/cloud/logging_v2/handlers/transports/sync.py +++ b/google/cloud/logging_v2/handlers/transports/sync.py @@ -31,7 +31,8 @@ def __init__(self, client, name): self.logger = client.logger(name) def send( - self, record, message, *, resource=None, labels=None, trace=None, span_id=None + self, record, message, *, resource=None, labels=None, trace=None, + span_id=None, http_request=None, ): """Overrides transport.send(). @@ -43,6 +44,8 @@ def send( resource (Optional[~logging_v2.resource.Resource]): Monitored resource of the entry. labels (Optional[dict]): Mapping of labels for the entry. + http_request (Optional[dict]): Info about HTTP request associated + with the entry. """ info = {"message": message, "python_logger": record.name} self.logger.log_struct( @@ -52,4 +55,5 @@ def send( labels=labels, trace=trace, span_id=span_id, + http_request=http_request, ) From 3c67803d2b75c392e1f6abc623c4c58db3d788d2 Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Wed, 9 Dec 2020 12:14:44 -0800 Subject: [PATCH 02/35] got it working on gae flex --- google/cloud/logging_v2/handlers/_helpers.py | 36 +++++++++++++++++++ .../cloud/logging_v2/handlers/app_engine.py | 5 ++- .../handlers/transports/background_thread.py | 2 +- 3 files changed, 41 insertions(+), 2 deletions(-) diff --git a/google/cloud/logging_v2/handlers/_helpers.py b/google/cloud/logging_v2/handlers/_helpers.py index 3150e46c..bf483511 100644 --- a/google/cloud/logging_v2/handlers/_helpers.py +++ b/google/cloud/logging_v2/handlers/_helpers.py @@ -64,6 +64,24 @@ def get_trace_id_from_flask(): return trace_id +def get_http_request_from_flask(): + """Get trace_id from flask request headers. + + Returns: + str: TraceID in HTTP request headers. + """ + if flask is None or not flask.request: + return None + + obj = {'request_method':flask.request.method, + 'request_url': flask.request.host_url, + 'request_size': flask.request.content_length, + 'user_agent': flask.request.user_agent.string, + 'remote_ip': flask.request.remote_addr, + 'referer': flask.request.referrer, + } + + return obj def get_trace_id_from_django(): """Get trace_id from django request headers. @@ -102,3 +120,21 @@ def get_trace_id(): return trace_id return None + + +def get_http_request_data(): + """Helper to get trace_id from web application request header. + + Returns: + str: TraceID in HTTP request headers. + """ + checkers = ( + get_http_request_from_flask, + ) + + for checker in checkers: + obj = checker() + if obj is not None: + return obj + + return None diff --git a/google/cloud/logging_v2/handlers/app_engine.py b/google/cloud/logging_v2/handlers/app_engine.py index fed9bd20..641bbeba 100644 --- a/google/cloud/logging_v2/handlers/app_engine.py +++ b/google/cloud/logging_v2/handlers/app_engine.py @@ -22,6 +22,7 @@ import os from google.cloud.logging_v2.handlers._helpers import get_trace_id +from google.cloud.logging_v2.handlers._helpers import get_http_request_data from google.cloud.logging_v2.handlers.transports import BackgroundThreadTransport from google.cloud.logging_v2.resource import Resource @@ -119,6 +120,8 @@ def emit(self, record): if _TRACE_ID_LABEL in gae_labels else None ) + http_request = get_http_request_data() self.transport.send( - record, message, resource=self.resource, labels=gae_labels, trace=trace_id + record, message, resource=self.resource, labels=gae_labels, + trace=trace_id, http_request=http_request, ) diff --git a/google/cloud/logging_v2/handlers/transports/background_thread.py b/google/cloud/logging_v2/handlers/transports/background_thread.py index 7cea98ef..86e5578d 100644 --- a/google/cloud/logging_v2/handlers/transports/background_thread.py +++ b/google/cloud/logging_v2/handlers/transports/background_thread.py @@ -297,7 +297,7 @@ def __init__( def send( self, record, message, resource=None, labels=None, trace=None, - span_id=None, , http_request=None + span_id=None, http_request=None ): """Overrides Transport.send(). From 4be2e19fd0d62a5a44434e497382a809c3aa7d1b Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Wed, 9 Dec 2020 13:08:40 -0800 Subject: [PATCH 03/35] refactored _helpers to support http_requests --- google/cloud/logging_v2/handlers/_helpers.py | 99 ++++++++----------- .../cloud/logging_v2/handlers/app_engine.py | 14 +-- 2 files changed, 48 insertions(+), 65 deletions(-) diff --git a/google/cloud/logging_v2/handlers/_helpers.py b/google/cloud/logging_v2/handlers/_helpers.py index bf483511..d4ebcbda 100644 --- a/google/cloud/logging_v2/handlers/_helpers.py +++ b/google/cloud/logging_v2/handlers/_helpers.py @@ -25,6 +25,10 @@ from google.cloud.logging_v2.handlers.middleware.request import _get_django_request _DJANGO_TRACE_HEADER = "HTTP_X_CLOUD_TRACE_CONTEXT" +_DJANGO_LENGTH_HEADER = "CONTENT_LENGTH" +_DJANGO_USERAGENT_HEADER = "HTTP_USER_AGENT" +_DJANGO_REMOTE_ADDR_HEADER = "REMOTE_ADDR" +_DJANGO_REFERER_HEADER = "HTTP_REFERER" _FLASK_TRACE_HEADER = "X_CLOUD_TRACE_CONTEXT" @@ -46,44 +50,34 @@ def format_stackdriver_json(record, message): return json.dumps(payload) -def get_trace_id_from_flask(): +def get_request_data_from_flask(): """Get trace_id from flask request headers. Returns: str: TraceID in HTTP request headers. """ if flask is None or not flask.request: - return None + return None, None + + # build http_request + http_request = { + 'request_method': flask.request.method, + 'request_url': flask.request.host_url, + 'request_size': flask.request.content_length, + 'user_agent': flask.request.user_agent.string, + 'remote_ip': flask.request.remote_addr, + 'referer': flask.request.referrer, + } + # find trace id + trace_id = None header = flask.request.headers.get(_FLASK_TRACE_HEADER) + if header: + trace_id = header.split("/", 1)[0] - if header is None: - return None - - trace_id = header.split("/", 1)[0] + return trace_id, http_request - return trace_id - -def get_http_request_from_flask(): - """Get trace_id from flask request headers. - - Returns: - str: TraceID in HTTP request headers. - """ - if flask is None or not flask.request: - return None - - obj = {'request_method':flask.request.method, - 'request_url': flask.request.host_url, - 'request_size': flask.request.content_length, - 'user_agent': flask.request.user_agent.string, - 'remote_ip': flask.request.remote_addr, - 'referer': flask.request.referrer, - } - - return obj - -def get_trace_id_from_django(): +def get_request_data_from_django(): """Get trace_id from django request headers. Returns: @@ -92,49 +86,42 @@ def get_trace_id_from_django(): request = _get_django_request() if request is None: - return None + return None, None + + # build http_request + http_request = { + 'request_method': request.method, + 'request_url': request.get_full_path(), + 'request_size': request.META.get(_DJANGO_LENGTH_HEADER), + 'user_agent': request.META.get(_DJANGO_USERAGENT_HEADER), + 'remote_ip': request.META.get(_DJANGO_REMOTE_ADDR_HEADER), + 'referer': request.META.get(_DJANGO_REFERER_HEADER), + } + # find trace id + trace_id = None header = request.META.get(_DJANGO_TRACE_HEADER) - if header is None: - return None + if header: + trace_id = header.split("/", 1)[0] - trace_id = header.split("/", 1)[0] + return trace_id, http_request - return trace_id -def get_trace_id(): +def get_request_data(): """Helper to get trace_id from web application request header. Returns: str: TraceID in HTTP request headers. """ checkers = ( - get_trace_id_from_django, - get_trace_id_from_flask, + get_request_data_from_django, + get_request_data_from_flask, ) for checker in checkers: - trace_id = checker() + trace_id, http_request = checker() if trace_id is not None: - return trace_id - - return None - - -def get_http_request_data(): - """Helper to get trace_id from web application request header. - - Returns: - str: TraceID in HTTP request headers. - """ - checkers = ( - get_http_request_from_flask, - ) - - for checker in checkers: - obj = checker() - if obj is not None: - return obj + return trace_id, http_request return None diff --git a/google/cloud/logging_v2/handlers/app_engine.py b/google/cloud/logging_v2/handlers/app_engine.py index 641bbeba..68a58810 100644 --- a/google/cloud/logging_v2/handlers/app_engine.py +++ b/google/cloud/logging_v2/handlers/app_engine.py @@ -21,8 +21,7 @@ import logging import os -from google.cloud.logging_v2.handlers._helpers import get_trace_id -from google.cloud.logging_v2.handlers._helpers import get_http_request_data +from google.cloud.logging_v2.handlers._helpers import get_request_data from google.cloud.logging_v2.handlers.transports import BackgroundThreadTransport from google.cloud.logging_v2.resource import Resource @@ -97,7 +96,7 @@ def get_gae_labels(self): """ gae_labels = {} - trace_id = get_trace_id() + trace_id, _ = get_request_data() if trace_id is not None: gae_labels[_TRACE_ID_LABEL] = trace_id @@ -115,12 +114,9 @@ def emit(self, record): """ message = super(AppEngineHandler, self).format(record) gae_labels = self.get_gae_labels() - trace_id = ( - "projects/%s/traces/%s" % (self.project_id, gae_labels[_TRACE_ID_LABEL]) - if _TRACE_ID_LABEL in gae_labels - else None - ) - http_request = get_http_request_data() + trace_id, http_request = get_request_data() + if trace_id is not None: + trace_id = f"projects/{self.project_id}/{trace_id}" self.transport.send( record, message, resource=self.resource, labels=gae_labels, trace=trace_id, http_request=http_request, From 61115d172f716ec2f0614d784598aa3cd4257b82 Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Wed, 9 Dec 2020 14:41:38 -0800 Subject: [PATCH 04/35] added comments --- google/cloud/logging_v2/handlers/_helpers.py | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/google/cloud/logging_v2/handlers/_helpers.py b/google/cloud/logging_v2/handlers/_helpers.py index d4ebcbda..078cbdf3 100644 --- a/google/cloud/logging_v2/handlers/_helpers.py +++ b/google/cloud/logging_v2/handlers/_helpers.py @@ -51,10 +51,11 @@ def format_stackdriver_json(record, message): def get_request_data_from_flask(): - """Get trace_id from flask request headers. + """Get trace_id and http_request data from flask request headers. Returns: str: TraceID in HTTP request headers. + dict: data about the associated http request. """ if flask is None or not flask.request: return None, None @@ -78,10 +79,11 @@ def get_request_data_from_flask(): return trace_id, http_request def get_request_data_from_django(): - """Get trace_id from django request headers. + """Get trace_id and http_request data from django request headers. Returns: str: TraceID in HTTP request headers. + dict: data about the associated http request. """ request = _get_django_request() @@ -109,10 +111,12 @@ def get_request_data_from_django(): def get_request_data(): - """Helper to get trace_id from web application request header. + """Helper to get trace_id and http_request data from supported web + frameworks. Returns: str: TraceID in HTTP request headers. + dict: data about the associated http request. """ checkers = ( get_request_data_from_django, @@ -121,7 +125,7 @@ def get_request_data(): for checker in checkers: trace_id, http_request = checker() - if trace_id is not None: + if http_request is not None: return trace_id, http_request - return None + return None, None From f7aa0e07d76f65308e0b37ed805c1f7a81466b94 Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Wed, 9 Dec 2020 14:42:05 -0800 Subject: [PATCH 05/35] fixed helper tests --- tests/unit/handlers/test__helpers.py | 51 ++++++++++++++++------------ 1 file changed, 30 insertions(+), 21 deletions(-) diff --git a/tests/unit/handlers/test__helpers.py b/tests/unit/handlers/test__helpers.py index 1fbf6c86..7aad4cf0 100644 --- a/tests/unit/handlers/test__helpers.py +++ b/tests/unit/handlers/test__helpers.py @@ -22,7 +22,7 @@ class Test_get_trace_id_from_flask(unittest.TestCase): def _call_fut(): from google.cloud.logging_v2.handlers import _helpers - return _helpers.get_trace_id_from_flask() + return _helpers.get_request_data_from_flask() @staticmethod def create_app(): @@ -39,7 +39,7 @@ def index(): def test_no_context_header(self): app = self.create_app() with app.test_request_context(path="/", headers={}): - trace_id = self._call_fut() + trace_id, http_data = self._call_fut() self.assertIsNone(trace_id) @@ -54,7 +54,7 @@ def test_valid_context_header(self): ) with context: - trace_id = self._call_fut() + trace_id, http_data = self._call_fut() self.assertEqual(trace_id, expected_trace_id) @@ -64,7 +64,7 @@ class Test_get_trace_id_from_django(unittest.TestCase): def _call_fut(): from google.cloud.logging_v2.handlers import _helpers - return _helpers.get_trace_id_from_django() + return _helpers.get_request_data_from_django() def setUp(self): from django.conf import settings @@ -89,7 +89,7 @@ def test_no_context_header(self): middleware = request.RequestMiddleware(None) middleware.process_request(django_request) - trace_id = self._call_fut() + trace_id, http_request = self._call_fut() self.assertIsNone(trace_id) def test_valid_context_header(self): @@ -106,7 +106,7 @@ def test_valid_context_header(self): middleware = request.RequestMiddleware(None) middleware.process_request(django_request) - trace_id = self._call_fut() + trace_id, http_request = self._call_fut() self.assertEqual(trace_id, expected_trace_id) @@ -116,51 +116,60 @@ class Test_get_trace_id(unittest.TestCase): def _call_fut(): from google.cloud.logging_v2.handlers import _helpers - return _helpers.get_trace_id() + return _helpers.get_request_data() def _helper(self, django_return, flask_return): django_patch = mock.patch( - "google.cloud.logging_v2.handlers._helpers.get_trace_id_from_django", + "google.cloud.logging_v2.handlers._helpers.get_request_data_from_django", return_value=django_return, ) flask_patch = mock.patch( - "google.cloud.logging_v2.handlers._helpers.get_trace_id_from_flask", + "google.cloud.logging_v2.handlers._helpers.get_request_data_from_flask", return_value=flask_return, ) with django_patch as django_mock: with flask_patch as flask_mock: - trace_id = self._call_fut() + result = self._call_fut() - return django_mock, flask_mock, trace_id + return django_mock, flask_mock, result def test_from_django(self): - django_mock, flask_mock, trace_id = self._helper("test-django-trace-id", None) - self.assertEqual(trace_id, django_mock.return_value) + django_expected = ('django-id', {'request_url':'https://www.djangoproject.com/'}) + flask_expected = (None, None) + django_mock, flask_mock, output = self._helper(django_expected, flask_expected) + self.assertEqual(output, django_expected) django_mock.assert_called_once_with() flask_mock.assert_not_called() def test_from_flask(self): - django_mock, flask_mock, trace_id = self._helper(None, "test-flask-trace-id") - self.assertEqual(trace_id, flask_mock.return_value) + django_expected = (None, None) + flask_expected = ('flask-id', {'request_url':'https://flask.palletsprojects.com/en/1.1.x/'}) + + django_mock, flask_mock, output = self._helper(django_expected, flask_expected) + self.assertEqual(output, flask_expected) django_mock.assert_called_once_with() flask_mock.assert_called_once_with() def test_from_django_and_flask(self): - django_mock, flask_mock, trace_id = self._helper( - "test-django-trace-id", "test-flask-trace-id" - ) + django_expected = ('django-id', {'request_url':'https://www.djangoproject.com/'}) + flask_expected = ('flask-id', {'request_url':'https://flask.palletsprojects.com/en/1.1.x/'}) + + django_mock, flask_mock, output = self._helper(django_expected, flask_expected) + # Django wins. - self.assertEqual(trace_id, django_mock.return_value) + self.assertEqual(output, django_expected) django_mock.assert_called_once_with() flask_mock.assert_not_called() def test_missing(self): - django_mock, flask_mock, trace_id = self._helper(None, None) - self.assertIsNone(trace_id) + flask_expected = (None, None) + django_expected = (None, None) + django_mock, flask_mock, output = self._helper(django_expected, flask_expected) + self.assertEqual(output, (None, None)) django_mock.assert_called_once_with() flask_mock.assert_called_once_with() From 1459f7e46bbe69f04de6c8fbd2f0f45f32a067f6 Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Wed, 9 Dec 2020 14:49:26 -0800 Subject: [PATCH 06/35] changed order of outputs --- google/cloud/logging_v2/handlers/_helpers.py | 8 +++---- .../cloud/logging_v2/handlers/app_engine.py | 4 ++-- tests/unit/handlers/test__helpers.py | 22 +++++++++++-------- 3 files changed, 19 insertions(+), 15 deletions(-) diff --git a/google/cloud/logging_v2/handlers/_helpers.py b/google/cloud/logging_v2/handlers/_helpers.py index 078cbdf3..7ee74b7a 100644 --- a/google/cloud/logging_v2/handlers/_helpers.py +++ b/google/cloud/logging_v2/handlers/_helpers.py @@ -76,7 +76,7 @@ def get_request_data_from_flask(): if header: trace_id = header.split("/", 1)[0] - return trace_id, http_request + return http_request, trace_id def get_request_data_from_django(): """Get trace_id and http_request data from django request headers. @@ -106,7 +106,7 @@ def get_request_data_from_django(): if header: trace_id = header.split("/", 1)[0] - return trace_id, http_request + return http_request, trace_id @@ -124,8 +124,8 @@ def get_request_data(): ) for checker in checkers: - trace_id, http_request = checker() + http_request, trace_id = checker() if http_request is not None: - return trace_id, http_request + return http_request, trace_id return None, None diff --git a/google/cloud/logging_v2/handlers/app_engine.py b/google/cloud/logging_v2/handlers/app_engine.py index 68a58810..5ac4223f 100644 --- a/google/cloud/logging_v2/handlers/app_engine.py +++ b/google/cloud/logging_v2/handlers/app_engine.py @@ -96,7 +96,7 @@ def get_gae_labels(self): """ gae_labels = {} - trace_id, _ = get_request_data() + _, trace_id = get_request_data() if trace_id is not None: gae_labels[_TRACE_ID_LABEL] = trace_id @@ -114,7 +114,7 @@ def emit(self, record): """ message = super(AppEngineHandler, self).format(record) gae_labels = self.get_gae_labels() - trace_id, http_request = get_request_data() + http_request, trace_id = get_request_data() if trace_id is not None: trace_id = f"projects/{self.project_id}/{trace_id}" self.transport.send( diff --git a/tests/unit/handlers/test__helpers.py b/tests/unit/handlers/test__helpers.py index 7aad4cf0..fd5187ea 100644 --- a/tests/unit/handlers/test__helpers.py +++ b/tests/unit/handlers/test__helpers.py @@ -16,6 +16,10 @@ import mock +_FLASK_TRACE_ID = 'flask-id' +_FLASK_HTTP_REQUEST = {'request_url': "https://flask.palletsprojects.com/en/1.1.x/"} +_DJANGO_TRACE_ID = 'django-id' +_DJANGO_HTTP_REQUEST = {'request_url': "https://www.djangoproject.com/"} class Test_get_trace_id_from_flask(unittest.TestCase): @staticmethod @@ -39,13 +43,13 @@ def index(): def test_no_context_header(self): app = self.create_app() with app.test_request_context(path="/", headers={}): - trace_id, http_data = self._call_fut() + http_request, trace_id = self._call_fut() self.assertIsNone(trace_id) def test_valid_context_header(self): flask_trace_header = "X_CLOUD_TRACE_CONTEXT" - expected_trace_id = "testtraceidflask" + expected_trace_id = _FLASK_TRACE_ID flask_trace_id = expected_trace_id + "/testspanid" app = self.create_app() @@ -54,7 +58,7 @@ def test_valid_context_header(self): ) with context: - trace_id, http_data = self._call_fut() + http_data, trace_id = self._call_fut() self.assertEqual(trace_id, expected_trace_id) @@ -89,7 +93,7 @@ def test_no_context_header(self): middleware = request.RequestMiddleware(None) middleware.process_request(django_request) - trace_id, http_request = self._call_fut() + http_request, trace_id = self._call_fut() self.assertIsNone(trace_id) def test_valid_context_header(self): @@ -106,7 +110,7 @@ def test_valid_context_header(self): middleware = request.RequestMiddleware(None) middleware.process_request(django_request) - trace_id, http_request = self._call_fut() + http_request, trace_id = self._call_fut() self.assertEqual(trace_id, expected_trace_id) @@ -135,7 +139,7 @@ def _helper(self, django_return, flask_return): return django_mock, flask_mock, result def test_from_django(self): - django_expected = ('django-id', {'request_url':'https://www.djangoproject.com/'}) + django_expected = (_DJANGO_HTTP_REQUEST, _DJANGO_TRACE_ID) flask_expected = (None, None) django_mock, flask_mock, output = self._helper(django_expected, flask_expected) self.assertEqual(output, django_expected) @@ -145,7 +149,7 @@ def test_from_django(self): def test_from_flask(self): django_expected = (None, None) - flask_expected = ('flask-id', {'request_url':'https://flask.palletsprojects.com/en/1.1.x/'}) + flask_expected = (_FLASK_HTTP_REQUEST, _FLASK_TRACE_ID) django_mock, flask_mock, output = self._helper(django_expected, flask_expected) self.assertEqual(output, flask_expected) @@ -154,8 +158,8 @@ def test_from_flask(self): flask_mock.assert_called_once_with() def test_from_django_and_flask(self): - django_expected = ('django-id', {'request_url':'https://www.djangoproject.com/'}) - flask_expected = ('flask-id', {'request_url':'https://flask.palletsprojects.com/en/1.1.x/'}) + django_expected = (_DJANGO_HTTP_REQUEST, _DJANGO_TRACE_ID) + flask_expected = (_FLASK_HTTP_REQUEST, _FLASK_TRACE_ID) django_mock, flask_mock, output = self._helper(django_expected, flask_expected) From 96cf896882ed407eb53315004b622afe8568764b Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Wed, 9 Dec 2020 15:05:03 -0800 Subject: [PATCH 07/35] improved test_django --- tests/unit/handlers/test__helpers.py | 41 ++++++++++++++++++++++++++-- 1 file changed, 38 insertions(+), 3 deletions(-) diff --git a/tests/unit/handlers/test__helpers.py b/tests/unit/handlers/test__helpers.py index fd5187ea..70aaddd2 100644 --- a/tests/unit/handlers/test__helpers.py +++ b/tests/unit/handlers/test__helpers.py @@ -20,6 +20,8 @@ _FLASK_HTTP_REQUEST = {'request_url': "https://flask.palletsprojects.com/en/1.1.x/"} _DJANGO_TRACE_ID = 'django-id' _DJANGO_HTTP_REQUEST = {'request_url': "https://www.djangoproject.com/"} +_HTTP_REQUEST_FIELDS = ['request_method', 'request_url', 'request_size', + 'user_agent', 'remote_ip', 'referer'] class Test_get_trace_id_from_flask(unittest.TestCase): @staticmethod @@ -63,7 +65,7 @@ def test_valid_context_header(self): self.assertEqual(trace_id, expected_trace_id) -class Test_get_trace_id_from_django(unittest.TestCase): +class Test_get_request_data_from_django(unittest.TestCase): @staticmethod def _call_fut(): from google.cloud.logging_v2.handlers import _helpers @@ -94,6 +96,11 @@ def test_no_context_header(self): middleware = request.RequestMiddleware(None) middleware.process_request(django_request) http_request, trace_id = self._call_fut() + self.assertEqual(http_request['request_method'], "GET") + self.assertEqual(http_request['request_url'], "/") + self.assertEqual(set(http_request.keys()), set(_HTTP_REQUEST_FIELDS)) + for field in _HTTP_REQUEST_FIELDS: + self.assertTrue(field in http_request) self.assertIsNone(trace_id) def test_valid_context_header(self): @@ -113,9 +120,14 @@ def test_valid_context_header(self): http_request, trace_id = self._call_fut() self.assertEqual(trace_id, expected_trace_id) + self.assertEqual(http_request['request_method'], "GET") + self.assertEqual(http_request['request_url'], "/") + self.assertEqual(set(http_request.keys()), set(_HTTP_REQUEST_FIELDS)) + for field in _HTTP_REQUEST_FIELDS: + self.assertTrue(field in http_request) -class Test_get_trace_id(unittest.TestCase): +class Test_get_request_data(unittest.TestCase): @staticmethod def _call_fut(): from google.cloud.logging_v2.handlers import _helpers @@ -169,7 +181,30 @@ def test_from_django_and_flask(self): django_mock.assert_called_once_with() flask_mock.assert_not_called() - def test_missing(self): + def test_missing_http_request(self): + flask_expected = (None, _FLASK_TRACE_ID) + django_expected = (None, _DJANGO_TRACE_ID) + django_mock, flask_mock, output = self._helper(django_expected, flask_expected) + + # function only returns trace if http_request data is present + self.assertEqual(output, (None, None)) + + django_mock.assert_called_once_with() + flask_mock.assert_called_once_with() + + def test_missing_trace_id(self): + flask_expected = (_FLASK_HTTP_REQUEST, None) + django_expected = (None, _DJANGO_TRACE_ID) + django_mock, flask_mock, output = self._helper(django_expected, flask_expected) + + # trace_id is optional + self.assertEqual(output, flask_expected) + + django_mock.assert_called_once_with() + flask_mock.assert_called_once_with() + + + def test_missing_both(self): flask_expected = (None, None) django_expected = (None, None) django_mock, flask_mock, output = self._helper(django_expected, flask_expected) From a8d54c8e5abf491ec1bdd929236d913526553eec Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Wed, 9 Dec 2020 15:21:30 -0800 Subject: [PATCH 08/35] added django http tests --- google/cloud/logging_v2/handlers/_helpers.py | 2 +- tests/unit/handlers/test__helpers.py | 25 ++++++++++++++++++-- 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/google/cloud/logging_v2/handlers/_helpers.py b/google/cloud/logging_v2/handlers/_helpers.py index 7ee74b7a..e6376cfd 100644 --- a/google/cloud/logging_v2/handlers/_helpers.py +++ b/google/cloud/logging_v2/handlers/_helpers.py @@ -93,7 +93,7 @@ def get_request_data_from_django(): # build http_request http_request = { 'request_method': request.method, - 'request_url': request.get_full_path(), + 'request_url': request.build_absolute_uri(), 'request_size': request.META.get(_DJANGO_LENGTH_HEADER), 'user_agent': request.META.get(_DJANGO_USERAGENT_HEADER), 'remote_ip': request.META.get(_DJANGO_REMOTE_ADDR_HEADER), diff --git a/tests/unit/handlers/test__helpers.py b/tests/unit/handlers/test__helpers.py index 70aaddd2..7a2d584a 100644 --- a/tests/unit/handlers/test__helpers.py +++ b/tests/unit/handlers/test__helpers.py @@ -97,7 +97,6 @@ def test_no_context_header(self): middleware.process_request(django_request) http_request, trace_id = self._call_fut() self.assertEqual(http_request['request_method'], "GET") - self.assertEqual(http_request['request_url'], "/") self.assertEqual(set(http_request.keys()), set(_HTTP_REQUEST_FIELDS)) for field in _HTTP_REQUEST_FIELDS: self.assertTrue(field in http_request) @@ -121,11 +120,33 @@ def test_valid_context_header(self): self.assertEqual(trace_id, expected_trace_id) self.assertEqual(http_request['request_method'], "GET") - self.assertEqual(http_request['request_url'], "/") self.assertEqual(set(http_request.keys()), set(_HTTP_REQUEST_FIELDS)) for field in _HTTP_REQUEST_FIELDS: self.assertTrue(field in http_request) + def test_http_data(self): + from django.test import RequestFactory + from google.cloud.logging_v2.handlers.middleware import request + expected_path = 'http://testserver/123' + expected_agent = 'Mozilla/5.0' + expected_referrer = "self" + body_content = 'test' + django_request = RequestFactory().put( + expected_path, + data=body_content, + HTTP_USER_AGENT=expected_agent, + HTTP_REFERER=expected_referrer) + + middleware = request.RequestMiddleware(None) + middleware.process_request(django_request) + http_request, trace_id = self._call_fut() + self.assertEqual(http_request['request_method'], "PUT") + self.assertEqual(http_request['request_url'], expected_path) + self.assertEqual(http_request['user_agent'], expected_agent) + self.assertEqual(http_request['referer'], expected_referrer) + self.assertEqual(http_request['remote_ip'], '127.0.0.1') + self.assertEqual(http_request['request_size'], str(len(body_content))) + self.assertEqual(set(http_request.keys()), set(_HTTP_REQUEST_FIELDS)) class Test_get_request_data(unittest.TestCase): @staticmethod From 841e68a9fb92ae21e461d7cadcda34b23579f758 Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Wed, 9 Dec 2020 15:35:57 -0800 Subject: [PATCH 09/35] added tests to flask --- google/cloud/logging_v2/handlers/_helpers.py | 4 +-- tests/unit/handlers/test__helpers.py | 37 ++++++++++++++++++-- 2 files changed, 37 insertions(+), 4 deletions(-) diff --git a/google/cloud/logging_v2/handlers/_helpers.py b/google/cloud/logging_v2/handlers/_helpers.py index e6376cfd..d3b668bd 100644 --- a/google/cloud/logging_v2/handlers/_helpers.py +++ b/google/cloud/logging_v2/handlers/_helpers.py @@ -63,8 +63,8 @@ def get_request_data_from_flask(): # build http_request http_request = { 'request_method': flask.request.method, - 'request_url': flask.request.host_url, - 'request_size': flask.request.content_length, + 'request_url': flask.request.url, + 'request_size': str(flask.request.content_length), 'user_agent': flask.request.user_agent.string, 'remote_ip': flask.request.remote_addr, 'referer': flask.request.referrer, diff --git a/tests/unit/handlers/test__helpers.py b/tests/unit/handlers/test__helpers.py index 7a2d584a..5ca70224 100644 --- a/tests/unit/handlers/test__helpers.py +++ b/tests/unit/handlers/test__helpers.py @@ -23,7 +23,7 @@ _HTTP_REQUEST_FIELDS = ['request_method', 'request_url', 'request_size', 'user_agent', 'remote_ip', 'referer'] -class Test_get_trace_id_from_flask(unittest.TestCase): +class Test_get_request_data_from_flask(unittest.TestCase): @staticmethod def _call_fut(): from google.cloud.logging_v2.handlers import _helpers @@ -48,6 +48,10 @@ def test_no_context_header(self): http_request, trace_id = self._call_fut() self.assertIsNone(trace_id) + self.assertEqual(http_request['request_method'], "GET") + self.assertEqual(set(http_request.keys()), set(_HTTP_REQUEST_FIELDS)) + for field in _HTTP_REQUEST_FIELDS: + self.assertTrue(field in http_request) def test_valid_context_header(self): flask_trace_header = "X_CLOUD_TRACE_CONTEXT" @@ -60,10 +64,39 @@ def test_valid_context_header(self): ) with context: - http_data, trace_id = self._call_fut() + http_request, trace_id = self._call_fut() self.assertEqual(trace_id, expected_trace_id) + self.assertEqual(http_request['request_method'], "GET") + self.assertEqual(set(http_request.keys()), set(_HTTP_REQUEST_FIELDS)) + for field in _HTTP_REQUEST_FIELDS: + self.assertTrue(field in http_request) + + def test_http_data(self): + expected_path = 'http://testserver/123' + expected_agent = 'Mozilla/5.0' + expected_referrer = "self" + expected_ip = "10.1.2.3" + body_content = 'test' + headers = {'User-Agent': expected_agent, + 'Referer': expected_referrer, + 'HTTP_X_FORWARDED_FOR': '0.0.0.0'} + + app = self.create_app() + with app.test_client() as c: + c.put(path=expected_path, + data=body_content, + environ_base={'REMOTE_ADDR': expected_ip}, + headers=headers) + http_request, trace_id = self._call_fut() + self.assertEqual(http_request['request_method'], "PUT") + self.assertEqual(http_request['request_url'], expected_path) + self.assertEqual(http_request['user_agent'], expected_agent) + self.assertEqual(http_request['referer'], expected_referrer) + self.assertEqual(http_request['remote_ip'], expected_ip) + self.assertEqual(http_request['request_size'], str(len(body_content))) + self.assertEqual(set(http_request.keys()), set(_HTTP_REQUEST_FIELDS)) class Test_get_request_data_from_django(unittest.TestCase): @staticmethod From a3bd27c1836c12ceaa2132633172d073a70a1670 Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Wed, 9 Dec 2020 15:54:39 -0800 Subject: [PATCH 10/35] fixed appengine tests --- tests/unit/handlers/test_app_engine.py | 53 +++++++++++++++----------- 1 file changed, 31 insertions(+), 22 deletions(-) diff --git a/tests/unit/handlers/test_app_engine.py b/tests/unit/handlers/test_app_engine.py index ea16e3c8..6546c6b2 100644 --- a/tests/unit/handlers/test_app_engine.py +++ b/tests/unit/handlers/test_app_engine.py @@ -87,36 +87,45 @@ def test_constructor_w_gae_flex_env(self): self.assertIs(handler.stream, stream) def test_emit(self): - client = mock.Mock(project=self.PROJECT, spec=["project"]) - handler = self._make_one(client, transport=_Transport) - gae_resource = handler.get_gae_resource() - gae_labels = handler.get_gae_labels() - trace = None - logname = "app" - message = "hello world" - record = logging.LogRecord(logname, logging, None, None, message, None, None) - handler.emit(record) - - self.assertIs(handler.transport.client, client) - self.assertEqual(handler.transport.name, logname) - self.assertEqual( - handler.transport.send_called_with, - (record, message, gae_resource, gae_labels, trace), + expected_http_request = {'request_url':'test'} + trace_id = 'trace-test' + expected_trace_id = f'projects/{self.PROJECT}/{trace_id}' + get_request_patch = mock.patch( + "google.cloud.logging_v2.handlers.app_engine.get_request_data", + return_value=({'request_url':'test'}, 'trace-test'), ) + with get_request_patch as mock_get_request: + + client = mock.Mock(project=self.PROJECT, spec=["project"]) + handler = self._make_one(client, transport=_Transport) + gae_resource = handler.get_gae_resource() + gae_labels = handler.get_gae_labels() + logname = "app" + message = "hello world" + record = logging.LogRecord(logname, logging, None, None, message, None, None) + handler.project_id = self.PROJECT + handler.emit(record) + + self.assertIs(handler.transport.client, client) + self.assertEqual(handler.transport.name, logname) + self.assertEqual( + handler.transport.send_called_with, + (record, message, gae_resource, gae_labels, expected_trace_id, expected_http_request) + ) def _get_gae_labels_helper(self, trace_id): - get_trace_patch = mock.patch( - "google.cloud.logging_v2.handlers.app_engine.get_trace_id", - return_value=trace_id, + get_request_patch = mock.patch( + "google.cloud.logging_v2.handlers.app_engine.get_request_data", + return_value=(None, trace_id), ) client = mock.Mock(project=self.PROJECT, spec=["project"]) # The handler actually calls ``get_gae_labels()``. - with get_trace_patch as mock_get_trace: + with get_request_patch as mock_get_request: handler = self._make_one(client, transport=_Transport) gae_labels = handler.get_gae_labels() - self.assertEqual(mock_get_trace.mock_calls, [mock.call()]) + self.assertEqual(mock_get_request.mock_calls, [mock.call()]) return gae_labels @@ -138,5 +147,5 @@ def __init__(self, client, name): self.client = client self.name = name - def send(self, record, message, resource, labels, trace): - self.send_called_with = (record, message, resource, labels, trace) + def send(self, record, message, resource, labels, trace, http_request): + self.send_called_with = (record, message, resource, labels, trace, http_request) From e8733f4578d9e81a266c519a20d28d93fbb05865 Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Wed, 9 Dec 2020 16:11:04 -0800 Subject: [PATCH 11/35] fixed transport tests --- tests/unit/handlers/transports/test_background_thread.py | 5 +++++ tests/unit/handlers/transports/test_sync.py | 3 +++ 2 files changed, 8 insertions(+) diff --git a/tests/unit/handlers/transports/test_background_thread.py b/tests/unit/handlers/transports/test_background_thread.py index e9626a75..2a25cae0 100644 --- a/tests/unit/handlers/transports/test_background_thread.py +++ b/tests/unit/handlers/transports/test_background_thread.py @@ -70,6 +70,7 @@ def test_send(self): labels=None, trace=None, span_id=None, + http_request=None ) def test_trace_send(self): @@ -97,6 +98,7 @@ def test_trace_send(self): labels=None, trace=trace, span_id=None, + http_request=None ) def test_span_send(self): @@ -124,6 +126,7 @@ def test_span_send(self): labels=None, trace=None, span_id=span_id, + http_request=None ) def test_flush(self): @@ -301,6 +304,7 @@ def test_enqueue_defaults(self): self.assertIsNone(entry["labels"]) self.assertIsNone(entry["trace"]) self.assertIsNone(entry["span_id"]) + self.assertIsNone(entry["http_request"]) self.assertIsInstance(entry["timestamp"], datetime.datetime) def test_enqueue_explicit(self): @@ -503,6 +507,7 @@ def log_struct( trace=None, span_id=None, timestamp=None, + http_request=None, ): from google.cloud.logging_v2.logger import _GLOBAL_RESOURCE diff --git a/tests/unit/handlers/transports/test_sync.py b/tests/unit/handlers/transports/test_sync.py index 0ee6db22..9f064275 100644 --- a/tests/unit/handlers/transports/test_sync.py +++ b/tests/unit/handlers/transports/test_sync.py @@ -58,6 +58,7 @@ def test_send(self): None, None, None, + None, ) self.assertEqual(transport.logger.log_struct_called_with, EXPECTED_SENT) @@ -76,6 +77,7 @@ def log_struct( labels=None, trace=None, span_id=None, + http_request=None, ): self.log_struct_called_with = ( message, @@ -84,6 +86,7 @@ def log_struct( labels, trace, span_id, + http_request, ) From 2f7304999b3b8c7ef087b2d1239539f318b69f69 Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Wed, 9 Dec 2020 16:12:19 -0800 Subject: [PATCH 12/35] ran blacken --- google/cloud/logging_v2/handlers/_helpers.py | 26 +++--- .../cloud/logging_v2/handlers/app_engine.py | 8 +- .../handlers/transports/background_thread.py | 23 +++-- .../logging_v2/handlers/transports/base.py | 11 ++- .../logging_v2/handlers/transports/sync.py | 11 ++- tests/unit/handlers/test__helpers.py | 88 +++++++++++-------- tests/unit/handlers/test_app_engine.py | 21 +++-- .../transports/test_background_thread.py | 6 +- 8 files changed, 124 insertions(+), 70 deletions(-) diff --git a/google/cloud/logging_v2/handlers/_helpers.py b/google/cloud/logging_v2/handlers/_helpers.py index d3b668bd..108a21ab 100644 --- a/google/cloud/logging_v2/handlers/_helpers.py +++ b/google/cloud/logging_v2/handlers/_helpers.py @@ -62,12 +62,12 @@ def get_request_data_from_flask(): # build http_request http_request = { - 'request_method': flask.request.method, - 'request_url': flask.request.url, - 'request_size': str(flask.request.content_length), - 'user_agent': flask.request.user_agent.string, - 'remote_ip': flask.request.remote_addr, - 'referer': flask.request.referrer, + "request_method": flask.request.method, + "request_url": flask.request.url, + "request_size": str(flask.request.content_length), + "user_agent": flask.request.user_agent.string, + "remote_ip": flask.request.remote_addr, + "referer": flask.request.referrer, } # find trace id @@ -78,6 +78,7 @@ def get_request_data_from_flask(): return http_request, trace_id + def get_request_data_from_django(): """Get trace_id and http_request data from django request headers. @@ -92,12 +93,12 @@ def get_request_data_from_django(): # build http_request http_request = { - 'request_method': request.method, - 'request_url': request.build_absolute_uri(), - 'request_size': request.META.get(_DJANGO_LENGTH_HEADER), - 'user_agent': request.META.get(_DJANGO_USERAGENT_HEADER), - 'remote_ip': request.META.get(_DJANGO_REMOTE_ADDR_HEADER), - 'referer': request.META.get(_DJANGO_REFERER_HEADER), + "request_method": request.method, + "request_url": request.build_absolute_uri(), + "request_size": request.META.get(_DJANGO_LENGTH_HEADER), + "user_agent": request.META.get(_DJANGO_USERAGENT_HEADER), + "remote_ip": request.META.get(_DJANGO_REMOTE_ADDR_HEADER), + "referer": request.META.get(_DJANGO_REFERER_HEADER), } # find trace id @@ -109,7 +110,6 @@ def get_request_data_from_django(): return http_request, trace_id - def get_request_data(): """Helper to get trace_id and http_request data from supported web frameworks. diff --git a/google/cloud/logging_v2/handlers/app_engine.py b/google/cloud/logging_v2/handlers/app_engine.py index 5ac4223f..a74973d2 100644 --- a/google/cloud/logging_v2/handlers/app_engine.py +++ b/google/cloud/logging_v2/handlers/app_engine.py @@ -118,6 +118,10 @@ def emit(self, record): if trace_id is not None: trace_id = f"projects/{self.project_id}/{trace_id}" self.transport.send( - record, message, resource=self.resource, labels=gae_labels, - trace=trace_id, http_request=http_request, + record, + message, + resource=self.resource, + labels=gae_labels, + trace=trace_id, + http_request=http_request, ) diff --git a/google/cloud/logging_v2/handlers/transports/background_thread.py b/google/cloud/logging_v2/handlers/transports/background_thread.py index 86e5578d..29efbf13 100644 --- a/google/cloud/logging_v2/handlers/transports/background_thread.py +++ b/google/cloud/logging_v2/handlers/transports/background_thread.py @@ -223,8 +223,15 @@ def _main_thread_terminated(self): ) def enqueue( - self, record, message, *, resource=None, labels=None, trace=None, - span_id=None, http_request=None + self, + record, + message, + *, + resource=None, + labels=None, + trace=None, + span_id=None, + http_request=None, ): """Queues a log entry to be written by the background thread. @@ -249,7 +256,7 @@ def enqueue( "trace": trace, "span_id": span_id, "timestamp": datetime.datetime.utcfromtimestamp(record.created), - "http_request": http_request + "http_request": http_request, } self._queue.put_nowait(queue_entry) @@ -296,8 +303,14 @@ def __init__( self.worker.start() def send( - self, record, message, resource=None, labels=None, trace=None, - span_id=None, http_request=None + self, + record, + message, + resource=None, + labels=None, + trace=None, + span_id=None, + http_request=None, ): """Overrides Transport.send(). diff --git a/google/cloud/logging_v2/handlers/transports/base.py b/google/cloud/logging_v2/handlers/transports/base.py index 9904b5ae..1572108b 100644 --- a/google/cloud/logging_v2/handlers/transports/base.py +++ b/google/cloud/logging_v2/handlers/transports/base.py @@ -23,8 +23,15 @@ class Transport(object): """ def send( - self, record, message, *, resource=None, labels=None, trace=None, - span_id=None, http_request=None + self, + record, + message, + *, + resource=None, + labels=None, + trace=None, + span_id=None, + http_request=None ): """Transport send to be implemented by subclasses. diff --git a/google/cloud/logging_v2/handlers/transports/sync.py b/google/cloud/logging_v2/handlers/transports/sync.py index ae91c68c..761b60fe 100644 --- a/google/cloud/logging_v2/handlers/transports/sync.py +++ b/google/cloud/logging_v2/handlers/transports/sync.py @@ -31,8 +31,15 @@ def __init__(self, client, name): self.logger = client.logger(name) def send( - self, record, message, *, resource=None, labels=None, trace=None, - span_id=None, http_request=None, + self, + record, + message, + *, + resource=None, + labels=None, + trace=None, + span_id=None, + http_request=None, ): """Overrides transport.send(). diff --git a/tests/unit/handlers/test__helpers.py b/tests/unit/handlers/test__helpers.py index 5ca70224..cc6de761 100644 --- a/tests/unit/handlers/test__helpers.py +++ b/tests/unit/handlers/test__helpers.py @@ -16,12 +16,19 @@ import mock -_FLASK_TRACE_ID = 'flask-id' -_FLASK_HTTP_REQUEST = {'request_url': "https://flask.palletsprojects.com/en/1.1.x/"} -_DJANGO_TRACE_ID = 'django-id' -_DJANGO_HTTP_REQUEST = {'request_url': "https://www.djangoproject.com/"} -_HTTP_REQUEST_FIELDS = ['request_method', 'request_url', 'request_size', - 'user_agent', 'remote_ip', 'referer'] +_FLASK_TRACE_ID = "flask-id" +_FLASK_HTTP_REQUEST = {"request_url": "https://flask.palletsprojects.com/en/1.1.x/"} +_DJANGO_TRACE_ID = "django-id" +_DJANGO_HTTP_REQUEST = {"request_url": "https://www.djangoproject.com/"} +_HTTP_REQUEST_FIELDS = [ + "request_method", + "request_url", + "request_size", + "user_agent", + "remote_ip", + "referer", +] + class Test_get_request_data_from_flask(unittest.TestCase): @staticmethod @@ -48,7 +55,7 @@ def test_no_context_header(self): http_request, trace_id = self._call_fut() self.assertIsNone(trace_id) - self.assertEqual(http_request['request_method'], "GET") + self.assertEqual(http_request["request_method"], "GET") self.assertEqual(set(http_request.keys()), set(_HTTP_REQUEST_FIELDS)) for field in _HTTP_REQUEST_FIELDS: self.assertTrue(field in http_request) @@ -67,37 +74,42 @@ def test_valid_context_header(self): http_request, trace_id = self._call_fut() self.assertEqual(trace_id, expected_trace_id) - self.assertEqual(http_request['request_method'], "GET") + self.assertEqual(http_request["request_method"], "GET") self.assertEqual(set(http_request.keys()), set(_HTTP_REQUEST_FIELDS)) for field in _HTTP_REQUEST_FIELDS: self.assertTrue(field in http_request) def test_http_data(self): - expected_path = 'http://testserver/123' - expected_agent = 'Mozilla/5.0' + expected_path = "http://testserver/123" + expected_agent = "Mozilla/5.0" expected_referrer = "self" expected_ip = "10.1.2.3" - body_content = 'test' - headers = {'User-Agent': expected_agent, - 'Referer': expected_referrer, - 'HTTP_X_FORWARDED_FOR': '0.0.0.0'} + body_content = "test" + headers = { + "User-Agent": expected_agent, + "Referer": expected_referrer, + "HTTP_X_FORWARDED_FOR": "0.0.0.0", + } app = self.create_app() with app.test_client() as c: - c.put(path=expected_path, - data=body_content, - environ_base={'REMOTE_ADDR': expected_ip}, - headers=headers) + c.put( + path=expected_path, + data=body_content, + environ_base={"REMOTE_ADDR": expected_ip}, + headers=headers, + ) http_request, trace_id = self._call_fut() - self.assertEqual(http_request['request_method'], "PUT") - self.assertEqual(http_request['request_url'], expected_path) - self.assertEqual(http_request['user_agent'], expected_agent) - self.assertEqual(http_request['referer'], expected_referrer) - self.assertEqual(http_request['remote_ip'], expected_ip) - self.assertEqual(http_request['request_size'], str(len(body_content))) + self.assertEqual(http_request["request_method"], "PUT") + self.assertEqual(http_request["request_url"], expected_path) + self.assertEqual(http_request["user_agent"], expected_agent) + self.assertEqual(http_request["referer"], expected_referrer) + self.assertEqual(http_request["remote_ip"], expected_ip) + self.assertEqual(http_request["request_size"], str(len(body_content))) self.assertEqual(set(http_request.keys()), set(_HTTP_REQUEST_FIELDS)) + class Test_get_request_data_from_django(unittest.TestCase): @staticmethod def _call_fut(): @@ -129,7 +141,7 @@ def test_no_context_header(self): middleware = request.RequestMiddleware(None) middleware.process_request(django_request) http_request, trace_id = self._call_fut() - self.assertEqual(http_request['request_method'], "GET") + self.assertEqual(http_request["request_method"], "GET") self.assertEqual(set(http_request.keys()), set(_HTTP_REQUEST_FIELDS)) for field in _HTTP_REQUEST_FIELDS: self.assertTrue(field in http_request) @@ -152,7 +164,7 @@ def test_valid_context_header(self): http_request, trace_id = self._call_fut() self.assertEqual(trace_id, expected_trace_id) - self.assertEqual(http_request['request_method'], "GET") + self.assertEqual(http_request["request_method"], "GET") self.assertEqual(set(http_request.keys()), set(_HTTP_REQUEST_FIELDS)) for field in _HTTP_REQUEST_FIELDS: self.assertTrue(field in http_request) @@ -160,27 +172,30 @@ def test_valid_context_header(self): def test_http_data(self): from django.test import RequestFactory from google.cloud.logging_v2.handlers.middleware import request - expected_path = 'http://testserver/123' - expected_agent = 'Mozilla/5.0' + + expected_path = "http://testserver/123" + expected_agent = "Mozilla/5.0" expected_referrer = "self" - body_content = 'test' + body_content = "test" django_request = RequestFactory().put( expected_path, data=body_content, HTTP_USER_AGENT=expected_agent, - HTTP_REFERER=expected_referrer) + HTTP_REFERER=expected_referrer, + ) middleware = request.RequestMiddleware(None) middleware.process_request(django_request) http_request, trace_id = self._call_fut() - self.assertEqual(http_request['request_method'], "PUT") - self.assertEqual(http_request['request_url'], expected_path) - self.assertEqual(http_request['user_agent'], expected_agent) - self.assertEqual(http_request['referer'], expected_referrer) - self.assertEqual(http_request['remote_ip'], '127.0.0.1') - self.assertEqual(http_request['request_size'], str(len(body_content))) + self.assertEqual(http_request["request_method"], "PUT") + self.assertEqual(http_request["request_url"], expected_path) + self.assertEqual(http_request["user_agent"], expected_agent) + self.assertEqual(http_request["referer"], expected_referrer) + self.assertEqual(http_request["remote_ip"], "127.0.0.1") + self.assertEqual(http_request["request_size"], str(len(body_content))) self.assertEqual(set(http_request.keys()), set(_HTTP_REQUEST_FIELDS)) + class Test_get_request_data(unittest.TestCase): @staticmethod def _call_fut(): @@ -257,7 +272,6 @@ def test_missing_trace_id(self): django_mock.assert_called_once_with() flask_mock.assert_called_once_with() - def test_missing_both(self): flask_expected = (None, None) django_expected = (None, None) diff --git a/tests/unit/handlers/test_app_engine.py b/tests/unit/handlers/test_app_engine.py index 6546c6b2..16f16ce6 100644 --- a/tests/unit/handlers/test_app_engine.py +++ b/tests/unit/handlers/test_app_engine.py @@ -87,12 +87,12 @@ def test_constructor_w_gae_flex_env(self): self.assertIs(handler.stream, stream) def test_emit(self): - expected_http_request = {'request_url':'test'} - trace_id = 'trace-test' - expected_trace_id = f'projects/{self.PROJECT}/{trace_id}' + expected_http_request = {"request_url": "test"} + trace_id = "trace-test" + expected_trace_id = f"projects/{self.PROJECT}/{trace_id}" get_request_patch = mock.patch( "google.cloud.logging_v2.handlers.app_engine.get_request_data", - return_value=({'request_url':'test'}, 'trace-test'), + return_value=({"request_url": "test"}, "trace-test"), ) with get_request_patch as mock_get_request: @@ -102,7 +102,9 @@ def test_emit(self): gae_labels = handler.get_gae_labels() logname = "app" message = "hello world" - record = logging.LogRecord(logname, logging, None, None, message, None, None) + record = logging.LogRecord( + logname, logging, None, None, message, None, None + ) handler.project_id = self.PROJECT handler.emit(record) @@ -110,7 +112,14 @@ def test_emit(self): self.assertEqual(handler.transport.name, logname) self.assertEqual( handler.transport.send_called_with, - (record, message, gae_resource, gae_labels, expected_trace_id, expected_http_request) + ( + record, + message, + gae_resource, + gae_labels, + expected_trace_id, + expected_http_request, + ), ) def _get_gae_labels_helper(self, trace_id): diff --git a/tests/unit/handlers/transports/test_background_thread.py b/tests/unit/handlers/transports/test_background_thread.py index 2a25cae0..e4feb83c 100644 --- a/tests/unit/handlers/transports/test_background_thread.py +++ b/tests/unit/handlers/transports/test_background_thread.py @@ -70,7 +70,7 @@ def test_send(self): labels=None, trace=None, span_id=None, - http_request=None + http_request=None, ) def test_trace_send(self): @@ -98,7 +98,7 @@ def test_trace_send(self): labels=None, trace=trace, span_id=None, - http_request=None + http_request=None, ) def test_span_send(self): @@ -126,7 +126,7 @@ def test_span_send(self): labels=None, trace=None, span_id=span_id, - http_request=None + http_request=None, ) def test_flush(self): From c28d3fd6183051468105d57fbe7f67d91e13bda7 Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Thu, 10 Dec 2020 10:33:49 -0800 Subject: [PATCH 13/35] allow user overrides --- google/cloud/logging_v2/handlers/app_engine.py | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/google/cloud/logging_v2/handlers/app_engine.py b/google/cloud/logging_v2/handlers/app_engine.py index a74973d2..31e0e6ec 100644 --- a/google/cloud/logging_v2/handlers/app_engine.py +++ b/google/cloud/logging_v2/handlers/app_engine.py @@ -113,15 +113,23 @@ def emit(self, record): record (logging.LogRecord): The record to be logged. """ message = super(AppEngineHandler, self).format(record) + inferred_http, inferred_trace_id = get_request_data() + # allow user overrides + trace_id = record.__dict__.get('trace', inferred_trace_id) + span_id = record.__dict__.get('span_id', None) + http_request = record.__dict__.get('http_request', inferred_http) + resource = record.__dict__.get('resource', self.resource) + user_labels = record.__dict__.get('labels', {}) + # merge labels gae_labels = self.get_gae_labels() - http_request, trace_id = get_request_data() - if trace_id is not None: - trace_id = f"projects/{self.project_id}/{trace_id}" + gae_labels.update(user_labels) + # send off request self.transport.send( record, message, - resource=self.resource, + resource=resource, labels=gae_labels, - trace=trace_id, + trace= f"projects/{self.project_id}/{trace_id}", + # span_id=span_id, http_request=http_request, ) From e6ccd30e56188b845c0e04934b61b493a0630e89 Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Thu, 10 Dec 2020 10:35:41 -0800 Subject: [PATCH 14/35] fixed lint issue --- tests/unit/handlers/test_app_engine.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/handlers/test_app_engine.py b/tests/unit/handlers/test_app_engine.py index 16f16ce6..0f6f08cb 100644 --- a/tests/unit/handlers/test_app_engine.py +++ b/tests/unit/handlers/test_app_engine.py @@ -94,7 +94,7 @@ def test_emit(self): "google.cloud.logging_v2.handlers.app_engine.get_request_data", return_value=({"request_url": "test"}, "trace-test"), ) - with get_request_patch as mock_get_request: + with get_request_patch: client = mock.Mock(project=self.PROJECT, spec=["project"]) handler = self._make_one(client, transport=_Transport) From b653b227625adae6be8c63643d726dc0bb6d1e01 Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Thu, 10 Dec 2020 11:25:36 -0800 Subject: [PATCH 15/35] fixed trace path --- google/cloud/logging_v2/handlers/app_engine.py | 2 +- tests/unit/handlers/test_app_engine.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/google/cloud/logging_v2/handlers/app_engine.py b/google/cloud/logging_v2/handlers/app_engine.py index a74973d2..4d1fe808 100644 --- a/google/cloud/logging_v2/handlers/app_engine.py +++ b/google/cloud/logging_v2/handlers/app_engine.py @@ -116,7 +116,7 @@ def emit(self, record): gae_labels = self.get_gae_labels() http_request, trace_id = get_request_data() if trace_id is not None: - trace_id = f"projects/{self.project_id}/{trace_id}" + trace_id = f"projects/{self.project_id}/traces/{trace_id}" self.transport.send( record, message, diff --git a/tests/unit/handlers/test_app_engine.py b/tests/unit/handlers/test_app_engine.py index 0f6f08cb..e0109264 100644 --- a/tests/unit/handlers/test_app_engine.py +++ b/tests/unit/handlers/test_app_engine.py @@ -89,7 +89,7 @@ def test_constructor_w_gae_flex_env(self): def test_emit(self): expected_http_request = {"request_url": "test"} trace_id = "trace-test" - expected_trace_id = f"projects/{self.PROJECT}/{trace_id}" + expected_trace_id = f"projects/{self.PROJECT}/traces/{trace_id}" get_request_patch = mock.patch( "google.cloud.logging_v2.handlers.app_engine.get_request_data", return_value=({"request_url": "test"}, "trace-test"), From 889af7743b6bd29d3de6453492aaa1bb7a03b51f Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Thu, 10 Dec 2020 15:05:27 -0800 Subject: [PATCH 16/35] implemented update to base handler --- .../cloud/logging_v2/handlers/app_engine.py | 16 ++--- google/cloud/logging_v2/handlers/handlers.py | 24 +++++++- tests/unit/handlers/test_app_engine.py | 58 +++++++++++++++++-- tests/unit/handlers/test_handlers.py | 6 +- 4 files changed, 89 insertions(+), 15 deletions(-) diff --git a/google/cloud/logging_v2/handlers/app_engine.py b/google/cloud/logging_v2/handlers/app_engine.py index 31e0e6ec..0a766342 100644 --- a/google/cloud/logging_v2/handlers/app_engine.py +++ b/google/cloud/logging_v2/handlers/app_engine.py @@ -115,11 +115,13 @@ def emit(self, record): message = super(AppEngineHandler, self).format(record) inferred_http, inferred_trace_id = get_request_data() # allow user overrides - trace_id = record.__dict__.get('trace', inferred_trace_id) - span_id = record.__dict__.get('span_id', None) - http_request = record.__dict__.get('http_request', inferred_http) - resource = record.__dict__.get('resource', self.resource) - user_labels = record.__dict__.get('labels', {}) + trace_id = getattr(record, 'trace', inferred_trace_id) + if trace_id is not None and 'projects/' not in trace_id: + trace_id = f"projects/{self.project_id}/{trace_id}" + span_id = getattr(record, 'span_id', None) + http_request = getattr(record, 'http_request', inferred_http) + resource = getattr(record, 'resource', self.resource) + user_labels = getattr(record, 'labels', {}) # merge labels gae_labels = self.get_gae_labels() gae_labels.update(user_labels) @@ -129,7 +131,7 @@ def emit(self, record): message, resource=resource, labels=gae_labels, - trace= f"projects/{self.project_id}/{trace_id}", - # span_id=span_id, + trace=trace_id, + span_id=span_id, http_request=http_request, ) diff --git a/google/cloud/logging_v2/handlers/handlers.py b/google/cloud/logging_v2/handlers/handlers.py index d45c7b61..f65b7e6f 100644 --- a/google/cloud/logging_v2/handlers/handlers.py +++ b/google/cloud/logging_v2/handlers/handlers.py @@ -87,6 +87,7 @@ def __init__( self.name = name self.client = client self.transport = transport(client, name) + self.project_id = client.project self.resource = resource self.labels = labels @@ -101,7 +102,28 @@ def emit(self, record): record (logging.LogRecord): The record to be logged. """ message = super(CloudLoggingHandler, self).format(record) - self.transport.send(record, message, resource=self.resource, labels=self.labels) + trace_id = getattr(record, 'trace', None) + if trace_id is not None and 'projects/' not in trace_id: + trace_id = f"projects/{self.project_id}/{trace_id}" + span_id = getattr(record, 'span_id', None) + http_request = getattr(record, 'http_request', None) + resource = getattr(record, 'resource', self.resource) + user_labels = getattr(record, 'labels', {}) + # merge labels + total_labels = self.labels if self.labels is not None else {} + total_labels.update(user_labels) + if len(total_labels) == 0: + total_labels = None + # send off request + self.transport.send( + record, + message, + resource=resource, + labels=total_labels, + trace=trace_id, + span_id=span_id, + http_request=http_request, + ) def setup_logging( diff --git a/tests/unit/handlers/test_app_engine.py b/tests/unit/handlers/test_app_engine.py index 16f16ce6..3241498a 100644 --- a/tests/unit/handlers/test_app_engine.py +++ b/tests/unit/handlers/test_app_engine.py @@ -92,7 +92,7 @@ def test_emit(self): expected_trace_id = f"projects/{self.PROJECT}/{trace_id}" get_request_patch = mock.patch( "google.cloud.logging_v2.handlers.app_engine.get_request_data", - return_value=({"request_url": "test"}, "trace-test"), + return_value=(expected_http_request, trace_id), ) with get_request_patch as mock_get_request: @@ -118,10 +118,61 @@ def test_emit(self): gae_resource, gae_labels, expected_trace_id, + None, expected_http_request, ), ) + def test_emit_manual_field_override(self): + from google.cloud.logging_v2.resource import Resource + + inferred_http_request = {"request_url": "test"} + inferred_trace_id = "trace-test" + get_request_patch = mock.patch( + "google.cloud.logging_v2.handlers.app_engine.get_request_data", + return_value=(inferred_http_request, inferred_trace_id), + ) + with get_request_patch as mock_get_request: + client = mock.Mock(project=self.PROJECT, spec=["project"]) + handler = self._make_one(client, transport=_Transport) + gae_resource = handler.get_gae_resource() + gae_labels = handler.get_gae_labels() + logname = "app" + message = "hello world" + record = logging.LogRecord( + logname, logging, None, None, message, None, None + ) + handler.project_id = self.PROJECT + # set attributes manually + manual_trace_id = '123' + expected_trace = f"projects/{self.PROJECT}/{manual_trace_id}" + setattr(record, 'trace', manual_trace_id) + expected_span = '456' + setattr(record, 'span_id', expected_span) + expected_http = {'reuqest_url':'manual'} + setattr(record, 'http_request', expected_http) + expected_resource = gae_resource = Resource(type="test", labels={}) + setattr(record, 'resource', expected_resource) + additional_labels = {'test-label':'manual'} + expected_labels = dict(gae_labels) + expected_labels.update(additional_labels) + setattr(record, 'labels', additional_labels) + handler.emit(record) + self.assertIs(handler.transport.client, client) + self.assertEqual(handler.transport.name, logname) + self.assertEqual( + handler.transport.send_called_with, + ( + record, + message, + expected_resource, + expected_labels, + expected_trace, + expected_span, + expected_http, + ), + ) + def _get_gae_labels_helper(self, trace_id): get_request_patch = mock.patch( "google.cloud.logging_v2.handlers.app_engine.get_request_data", @@ -150,11 +201,10 @@ def test_get_gae_labels_without_label(self): gae_labels = self._get_gae_labels_helper(None) self.assertEqual(gae_labels, {}) - class _Transport(object): def __init__(self, client, name): self.client = client self.name = name - def send(self, record, message, resource, labels, trace, http_request): - self.send_called_with = (record, message, resource, labels, trace, http_request) + def send(self, record, message, resource, labels, trace, span_id, http_request): + self.send_called_with = (record, message, resource, labels, trace, span_id, http_request) diff --git a/tests/unit/handlers/test_handlers.py b/tests/unit/handlers/test_handlers.py index e967b201..e5325297 100644 --- a/tests/unit/handlers/test_handlers.py +++ b/tests/unit/handlers/test_handlers.py @@ -85,7 +85,7 @@ def test_emit(self): self.assertEqual( handler.transport.send_called_with, - (record, message, _GLOBAL_RESOURCE, None), + (record, message, _GLOBAL_RESOURCE, None, None, None, None), ) @@ -148,5 +148,5 @@ def __init__(self, client, name): self.client = client self.name = name - def send(self, record, message, resource, labels=None): - self.send_called_with = (record, message, resource, labels) + def send(self, record, message, resource, labels=None, trace=None, span_id=None, http_request=None): + self.send_called_with = (record, message, resource, labels, trace, span_id, http_request) From a1a4a257ba77ad04eac81db60babfea6ed30ae61 Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Thu, 10 Dec 2020 15:12:55 -0800 Subject: [PATCH 17/35] added unit test for base handler --- tests/unit/handlers/test_handlers.py | 38 ++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/tests/unit/handlers/test_handlers.py b/tests/unit/handlers/test_handlers.py index e5325297..a6820303 100644 --- a/tests/unit/handlers/test_handlers.py +++ b/tests/unit/handlers/test_handlers.py @@ -88,6 +88,44 @@ def test_emit(self): (record, message, _GLOBAL_RESOURCE, None, None, None, None), ) + def test_emit_manual_field_override(self): + from google.cloud.logging_v2.logger import _GLOBAL_RESOURCE + from google.cloud.logging_v2.resource import Resource + + client = _Client(self.PROJECT) + handler = self._make_one( + client, transport=_Transport, resource=_GLOBAL_RESOURCE + ) + logname = "loggername" + message = "hello world" + record = logging.LogRecord(logname, logging, None, None, message, None, None) + # set attributes manually + manual_trace_id = '123' + expected_trace = f"projects/{self.PROJECT}/{manual_trace_id}" + setattr(record, 'trace', manual_trace_id) + expected_span = '456' + setattr(record, 'span_id', expected_span) + expected_http = {'reuqest_url':'manual'} + setattr(record, 'http_request', expected_http) + expected_resource = gae_resource = Resource(type="test", labels={}) + setattr(record, 'resource', expected_resource) + expected_labels = {'test-label':'manual'} + setattr(record, 'labels', expected_labels) + handler.emit(record) + + self.assertEqual( + handler.transport.send_called_with, + ( + record, + message, + expected_resource, + expected_labels, + expected_trace, + expected_span, + expected_http, + ), + ) + class TestSetupLogging(unittest.TestCase): def _call_fut(self, handler, excludes=None): From 745a2c456acb157db8f58d433c8f9a8f97d42e73 Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Thu, 10 Dec 2020 15:34:59 -0800 Subject: [PATCH 18/35] fixed how traces are handled --- google/cloud/logging_v2/handlers/app_engine.py | 4 ++-- google/cloud/logging_v2/handlers/handlers.py | 4 +--- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/google/cloud/logging_v2/handlers/app_engine.py b/google/cloud/logging_v2/handlers/app_engine.py index 0a766342..3370d4ac 100644 --- a/google/cloud/logging_v2/handlers/app_engine.py +++ b/google/cloud/logging_v2/handlers/app_engine.py @@ -114,10 +114,10 @@ def emit(self, record): """ message = super(AppEngineHandler, self).format(record) inferred_http, inferred_trace_id = get_request_data() + if inferred_trace_id is not None: + inferred_trace_id = f"projects/{inferred_trace_id}/{trace_id}" # allow user overrides trace_id = getattr(record, 'trace', inferred_trace_id) - if trace_id is not None and 'projects/' not in trace_id: - trace_id = f"projects/{self.project_id}/{trace_id}" span_id = getattr(record, 'span_id', None) http_request = getattr(record, 'http_request', inferred_http) resource = getattr(record, 'resource', self.resource) diff --git a/google/cloud/logging_v2/handlers/handlers.py b/google/cloud/logging_v2/handlers/handlers.py index f65b7e6f..b3f16459 100644 --- a/google/cloud/logging_v2/handlers/handlers.py +++ b/google/cloud/logging_v2/handlers/handlers.py @@ -103,8 +103,6 @@ def emit(self, record): """ message = super(CloudLoggingHandler, self).format(record) trace_id = getattr(record, 'trace', None) - if trace_id is not None and 'projects/' not in trace_id: - trace_id = f"projects/{self.project_id}/{trace_id}" span_id = getattr(record, 'span_id', None) http_request = getattr(record, 'http_request', None) resource = getattr(record, 'resource', self.resource) @@ -119,7 +117,7 @@ def emit(self, record): record, message, resource=resource, - labels=total_labels, + labels=(total_labels if total_labels else None), trace=trace_id, span_id=span_id, http_request=http_request, From 2d5ad97ffac19e91d8ec98acb5e078c30ce41a63 Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Thu, 10 Dec 2020 17:01:00 -0800 Subject: [PATCH 19/35] added system test --- tests/system/test_system.py | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/tests/system/test_system.py b/tests/system/test_system.py index f9cb96e1..83d0daf2 100644 --- a/tests/system/test_system.py +++ b/tests/system/test_system.py @@ -308,6 +308,39 @@ def test_log_handler_sync(self): self.assertEqual(len(entries), 1) self.assertEqual(entries[0].payload, expected_payload) + @pytest.mark.run_these_please + def test_cloud_logging_handler_with_extras(self): + LOG_MESSAGE = "Testing with injected extras." + + handler_name = self._logger_name("handler_extras") + 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) + + LOGGER_NAME = "extras" + cloud_logger = logging.getLogger(LOGGER_NAME) + cloud_logger.addHandler(handler) + extra = { + 'trace': '123', + 'span_id': '456', + 'http_request': {'requestUrl':'manual'}, + 'resource': Resource(type="cloudiot_device", labels={}), + 'labels': {'test-label':'manual'} + } + cloud_logger.warn(LOG_MESSAGE, extra=extra) + + entries = _list_entries(logger) + self.assertEqual(len(entries), 1) + self.assertEqual(entries[0].trace, extra['trace']) + self.assertEqual(entries[0].span_id, extra['span_id']) + self.assertEqual(entries[0].http_request, extra['http_request']) + self.assertEqual(entries[0].labels, extra['labels']) + self.assertEqual(entries[0].resource.type, extra['resource'].type) + def test_log_root_handler(self): LOG_MESSAGE = "It was the best of times." From b5ad409dc87fa1cee5be4da1b691dd6ed9a926ac Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Thu, 10 Dec 2020 17:12:48 -0800 Subject: [PATCH 20/35] loop through both handler classes --- tests/system/test_system.py | 61 +++++++++++++++++++------------------ 1 file changed, 32 insertions(+), 29 deletions(-) diff --git a/tests/system/test_system.py b/tests/system/test_system.py index 83d0daf2..9d375730 100644 --- a/tests/system/test_system.py +++ b/tests/system/test_system.py @@ -27,7 +27,8 @@ from google.api_core.exceptions import ServiceUnavailable import google.cloud.logging from google.cloud._helpers import UTC -from google.cloud.logging_v2.handlers.handlers import CloudLoggingHandler +from google.cloud.logging_v2.handlers import AppEngineHandler +from google.cloud.logging_v2.handlers import CloudLoggingHandler from google.cloud.logging_v2.handlers.transports import SyncTransport from google.cloud.logging_v2 import client from google.cloud.logging_v2.resource import Resource @@ -312,34 +313,36 @@ def test_log_handler_sync(self): def test_cloud_logging_handler_with_extras(self): LOG_MESSAGE = "Testing with injected extras." - handler_name = self._logger_name("handler_extras") - 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) - - LOGGER_NAME = "extras" - cloud_logger = logging.getLogger(LOGGER_NAME) - cloud_logger.addHandler(handler) - extra = { - 'trace': '123', - 'span_id': '456', - 'http_request': {'requestUrl':'manual'}, - 'resource': Resource(type="cloudiot_device", labels={}), - 'labels': {'test-label':'manual'} - } - cloud_logger.warn(LOG_MESSAGE, extra=extra) - - entries = _list_entries(logger) - self.assertEqual(len(entries), 1) - self.assertEqual(entries[0].trace, extra['trace']) - self.assertEqual(entries[0].span_id, extra['span_id']) - self.assertEqual(entries[0].http_request, extra['http_request']) - self.assertEqual(entries[0].labels, extra['labels']) - self.assertEqual(entries[0].resource.type, extra['resource'].type) + for cls in [CloudLoggingHandler, AppEngineHandler]: + LOGGER_NAME = f"{cls.__name__}-handler_extras" + handler_name = self._logger_name(LOGGER_NAME) + + handler = cls( + 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) + extra = { + 'trace': '123', + 'span_id': '456', + 'http_request': {'requestUrl':'manual'}, + 'resource': Resource(type="cloudiot_device", labels={}), + 'labels': {'test-label':'manual'} + } + cloud_logger.warn(LOG_MESSAGE, extra=extra) + + entries = _list_entries(logger) + self.assertEqual(len(entries), 1) + self.assertEqual(entries[0].trace, extra['trace']) + self.assertEqual(entries[0].span_id, extra['span_id']) + self.assertEqual(entries[0].http_request, extra['http_request']) + self.assertEqual(entries[0].labels, extra['labels']) + self.assertEqual(entries[0].resource.type, extra['resource'].type) def test_log_root_handler(self): LOG_MESSAGE = "It was the best of times." From a298856b8820ed027d2ef433c8c1d10927bc663a Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Thu, 10 Dec 2020 17:17:51 -0800 Subject: [PATCH 21/35] renamed test function --- tests/system/test_system.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/system/test_system.py b/tests/system/test_system.py index 9d375730..d75b639a 100644 --- a/tests/system/test_system.py +++ b/tests/system/test_system.py @@ -309,8 +309,7 @@ def test_log_handler_sync(self): self.assertEqual(len(entries), 1) self.assertEqual(entries[0].payload, expected_payload) - @pytest.mark.run_these_please - def test_cloud_logging_handler_with_extras(self): + def test_handlers_w_extras(self): LOG_MESSAGE = "Testing with injected extras." for cls in [CloudLoggingHandler, AppEngineHandler]: From 6ca954e7c647951c3b5b9f69961f8d5c999805b6 Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Thu, 10 Dec 2020 17:30:14 -0800 Subject: [PATCH 22/35] added protocol to http request --- google/cloud/logging_v2/handlers/_helpers.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/google/cloud/logging_v2/handlers/_helpers.py b/google/cloud/logging_v2/handlers/_helpers.py index 108a21ab..15c9c68b 100644 --- a/google/cloud/logging_v2/handlers/_helpers.py +++ b/google/cloud/logging_v2/handlers/_helpers.py @@ -30,6 +30,7 @@ _DJANGO_REMOTE_ADDR_HEADER = "REMOTE_ADDR" _DJANGO_REFERER_HEADER = "HTTP_REFERER" _FLASK_TRACE_HEADER = "X_CLOUD_TRACE_CONTEXT" +_PROTOCOL_HEADER = "SERVER_PROTOCOL" def format_stackdriver_json(record, message): @@ -68,6 +69,7 @@ def get_request_data_from_flask(): "user_agent": flask.request.user_agent.string, "remote_ip": flask.request.remote_addr, "referer": flask.request.referrer, + "protocol": request.environ.get(_PROTOCOL_HEADER) } # find trace id @@ -99,6 +101,7 @@ def get_request_data_from_django(): "user_agent": request.META.get(_DJANGO_USERAGENT_HEADER), "remote_ip": request.META.get(_DJANGO_REMOTE_ADDR_HEADER), "referer": request.META.get(_DJANGO_REFERER_HEADER), + "protocol": request.META.get(_PROTOCOL_HEADER), } # find trace id From 16e7888e577d662159fbc98f76380c1e33321985 Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Thu, 10 Dec 2020 17:32:46 -0800 Subject: [PATCH 23/35] added supported frameworks --- google/cloud/logging_v2/handlers/_helpers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/google/cloud/logging_v2/handlers/_helpers.py b/google/cloud/logging_v2/handlers/_helpers.py index 15c9c68b..bd1f56a3 100644 --- a/google/cloud/logging_v2/handlers/_helpers.py +++ b/google/cloud/logging_v2/handlers/_helpers.py @@ -115,7 +115,7 @@ def get_request_data_from_django(): def get_request_data(): """Helper to get trace_id and http_request data from supported web - frameworks. + frameworks (currently supported: Flask and Django). Returns: str: TraceID in HTTP request headers. From 36a71ab38c97495c8f7492e44d3b62ee727db64a Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Fri, 11 Dec 2020 12:20:08 -0800 Subject: [PATCH 24/35] added tests for protocol --- google/cloud/logging_v2/handlers/_helpers.py | 2 +- tests/unit/handlers/test__helpers.py | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/google/cloud/logging_v2/handlers/_helpers.py b/google/cloud/logging_v2/handlers/_helpers.py index bd1f56a3..a488234b 100644 --- a/google/cloud/logging_v2/handlers/_helpers.py +++ b/google/cloud/logging_v2/handlers/_helpers.py @@ -69,7 +69,7 @@ def get_request_data_from_flask(): "user_agent": flask.request.user_agent.string, "remote_ip": flask.request.remote_addr, "referer": flask.request.referrer, - "protocol": request.environ.get(_PROTOCOL_HEADER) + "protocol": flask.request.environ.get(_PROTOCOL_HEADER) } # find trace id diff --git a/tests/unit/handlers/test__helpers.py b/tests/unit/handlers/test__helpers.py index cc6de761..d10b6c4e 100644 --- a/tests/unit/handlers/test__helpers.py +++ b/tests/unit/handlers/test__helpers.py @@ -27,6 +27,7 @@ "user_agent", "remote_ip", "referer", + "protocol", ] @@ -107,6 +108,7 @@ def test_http_data(self): self.assertEqual(http_request["referer"], expected_referrer) self.assertEqual(http_request["remote_ip"], expected_ip) self.assertEqual(http_request["request_size"], str(len(body_content))) + self.assertEqual(http_request["protocol"], "HTTP/1.1") self.assertEqual(set(http_request.keys()), set(_HTTP_REQUEST_FIELDS)) @@ -193,6 +195,7 @@ def test_http_data(self): self.assertEqual(http_request["referer"], expected_referrer) self.assertEqual(http_request["remote_ip"], "127.0.0.1") self.assertEqual(http_request["request_size"], str(len(body_content))) + self.assertEqual(http_request["protocol"], "HTTP/1.1") self.assertEqual(set(http_request.keys()), set(_HTTP_REQUEST_FIELDS)) From 6b549bdc7171ee2c1b701cbf59f310cad9fa9e56 Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Fri, 11 Dec 2020 12:55:05 -0800 Subject: [PATCH 25/35] removed unneded assertions --- tests/unit/handlers/test__helpers.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/tests/unit/handlers/test__helpers.py b/tests/unit/handlers/test__helpers.py index d10b6c4e..0bfb53cb 100644 --- a/tests/unit/handlers/test__helpers.py +++ b/tests/unit/handlers/test__helpers.py @@ -77,8 +77,6 @@ def test_valid_context_header(self): self.assertEqual(trace_id, expected_trace_id) self.assertEqual(http_request["request_method"], "GET") self.assertEqual(set(http_request.keys()), set(_HTTP_REQUEST_FIELDS)) - for field in _HTTP_REQUEST_FIELDS: - self.assertTrue(field in http_request) def test_http_data(self): expected_path = "http://testserver/123" @@ -89,7 +87,6 @@ def test_http_data(self): headers = { "User-Agent": expected_agent, "Referer": expected_referrer, - "HTTP_X_FORWARDED_FOR": "0.0.0.0", } app = self.create_app() @@ -145,8 +142,6 @@ def test_no_context_header(self): http_request, trace_id = self._call_fut() self.assertEqual(http_request["request_method"], "GET") self.assertEqual(set(http_request.keys()), set(_HTTP_REQUEST_FIELDS)) - for field in _HTTP_REQUEST_FIELDS: - self.assertTrue(field in http_request) self.assertIsNone(trace_id) def test_valid_context_header(self): From e91548cd69bd62885712215471de274bd8eb7fb4 Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Fri, 11 Dec 2020 12:56:11 -0800 Subject: [PATCH 26/35] renamed test function --- tests/unit/handlers/test__helpers.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/unit/handlers/test__helpers.py b/tests/unit/handlers/test__helpers.py index 0bfb53cb..07ae26df 100644 --- a/tests/unit/handlers/test__helpers.py +++ b/tests/unit/handlers/test__helpers.py @@ -78,7 +78,7 @@ def test_valid_context_header(self): self.assertEqual(http_request["request_method"], "GET") self.assertEqual(set(http_request.keys()), set(_HTTP_REQUEST_FIELDS)) - def test_http_data(self): + def test_http_request_auto_populated(self): expected_path = "http://testserver/123" expected_agent = "Mozilla/5.0" expected_referrer = "self" @@ -166,7 +166,7 @@ def test_valid_context_header(self): for field in _HTTP_REQUEST_FIELDS: self.assertTrue(field in http_request) - def test_http_data(self): + def test_http_request_auto_populated(self): from django.test import RequestFactory from google.cloud.logging_v2.handlers.middleware import request From 5f9d93276af1c27ee7194d892ce63635a619f222 Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Fri, 11 Dec 2020 13:16:32 -0800 Subject: [PATCH 27/35] added tests for sparse http requests --- google/cloud/logging_v2/handlers/_helpers.py | 3 +- tests/unit/handlers/test__helpers.py | 34 ++++++++++++++++++-- 2 files changed, 34 insertions(+), 3 deletions(-) diff --git a/google/cloud/logging_v2/handlers/_helpers.py b/google/cloud/logging_v2/handlers/_helpers.py index a488234b..a8cc1458 100644 --- a/google/cloud/logging_v2/handlers/_helpers.py +++ b/google/cloud/logging_v2/handlers/_helpers.py @@ -62,10 +62,11 @@ def get_request_data_from_flask(): return None, None # build http_request + length = flask.request.content_length http_request = { "request_method": flask.request.method, "request_url": flask.request.url, - "request_size": str(flask.request.content_length), + "request_size": str(length) if length else None, "user_agent": flask.request.user_agent.string, "remote_ip": flask.request.remote_addr, "referer": flask.request.referrer, diff --git a/tests/unit/handlers/test__helpers.py b/tests/unit/handlers/test__helpers.py index 07ae26df..3b4d1608 100644 --- a/tests/unit/handlers/test__helpers.py +++ b/tests/unit/handlers/test__helpers.py @@ -78,7 +78,7 @@ def test_valid_context_header(self): self.assertEqual(http_request["request_method"], "GET") self.assertEqual(set(http_request.keys()), set(_HTTP_REQUEST_FIELDS)) - def test_http_request_auto_populated(self): + def test_http_request_populated(self): expected_path = "http://testserver/123" expected_agent = "Mozilla/5.0" expected_referrer = "self" @@ -108,6 +108,18 @@ def test_http_request_auto_populated(self): self.assertEqual(http_request["protocol"], "HTTP/1.1") self.assertEqual(set(http_request.keys()), set(_HTTP_REQUEST_FIELDS)) + def test_http_request_sparse(self): + expected_path = "http://testserver/123" + app = self.create_app() + with app.test_client() as c: + c.put(path=expected_path) + http_request, trace_id = self._call_fut() + self.assertEqual(http_request["request_method"], "PUT") + self.assertEqual(http_request["request_url"], expected_path) + self.assertEqual(http_request["protocol"], "HTTP/1.1") + self.assertIsNone(http_request["referer"]) + self.assertIsNone(http_request["request_size"]) + self.assertEqual(set(http_request.keys()), set(_HTTP_REQUEST_FIELDS)) class Test_get_request_data_from_django(unittest.TestCase): @staticmethod @@ -166,7 +178,7 @@ def test_valid_context_header(self): for field in _HTTP_REQUEST_FIELDS: self.assertTrue(field in http_request) - def test_http_request_auto_populated(self): + def test_http_request_populated(self): from django.test import RequestFactory from google.cloud.logging_v2.handlers.middleware import request @@ -193,6 +205,24 @@ def test_http_request_auto_populated(self): self.assertEqual(http_request["protocol"], "HTTP/1.1") self.assertEqual(set(http_request.keys()), set(_HTTP_REQUEST_FIELDS)) + def test_http_request_sparse(self): + from django.test import RequestFactory + from google.cloud.logging_v2.handlers.middleware import request + + expected_path = "http://testserver/123" + django_request = RequestFactory().put(expected_path) + middleware = request.RequestMiddleware(None) + middleware.process_request(django_request) + http_request, trace_id = self._call_fut() + self.assertEqual(http_request["request_method"], "PUT") + self.assertEqual(http_request["request_url"], expected_path) + self.assertIsNone(http_request["referer"]) + self.assertEqual(http_request["remote_ip"], "127.0.0.1") + self.assertIsNone(http_request["request_size"]) + self.assertEqual(http_request["protocol"], "HTTP/1.1") + self.assertEqual(set(http_request.keys()), set(_HTTP_REQUEST_FIELDS)) + + class Test_get_request_data(unittest.TestCase): @staticmethod From 69c0d396159bfee857825368ce0c1900595f2a9d Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Fri, 11 Dec 2020 13:30:00 -0800 Subject: [PATCH 28/35] added comment --- tests/unit/handlers/test_app_engine.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/unit/handlers/test_app_engine.py b/tests/unit/handlers/test_app_engine.py index e0109264..71672fa6 100644 --- a/tests/unit/handlers/test_app_engine.py +++ b/tests/unit/handlers/test_app_engine.py @@ -92,10 +92,10 @@ def test_emit(self): expected_trace_id = f"projects/{self.PROJECT}/traces/{trace_id}" get_request_patch = mock.patch( "google.cloud.logging_v2.handlers.app_engine.get_request_data", - return_value=({"request_url": "test"}, "trace-test"), + return_value=(expected_http_request, trace_id), ) with get_request_patch: - + # library integrations mocked to return test data client = mock.Mock(project=self.PROJECT, spec=["project"]) handler = self._make_one(client, transport=_Transport) gae_resource = handler.get_gae_resource() From 5bb9e51dc8f5aad6efc504aba379cebfbfb86b43 Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Fri, 11 Dec 2020 13:30:41 -0800 Subject: [PATCH 29/35] blackened --- google/cloud/logging_v2/handlers/_helpers.py | 2 +- tests/unit/handlers/test__helpers.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/google/cloud/logging_v2/handlers/_helpers.py b/google/cloud/logging_v2/handlers/_helpers.py index a8cc1458..0381e20f 100644 --- a/google/cloud/logging_v2/handlers/_helpers.py +++ b/google/cloud/logging_v2/handlers/_helpers.py @@ -70,7 +70,7 @@ def get_request_data_from_flask(): "user_agent": flask.request.user_agent.string, "remote_ip": flask.request.remote_addr, "referer": flask.request.referrer, - "protocol": flask.request.environ.get(_PROTOCOL_HEADER) + "protocol": flask.request.environ.get(_PROTOCOL_HEADER), } # find trace id diff --git a/tests/unit/handlers/test__helpers.py b/tests/unit/handlers/test__helpers.py index 3b4d1608..ccc8335f 100644 --- a/tests/unit/handlers/test__helpers.py +++ b/tests/unit/handlers/test__helpers.py @@ -121,6 +121,7 @@ def test_http_request_sparse(self): self.assertIsNone(http_request["request_size"]) self.assertEqual(set(http_request.keys()), set(_HTTP_REQUEST_FIELDS)) + class Test_get_request_data_from_django(unittest.TestCase): @staticmethod def _call_fut(): @@ -223,7 +224,6 @@ def test_http_request_sparse(self): self.assertEqual(set(http_request.keys()), set(_HTTP_REQUEST_FIELDS)) - class Test_get_request_data(unittest.TestCase): @staticmethod def _call_fut(): From 6d1974db245fd3835b7dec6096062635bed6086f Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Fri, 11 Dec 2020 14:43:05 -0800 Subject: [PATCH 30/35] use proto instead of dict for http_request --- google/cloud/logging_v2/handlers/_helpers.py | 48 ++++++------- tests/unit/handlers/test__helpers.py | 75 +++++++------------- 2 files changed, 47 insertions(+), 76 deletions(-) diff --git a/google/cloud/logging_v2/handlers/_helpers.py b/google/cloud/logging_v2/handlers/_helpers.py index 0381e20f..8dc8c75a 100644 --- a/google/cloud/logging_v2/handlers/_helpers.py +++ b/google/cloud/logging_v2/handlers/_helpers.py @@ -23,9 +23,9 @@ flask = None from google.cloud.logging_v2.handlers.middleware.request import _get_django_request +from google.logging.type.http_request_pb2 import HttpRequest _DJANGO_TRACE_HEADER = "HTTP_X_CLOUD_TRACE_CONTEXT" -_DJANGO_LENGTH_HEADER = "CONTENT_LENGTH" _DJANGO_USERAGENT_HEADER = "HTTP_USER_AGENT" _DJANGO_REMOTE_ADDR_HEADER = "REMOTE_ADDR" _DJANGO_REFERER_HEADER = "HTTP_REFERER" @@ -56,22 +56,19 @@ def get_request_data_from_flask(): Returns: str: TraceID in HTTP request headers. - dict: data about the associated http request. + HttpRequest: data about the associated http request. """ - if flask is None or not flask.request: - return None, None # build http_request - length = flask.request.content_length - http_request = { - "request_method": flask.request.method, - "request_url": flask.request.url, - "request_size": str(length) if length else None, - "user_agent": flask.request.user_agent.string, - "remote_ip": flask.request.remote_addr, - "referer": flask.request.referrer, - "protocol": flask.request.environ.get(_PROTOCOL_HEADER), - } + http_request = HttpRequest( + request_method=flask.request.method, + request_url=flask.request.url, + request_size=flask.request.content_length, + user_agent=flask.request.user_agent.string, + remote_ip=flask.request.remote_addr, + referer=flask.request.referrer, + protocol=flask.request.environ.get(_PROTOCOL_HEADER), + ) # find trace id trace_id = None @@ -87,23 +84,22 @@ def get_request_data_from_django(): Returns: str: TraceID in HTTP request headers. - dict: data about the associated http request. + HttpRequest: data about the associated http request. """ request = _get_django_request() if request is None: return None, None - # build http_request - http_request = { - "request_method": request.method, - "request_url": request.build_absolute_uri(), - "request_size": request.META.get(_DJANGO_LENGTH_HEADER), - "user_agent": request.META.get(_DJANGO_USERAGENT_HEADER), - "remote_ip": request.META.get(_DJANGO_REMOTE_ADDR_HEADER), - "referer": request.META.get(_DJANGO_REFERER_HEADER), - "protocol": request.META.get(_PROTOCOL_HEADER), - } + http_request = HttpRequest( + request_method=request.method, + request_url=request.build_absolute_uri(), + request_size=len(request.body), + user_agent=request.META.get(_DJANGO_USERAGENT_HEADER), + remote_ip=request.META.get(_DJANGO_REMOTE_ADDR_HEADER), + referer=request.META.get(_DJANGO_REFERER_HEADER), + protocol=request.META.get(_PROTOCOL_HEADER), + ) # find trace id trace_id = None @@ -120,7 +116,7 @@ def get_request_data(): Returns: str: TraceID in HTTP request headers. - dict: data about the associated http request. + HttpRequest: data about the associated http request. """ checkers = ( get_request_data_from_django, diff --git a/tests/unit/handlers/test__helpers.py b/tests/unit/handlers/test__helpers.py index ccc8335f..3a6d5567 100644 --- a/tests/unit/handlers/test__helpers.py +++ b/tests/unit/handlers/test__helpers.py @@ -20,15 +20,6 @@ _FLASK_HTTP_REQUEST = {"request_url": "https://flask.palletsprojects.com/en/1.1.x/"} _DJANGO_TRACE_ID = "django-id" _DJANGO_HTTP_REQUEST = {"request_url": "https://www.djangoproject.com/"} -_HTTP_REQUEST_FIELDS = [ - "request_method", - "request_url", - "request_size", - "user_agent", - "remote_ip", - "referer", - "protocol", -] class Test_get_request_data_from_flask(unittest.TestCase): @@ -56,10 +47,7 @@ def test_no_context_header(self): http_request, trace_id = self._call_fut() self.assertIsNone(trace_id) - self.assertEqual(http_request["request_method"], "GET") - self.assertEqual(set(http_request.keys()), set(_HTTP_REQUEST_FIELDS)) - for field in _HTTP_REQUEST_FIELDS: - self.assertTrue(field in http_request) + self.assertEqual(http_request.request_method, "GET") def test_valid_context_header(self): flask_trace_header = "X_CLOUD_TRACE_CONTEXT" @@ -75,8 +63,7 @@ def test_valid_context_header(self): http_request, trace_id = self._call_fut() self.assertEqual(trace_id, expected_trace_id) - self.assertEqual(http_request["request_method"], "GET") - self.assertEqual(set(http_request.keys()), set(_HTTP_REQUEST_FIELDS)) + self.assertEqual(http_request.request_method, "GET") def test_http_request_populated(self): expected_path = "http://testserver/123" @@ -99,14 +86,13 @@ def test_http_request_populated(self): ) http_request, trace_id = self._call_fut() - self.assertEqual(http_request["request_method"], "PUT") - self.assertEqual(http_request["request_url"], expected_path) - self.assertEqual(http_request["user_agent"], expected_agent) - self.assertEqual(http_request["referer"], expected_referrer) - self.assertEqual(http_request["remote_ip"], expected_ip) - self.assertEqual(http_request["request_size"], str(len(body_content))) - self.assertEqual(http_request["protocol"], "HTTP/1.1") - self.assertEqual(set(http_request.keys()), set(_HTTP_REQUEST_FIELDS)) + self.assertEqual(http_request.request_method, "PUT") + self.assertEqual(http_request.request_url, expected_path) + self.assertEqual(http_request.user_agent, expected_agent) + self.assertEqual(http_request.referer, expected_referrer) + self.assertEqual(http_request.remote_ip, expected_ip) + self.assertEqual(http_request.request_size, len(body_content)) + self.assertEqual(http_request.protocol, "HTTP/1.1") def test_http_request_sparse(self): expected_path = "http://testserver/123" @@ -114,12 +100,9 @@ def test_http_request_sparse(self): with app.test_client() as c: c.put(path=expected_path) http_request, trace_id = self._call_fut() - self.assertEqual(http_request["request_method"], "PUT") - self.assertEqual(http_request["request_url"], expected_path) - self.assertEqual(http_request["protocol"], "HTTP/1.1") - self.assertIsNone(http_request["referer"]) - self.assertIsNone(http_request["request_size"]) - self.assertEqual(set(http_request.keys()), set(_HTTP_REQUEST_FIELDS)) + self.assertEqual(http_request.request_method, "PUT") + self.assertEqual(http_request.request_url, expected_path) + self.assertEqual(http_request.protocol, "HTTP/1.1") class Test_get_request_data_from_django(unittest.TestCase): @@ -153,8 +136,7 @@ def test_no_context_header(self): middleware = request.RequestMiddleware(None) middleware.process_request(django_request) http_request, trace_id = self._call_fut() - self.assertEqual(http_request["request_method"], "GET") - self.assertEqual(set(http_request.keys()), set(_HTTP_REQUEST_FIELDS)) + self.assertEqual(http_request.request_method, "GET") self.assertIsNone(trace_id) def test_valid_context_header(self): @@ -174,10 +156,7 @@ def test_valid_context_header(self): http_request, trace_id = self._call_fut() self.assertEqual(trace_id, expected_trace_id) - self.assertEqual(http_request["request_method"], "GET") - self.assertEqual(set(http_request.keys()), set(_HTTP_REQUEST_FIELDS)) - for field in _HTTP_REQUEST_FIELDS: - self.assertTrue(field in http_request) + self.assertEqual(http_request.request_method, "GET") def test_http_request_populated(self): from django.test import RequestFactory @@ -197,14 +176,13 @@ def test_http_request_populated(self): middleware = request.RequestMiddleware(None) middleware.process_request(django_request) http_request, trace_id = self._call_fut() - self.assertEqual(http_request["request_method"], "PUT") - self.assertEqual(http_request["request_url"], expected_path) - self.assertEqual(http_request["user_agent"], expected_agent) - self.assertEqual(http_request["referer"], expected_referrer) - self.assertEqual(http_request["remote_ip"], "127.0.0.1") - self.assertEqual(http_request["request_size"], str(len(body_content))) - self.assertEqual(http_request["protocol"], "HTTP/1.1") - self.assertEqual(set(http_request.keys()), set(_HTTP_REQUEST_FIELDS)) + self.assertEqual(http_request.request_method, "PUT") + self.assertEqual(http_request.request_url, expected_path) + self.assertEqual(http_request.user_agent, expected_agent) + self.assertEqual(http_request.referer, expected_referrer) + self.assertEqual(http_request.remote_ip, "127.0.0.1") + self.assertEqual(http_request.request_size, len(body_content)) + self.assertEqual(http_request.protocol, "HTTP/1.1") def test_http_request_sparse(self): from django.test import RequestFactory @@ -215,13 +193,10 @@ def test_http_request_sparse(self): middleware = request.RequestMiddleware(None) middleware.process_request(django_request) http_request, trace_id = self._call_fut() - self.assertEqual(http_request["request_method"], "PUT") - self.assertEqual(http_request["request_url"], expected_path) - self.assertIsNone(http_request["referer"]) - self.assertEqual(http_request["remote_ip"], "127.0.0.1") - self.assertIsNone(http_request["request_size"]) - self.assertEqual(http_request["protocol"], "HTTP/1.1") - self.assertEqual(set(http_request.keys()), set(_HTTP_REQUEST_FIELDS)) + self.assertEqual(http_request.request_method, "PUT") + self.assertEqual(http_request.request_url, expected_path) + self.assertEqual(http_request.remote_ip, "127.0.0.1") + self.assertEqual(http_request.protocol, "HTTP/1.1") class Test_get_request_data(unittest.TestCase): From d22ed8e9d619a43c8723334267260e00145a690d Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Fri, 11 Dec 2020 15:39:12 -0800 Subject: [PATCH 31/35] fixed tests after merge --- google/cloud/logging_v2/handlers/app_engine.py | 10 +++++----- tests/unit/handlers/test_app_engine.py | 5 ++--- tests/unit/handlers/test_handlers.py | 5 ++--- 3 files changed, 9 insertions(+), 11 deletions(-) diff --git a/google/cloud/logging_v2/handlers/app_engine.py b/google/cloud/logging_v2/handlers/app_engine.py index 3370d4ac..419d1b8f 100644 --- a/google/cloud/logging_v2/handlers/app_engine.py +++ b/google/cloud/logging_v2/handlers/app_engine.py @@ -113,11 +113,11 @@ def emit(self, record): record (logging.LogRecord): The record to be logged. """ message = super(AppEngineHandler, self).format(record) - inferred_http, inferred_trace_id = get_request_data() - if inferred_trace_id is not None: - inferred_trace_id = f"projects/{inferred_trace_id}/{trace_id}" + inferred_http, inferred_trace = get_request_data() + if inferred_trace is not None: + inferred_trace = f"projects/{self.project_id}/traces/{inferred_trace}" # allow user overrides - trace_id = getattr(record, 'trace', inferred_trace_id) + trace = getattr(record, 'trace', inferred_trace) span_id = getattr(record, 'span_id', None) http_request = getattr(record, 'http_request', inferred_http) resource = getattr(record, 'resource', self.resource) @@ -131,7 +131,7 @@ def emit(self, record): message, resource=resource, labels=gae_labels, - trace=trace_id, + trace=trace, span_id=span_id, http_request=http_request, ) diff --git a/tests/unit/handlers/test_app_engine.py b/tests/unit/handlers/test_app_engine.py index f51edcb5..904abc72 100644 --- a/tests/unit/handlers/test_app_engine.py +++ b/tests/unit/handlers/test_app_engine.py @@ -144,9 +144,8 @@ def test_emit_manual_field_override(self): ) handler.project_id = self.PROJECT # set attributes manually - manual_trace_id = '123' - expected_trace = f"projects/{self.PROJECT}/{manual_trace_id}" - setattr(record, 'trace', manual_trace_id) + expected_trace = '123' + setattr(record, 'trace', expected_trace) expected_span = '456' setattr(record, 'span_id', expected_span) expected_http = {'reuqest_url':'manual'} diff --git a/tests/unit/handlers/test_handlers.py b/tests/unit/handlers/test_handlers.py index a6820303..0405656b 100644 --- a/tests/unit/handlers/test_handlers.py +++ b/tests/unit/handlers/test_handlers.py @@ -100,9 +100,8 @@ def test_emit_manual_field_override(self): message = "hello world" record = logging.LogRecord(logname, logging, None, None, message, None, None) # set attributes manually - manual_trace_id = '123' - expected_trace = f"projects/{self.PROJECT}/{manual_trace_id}" - setattr(record, 'trace', manual_trace_id) + expected_trace = '123' + setattr(record, 'trace', expected_trace) expected_span = '456' setattr(record, 'span_id', expected_span) expected_http = {'reuqest_url':'manual'} From d97f2d18dcc5ecc8354e32486600a7d212805f85 Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Fri, 11 Dec 2020 15:39:41 -0800 Subject: [PATCH 32/35] blacken --- .../cloud/logging_v2/handlers/app_engine.py | 10 ++--- google/cloud/logging_v2/handlers/handlers.py | 10 ++--- tests/system/test_system.py | 24 ++++++------ tests/unit/handlers/test_app_engine.py | 29 +++++++++----- tests/unit/handlers/test_handlers.py | 39 +++++++++++++------ 5 files changed, 68 insertions(+), 44 deletions(-) diff --git a/google/cloud/logging_v2/handlers/app_engine.py b/google/cloud/logging_v2/handlers/app_engine.py index 419d1b8f..a5d57c53 100644 --- a/google/cloud/logging_v2/handlers/app_engine.py +++ b/google/cloud/logging_v2/handlers/app_engine.py @@ -117,11 +117,11 @@ def emit(self, record): if inferred_trace is not None: inferred_trace = f"projects/{self.project_id}/traces/{inferred_trace}" # allow user overrides - trace = getattr(record, 'trace', inferred_trace) - span_id = getattr(record, 'span_id', None) - http_request = getattr(record, 'http_request', inferred_http) - resource = getattr(record, 'resource', self.resource) - user_labels = getattr(record, 'labels', {}) + trace = getattr(record, "trace", inferred_trace) + span_id = getattr(record, "span_id", None) + http_request = getattr(record, "http_request", inferred_http) + resource = getattr(record, "resource", self.resource) + user_labels = getattr(record, "labels", {}) # merge labels gae_labels = self.get_gae_labels() gae_labels.update(user_labels) diff --git a/google/cloud/logging_v2/handlers/handlers.py b/google/cloud/logging_v2/handlers/handlers.py index b3f16459..fd99f7ad 100644 --- a/google/cloud/logging_v2/handlers/handlers.py +++ b/google/cloud/logging_v2/handlers/handlers.py @@ -102,11 +102,11 @@ def emit(self, record): record (logging.LogRecord): The record to be logged. """ message = super(CloudLoggingHandler, self).format(record) - trace_id = getattr(record, 'trace', None) - span_id = getattr(record, 'span_id', None) - http_request = getattr(record, 'http_request', None) - resource = getattr(record, 'resource', self.resource) - user_labels = getattr(record, 'labels', {}) + trace_id = getattr(record, "trace", None) + span_id = getattr(record, "span_id", None) + http_request = getattr(record, "http_request", None) + resource = getattr(record, "resource", self.resource) + user_labels = getattr(record, "labels", {}) # merge labels total_labels = self.labels if self.labels is not None else {} total_labels.update(user_labels) diff --git a/tests/system/test_system.py b/tests/system/test_system.py index d75b639a..4ef403bb 100644 --- a/tests/system/test_system.py +++ b/tests/system/test_system.py @@ -316,9 +316,7 @@ def test_handlers_w_extras(self): LOGGER_NAME = f"{cls.__name__}-handler_extras" handler_name = self._logger_name(LOGGER_NAME) - handler = cls( - Config.CLIENT, name=handler_name, transport=SyncTransport - ) + handler = cls(Config.CLIENT, name=handler_name, transport=SyncTransport) # only create the logger to delete, hidden otherwise logger = Config.CLIENT.logger(handler.name) @@ -327,21 +325,21 @@ def test_handlers_w_extras(self): cloud_logger = logging.getLogger(LOGGER_NAME) cloud_logger.addHandler(handler) extra = { - 'trace': '123', - 'span_id': '456', - 'http_request': {'requestUrl':'manual'}, - 'resource': Resource(type="cloudiot_device", labels={}), - 'labels': {'test-label':'manual'} + "trace": "123", + "span_id": "456", + "http_request": {"requestUrl": "manual"}, + "resource": Resource(type="cloudiot_device", labels={}), + "labels": {"test-label": "manual"}, } cloud_logger.warn(LOG_MESSAGE, extra=extra) entries = _list_entries(logger) self.assertEqual(len(entries), 1) - self.assertEqual(entries[0].trace, extra['trace']) - self.assertEqual(entries[0].span_id, extra['span_id']) - self.assertEqual(entries[0].http_request, extra['http_request']) - self.assertEqual(entries[0].labels, extra['labels']) - self.assertEqual(entries[0].resource.type, extra['resource'].type) + self.assertEqual(entries[0].trace, extra["trace"]) + self.assertEqual(entries[0].span_id, extra["span_id"]) + self.assertEqual(entries[0].http_request, extra["http_request"]) + self.assertEqual(entries[0].labels, extra["labels"]) + self.assertEqual(entries[0].resource.type, extra["resource"].type) def test_log_root_handler(self): LOG_MESSAGE = "It was the best of times." diff --git a/tests/unit/handlers/test_app_engine.py b/tests/unit/handlers/test_app_engine.py index 904abc72..4f768b47 100644 --- a/tests/unit/handlers/test_app_engine.py +++ b/tests/unit/handlers/test_app_engine.py @@ -144,18 +144,18 @@ def test_emit_manual_field_override(self): ) handler.project_id = self.PROJECT # set attributes manually - expected_trace = '123' - setattr(record, 'trace', expected_trace) - expected_span = '456' - setattr(record, 'span_id', expected_span) - expected_http = {'reuqest_url':'manual'} - setattr(record, 'http_request', expected_http) + expected_trace = "123" + setattr(record, "trace", expected_trace) + expected_span = "456" + setattr(record, "span_id", expected_span) + expected_http = {"reuqest_url": "manual"} + setattr(record, "http_request", expected_http) expected_resource = gae_resource = Resource(type="test", labels={}) - setattr(record, 'resource', expected_resource) - additional_labels = {'test-label':'manual'} + setattr(record, "resource", expected_resource) + additional_labels = {"test-label": "manual"} expected_labels = dict(gae_labels) expected_labels.update(additional_labels) - setattr(record, 'labels', additional_labels) + setattr(record, "labels", additional_labels) handler.emit(record) self.assertIs(handler.transport.client, client) self.assertEqual(handler.transport.name, logname) @@ -200,10 +200,19 @@ def test_get_gae_labels_without_label(self): gae_labels = self._get_gae_labels_helper(None) self.assertEqual(gae_labels, {}) + class _Transport(object): def __init__(self, client, name): self.client = client self.name = name def send(self, record, message, resource, labels, trace, span_id, http_request): - self.send_called_with = (record, message, resource, labels, trace, span_id, http_request) + self.send_called_with = ( + record, + message, + resource, + labels, + trace, + span_id, + http_request, + ) diff --git a/tests/unit/handlers/test_handlers.py b/tests/unit/handlers/test_handlers.py index 0405656b..db5b0ca1 100644 --- a/tests/unit/handlers/test_handlers.py +++ b/tests/unit/handlers/test_handlers.py @@ -100,16 +100,16 @@ def test_emit_manual_field_override(self): message = "hello world" record = logging.LogRecord(logname, logging, None, None, message, None, None) # set attributes manually - expected_trace = '123' - setattr(record, 'trace', expected_trace) - expected_span = '456' - setattr(record, 'span_id', expected_span) - expected_http = {'reuqest_url':'manual'} - setattr(record, 'http_request', expected_http) + expected_trace = "123" + setattr(record, "trace", expected_trace) + expected_span = "456" + setattr(record, "span_id", expected_span) + expected_http = {"reuqest_url": "manual"} + setattr(record, "http_request", expected_http) expected_resource = gae_resource = Resource(type="test", labels={}) - setattr(record, 'resource', expected_resource) - expected_labels = {'test-label':'manual'} - setattr(record, 'labels', expected_labels) + setattr(record, "resource", expected_resource) + expected_labels = {"test-label": "manual"} + setattr(record, "labels", expected_labels) handler.emit(record) self.assertEqual( @@ -185,5 +185,22 @@ def __init__(self, client, name): self.client = client self.name = name - def send(self, record, message, resource, labels=None, trace=None, span_id=None, http_request=None): - self.send_called_with = (record, message, resource, labels, trace, span_id, http_request) + def send( + self, + record, + message, + resource, + labels=None, + trace=None, + span_id=None, + http_request=None, + ): + self.send_called_with = ( + record, + message, + resource, + labels, + trace, + span_id, + http_request, + ) From d67213d5495e85e4e16ff838e2cce7f18be8d97b Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Fri, 11 Dec 2020 16:37:32 -0800 Subject: [PATCH 33/35] fixed tests --- google/cloud/logging_v2/handlers/_helpers.py | 2 ++ tests/system/test_system.py | 5 +++-- tests/unit/handlers/test_app_engine.py | 6 +++--- tests/unit/handlers/test_handlers.py | 2 +- 4 files changed, 9 insertions(+), 6 deletions(-) diff --git a/google/cloud/logging_v2/handlers/_helpers.py b/google/cloud/logging_v2/handlers/_helpers.py index 8dc8c75a..f4bcda46 100644 --- a/google/cloud/logging_v2/handlers/_helpers.py +++ b/google/cloud/logging_v2/handlers/_helpers.py @@ -58,6 +58,8 @@ def get_request_data_from_flask(): str: TraceID in HTTP request headers. HttpRequest: data about the associated http request. """ + if flask is None or flask.request is None: + return None, None # build http_request http_request = HttpRequest( diff --git a/tests/system/test_system.py b/tests/system/test_system.py index 4ef403bb..dc578515 100644 --- a/tests/system/test_system.py +++ b/tests/system/test_system.py @@ -324,10 +324,11 @@ def test_handlers_w_extras(self): cloud_logger = logging.getLogger(LOGGER_NAME) cloud_logger.addHandler(handler) + expected_request = {"requestUrl": "localhost"} extra = { "trace": "123", "span_id": "456", - "http_request": {"requestUrl": "manual"}, + "http_request": expected_request, "resource": Resource(type="cloudiot_device", labels={}), "labels": {"test-label": "manual"}, } @@ -337,7 +338,7 @@ def test_handlers_w_extras(self): self.assertEqual(len(entries), 1) self.assertEqual(entries[0].trace, extra["trace"]) self.assertEqual(entries[0].span_id, extra["span_id"]) - self.assertEqual(entries[0].http_request, extra["http_request"]) + self.assertEqual(entries[0].http_request, expected_request) self.assertEqual(entries[0].labels, extra["labels"]) self.assertEqual(entries[0].resource.type, extra["resource"].type) diff --git a/tests/unit/handlers/test_app_engine.py b/tests/unit/handlers/test_app_engine.py index 4f768b47..1ac9c5dd 100644 --- a/tests/unit/handlers/test_app_engine.py +++ b/tests/unit/handlers/test_app_engine.py @@ -132,10 +132,10 @@ def test_emit_manual_field_override(self): "google.cloud.logging_v2.handlers.app_engine.get_request_data", return_value=(inferred_http_request, inferred_trace_id), ) - with get_request_patch as mock_get_request: + with get_request_patch: + # library integrations mocked to return test data client = mock.Mock(project=self.PROJECT, spec=["project"]) handler = self._make_one(client, transport=_Transport) - gae_resource = handler.get_gae_resource() gae_labels = handler.get_gae_labels() logname = "app" message = "hello world" @@ -150,7 +150,7 @@ def test_emit_manual_field_override(self): setattr(record, "span_id", expected_span) expected_http = {"reuqest_url": "manual"} setattr(record, "http_request", expected_http) - expected_resource = gae_resource = Resource(type="test", labels={}) + expected_resource = Resource(type="test", labels={}) setattr(record, "resource", expected_resource) additional_labels = {"test-label": "manual"} expected_labels = dict(gae_labels) diff --git a/tests/unit/handlers/test_handlers.py b/tests/unit/handlers/test_handlers.py index db5b0ca1..d84c1963 100644 --- a/tests/unit/handlers/test_handlers.py +++ b/tests/unit/handlers/test_handlers.py @@ -106,7 +106,7 @@ def test_emit_manual_field_override(self): setattr(record, "span_id", expected_span) expected_http = {"reuqest_url": "manual"} setattr(record, "http_request", expected_http) - expected_resource = gae_resource = Resource(type="test", labels={}) + expected_resource = Resource(type="test", labels={}) setattr(record, "resource", expected_resource) expected_labels = {"test-label": "manual"} setattr(record, "labels", expected_labels) From f995671705571403b8ac33362a9a803f39b458be Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Fri, 11 Dec 2020 16:43:47 -0800 Subject: [PATCH 34/35] fixed null check --- google/cloud/logging_v2/handlers/_helpers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/google/cloud/logging_v2/handlers/_helpers.py b/google/cloud/logging_v2/handlers/_helpers.py index f4bcda46..5f834a91 100644 --- a/google/cloud/logging_v2/handlers/_helpers.py +++ b/google/cloud/logging_v2/handlers/_helpers.py @@ -58,7 +58,7 @@ def get_request_data_from_flask(): str: TraceID in HTTP request headers. HttpRequest: data about the associated http request. """ - if flask is None or flask.request is None: + if flask is None or not flask.request: return None, None # build http_request From 2d4aff69a154c6681abd3f842b802eb57786ed60 Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Wed, 16 Dec 2020 17:40:22 -0800 Subject: [PATCH 35/35] fixed merge issues --- .../handlers/transports/background_thread.py | 1 - .../logging_v2/handlers/transports/base.py | 1 - tests/unit/handlers/test__helpers.py | 29 ------------------- tests/unit/handlers/test_app_engine.py | 1 - 4 files changed, 32 deletions(-) diff --git a/google/cloud/logging_v2/handlers/transports/background_thread.py b/google/cloud/logging_v2/handlers/transports/background_thread.py index 34ef67f3..3d654dbd 100644 --- a/google/cloud/logging_v2/handlers/transports/background_thread.py +++ b/google/cloud/logging_v2/handlers/transports/background_thread.py @@ -235,7 +235,6 @@ def enqueue(self, record, message, **kwargs): "info": {"message": message, "python_logger": record.name}, "severity": _helpers._normalize_severity(record.levelno), "timestamp": datetime.datetime.utcfromtimestamp(record.created), - "http_request": http_request, } queue_entry.update(kwargs) self._queue.put_nowait(queue_entry) diff --git a/google/cloud/logging_v2/handlers/transports/base.py b/google/cloud/logging_v2/handlers/transports/base.py index 192226b4..d60a5a07 100644 --- a/google/cloud/logging_v2/handlers/transports/base.py +++ b/google/cloud/logging_v2/handlers/transports/base.py @@ -22,7 +22,6 @@ class Transport(object): client and name object, and must override :meth:`send`. """ - def send(self, record, message, **kwargs): """Transport send to be implemented by subclasses. diff --git a/tests/unit/handlers/test__helpers.py b/tests/unit/handlers/test__helpers.py index 294d1221..8fb37305 100644 --- a/tests/unit/handlers/test__helpers.py +++ b/tests/unit/handlers/test__helpers.py @@ -105,35 +105,6 @@ def test_http_request_sparse(self): self.assertEqual(http_request.protocol, "HTTP/1.1") - app = self.create_app() - with app.test_client() as c: - c.put( - path=expected_path, - data=body_content, - environ_base={"REMOTE_ADDR": expected_ip}, - headers=headers, - ) - http_request, trace_id = self._call_fut() - - self.assertEqual(http_request.request_method, "PUT") - self.assertEqual(http_request.request_url, expected_path) - self.assertEqual(http_request.user_agent, expected_agent) - self.assertEqual(http_request.referer, expected_referrer) - self.assertEqual(http_request.remote_ip, expected_ip) - self.assertEqual(http_request.request_size, len(body_content)) - self.assertEqual(http_request.protocol, "HTTP/1.1") - - def test_http_request_sparse(self): - expected_path = "http://testserver/123" - app = self.create_app() - with app.test_client() as c: - c.put(path=expected_path) - http_request, trace_id = self._call_fut() - self.assertEqual(http_request.request_method, "PUT") - self.assertEqual(http_request.request_url, expected_path) - self.assertEqual(http_request.protocol, "HTTP/1.1") - - class Test_get_request_data_from_django(unittest.TestCase): @staticmethod def _call_fut(): diff --git a/tests/unit/handlers/test_app_engine.py b/tests/unit/handlers/test_app_engine.py index d50cade4..1ac9c5dd 100644 --- a/tests/unit/handlers/test_app_engine.py +++ b/tests/unit/handlers/test_app_engine.py @@ -172,7 +172,6 @@ def test_emit_manual_field_override(self): ), ) - def _get_gae_labels_helper(self, trace_id): get_request_patch = mock.patch( "google.cloud.logging_v2.handlers.app_engine.get_request_data",