Skip to content

Commit 6d1974d

Browse files
daniel-sanchedaniel-sanche
committed
use proto instead of dict for http_request
1 parent 5bb9e51 commit 6d1974d

File tree

2 files changed

+47
-76
lines changed

2 files changed

+47
-76
lines changed

‎google/cloud/logging_v2/handlers/_helpers.py‎

Lines changed: 22 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,9 @@
2323
flask = None
2424

2525
from google.cloud.logging_v2.handlers.middleware.request import _get_django_request
26+
from google.logging.type.http_request_pb2 import HttpRequest
2627

2728
_DJANGO_TRACE_HEADER = "HTTP_X_CLOUD_TRACE_CONTEXT"
28-
_DJANGO_LENGTH_HEADER = "CONTENT_LENGTH"
2929
_DJANGO_USERAGENT_HEADER = "HTTP_USER_AGENT"
3030
_DJANGO_REMOTE_ADDR_HEADER = "REMOTE_ADDR"
3131
_DJANGO_REFERER_HEADER = "HTTP_REFERER"
@@ -56,22 +56,19 @@ def get_request_data_from_flask():
5656
5757
Returns:
5858
str: TraceID in HTTP request headers.
59-
dict: data about the associated http request.
59+
HttpRequest: data about the associated http request.
6060
"""
61-
if flask is None or not flask.request:
62-
return None, None
6361

6462
# build http_request
65-
length = flask.request.content_length
66-
http_request = {
67-
"request_method": flask.request.method,
68-
"request_url": flask.request.url,
69-
"request_size": str(length) if length else None,
70-
"user_agent": flask.request.user_agent.string,
71-
"remote_ip": flask.request.remote_addr,
72-
"referer": flask.request.referrer,
73-
"protocol": flask.request.environ.get(_PROTOCOL_HEADER),
74-
}
63+
http_request = HttpRequest(
64+
request_method=flask.request.method,
65+
request_url=flask.request.url,
66+
request_size=flask.request.content_length,
67+
user_agent=flask.request.user_agent.string,
68+
remote_ip=flask.request.remote_addr,
69+
referer=flask.request.referrer,
70+
protocol=flask.request.environ.get(_PROTOCOL_HEADER),
71+
)
7572

7673
# find trace id
7774
trace_id = None
@@ -87,23 +84,22 @@ def get_request_data_from_django():
8784
8885
Returns:
8986
str: TraceID in HTTP request headers.
90-
dict: data about the associated http request.
87+
HttpRequest: data about the associated http request.
9188
"""
9289
request = _get_django_request()
9390

9491
if request is None:
9592
return None, None
96-
9793
# build http_request
98-
http_request = {
99-
"request_method": request.method,
100-
"request_url": request.build_absolute_uri(),
101-
"request_size": request.META.get(_DJANGO_LENGTH_HEADER),
102-
"user_agent": request.META.get(_DJANGO_USERAGENT_HEADER),
103-
"remote_ip": request.META.get(_DJANGO_REMOTE_ADDR_HEADER),
104-
"referer": request.META.get(_DJANGO_REFERER_HEADER),
105-
"protocol": request.META.get(_PROTOCOL_HEADER),
106-
}
94+
http_request = HttpRequest(
95+
request_method=request.method,
96+
request_url=request.build_absolute_uri(),
97+
request_size=len(request.body),
98+
user_agent=request.META.get(_DJANGO_USERAGENT_HEADER),
99+
remote_ip=request.META.get(_DJANGO_REMOTE_ADDR_HEADER),
100+
referer=request.META.get(_DJANGO_REFERER_HEADER),
101+
protocol=request.META.get(_PROTOCOL_HEADER),
102+
)
107103

108104
# find trace id
109105
trace_id = None
@@ -120,7 +116,7 @@ def get_request_data():
120116
121117
Returns:
122118
str: TraceID in HTTP request headers.
123-
dict: data about the associated http request.
119+
HttpRequest: data about the associated http request.
124120
"""
125121
checkers = (
126122
get_request_data_from_django,

‎tests/unit/handlers/test__helpers.py‎

Lines changed: 25 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -20,15 +20,6 @@
2020
_FLASK_HTTP_REQUEST = {"request_url": "https://flask.palletsprojects.com/en/1.1.x/"}
2121
_DJANGO_TRACE_ID = "django-id"
2222
_DJANGO_HTTP_REQUEST = {"request_url": "https://www.djangoproject.com/"}
23-
_HTTP_REQUEST_FIELDS = [
24-
"request_method",
25-
"request_url",
26-
"request_size",
27-
"user_agent",
28-
"remote_ip",
29-
"referer",
30-
"protocol",
31-
]
3223

3324

3425
class Test_get_request_data_from_flask(unittest.TestCase):
@@ -56,10 +47,7 @@ def test_no_context_header(self):
5647
http_request, trace_id = self._call_fut()
5748

5849
self.assertIsNone(trace_id)
59-
self.assertEqual(http_request["request_method"], "GET")
60-
self.assertEqual(set(http_request.keys()), set(_HTTP_REQUEST_FIELDS))
61-
for field in _HTTP_REQUEST_FIELDS:
62-
self.assertTrue(field in http_request)
50+
self.assertEqual(http_request.request_method, "GET")
6351

6452
def test_valid_context_header(self):
6553
flask_trace_header = "X_CLOUD_TRACE_CONTEXT"
@@ -75,8 +63,7 @@ def test_valid_context_header(self):
7563
http_request, trace_id = self._call_fut()
7664

7765
self.assertEqual(trace_id, expected_trace_id)
78-
self.assertEqual(http_request["request_method"], "GET")
79-
self.assertEqual(set(http_request.keys()), set(_HTTP_REQUEST_FIELDS))
66+
self.assertEqual(http_request.request_method, "GET")
8067

8168
def test_http_request_populated(self):
8269
expected_path = "http://testserver/123"
@@ -99,27 +86,23 @@ def test_http_request_populated(self):
9986
)
10087
http_request, trace_id = self._call_fut()
10188

102-
self.assertEqual(http_request["request_method"], "PUT")
103-
self.assertEqual(http_request["request_url"], expected_path)
104-
self.assertEqual(http_request["user_agent"], expected_agent)
105-
self.assertEqual(http_request["referer"], expected_referrer)
106-
self.assertEqual(http_request["remote_ip"], expected_ip)
107-
self.assertEqual(http_request["request_size"], str(len(body_content)))
108-
self.assertEqual(http_request["protocol"], "HTTP/1.1")
109-
self.assertEqual(set(http_request.keys()), set(_HTTP_REQUEST_FIELDS))
89+
self.assertEqual(http_request.request_method, "PUT")
90+
self.assertEqual(http_request.request_url, expected_path)
91+
self.assertEqual(http_request.user_agent, expected_agent)
92+
self.assertEqual(http_request.referer, expected_referrer)
93+
self.assertEqual(http_request.remote_ip, expected_ip)
94+
self.assertEqual(http_request.request_size, len(body_content))
95+
self.assertEqual(http_request.protocol, "HTTP/1.1")
11096

11197
def test_http_request_sparse(self):
11298
expected_path = "http://testserver/123"
11399
app = self.create_app()
114100
with app.test_client() as c:
115101
c.put(path=expected_path)
116102
http_request, trace_id = self._call_fut()
117-
self.assertEqual(http_request["request_method"], "PUT")
118-
self.assertEqual(http_request["request_url"], expected_path)
119-
self.assertEqual(http_request["protocol"], "HTTP/1.1")
120-
self.assertIsNone(http_request["referer"])
121-
self.assertIsNone(http_request["request_size"])
122-
self.assertEqual(set(http_request.keys()), set(_HTTP_REQUEST_FIELDS))
103+
self.assertEqual(http_request.request_method, "PUT")
104+
self.assertEqual(http_request.request_url, expected_path)
105+
self.assertEqual(http_request.protocol, "HTTP/1.1")
123106

124107

125108
class Test_get_request_data_from_django(unittest.TestCase):
@@ -153,8 +136,7 @@ def test_no_context_header(self):
153136
middleware = request.RequestMiddleware(None)
154137
middleware.process_request(django_request)
155138
http_request, trace_id = self._call_fut()
156-
self.assertEqual(http_request["request_method"], "GET")
157-
self.assertEqual(set(http_request.keys()), set(_HTTP_REQUEST_FIELDS))
139+
self.assertEqual(http_request.request_method, "GET")
158140
self.assertIsNone(trace_id)
159141

160142
def test_valid_context_header(self):
@@ -174,10 +156,7 @@ def test_valid_context_header(self):
174156
http_request, trace_id = self._call_fut()
175157

176158
self.assertEqual(trace_id, expected_trace_id)
177-
self.assertEqual(http_request["request_method"], "GET")
178-
self.assertEqual(set(http_request.keys()), set(_HTTP_REQUEST_FIELDS))
179-
for field in _HTTP_REQUEST_FIELDS:
180-
self.assertTrue(field in http_request)
159+
self.assertEqual(http_request.request_method, "GET")
181160

182161
def test_http_request_populated(self):
183162
from django.test import RequestFactory
@@ -197,14 +176,13 @@ def test_http_request_populated(self):
197176
middleware = request.RequestMiddleware(None)
198177
middleware.process_request(django_request)
199178
http_request, trace_id = self._call_fut()
200-
self.assertEqual(http_request["request_method"], "PUT")
201-
self.assertEqual(http_request["request_url"], expected_path)
202-
self.assertEqual(http_request["user_agent"], expected_agent)
203-
self.assertEqual(http_request["referer"], expected_referrer)
204-
self.assertEqual(http_request["remote_ip"], "127.0.0.1")
205-
self.assertEqual(http_request["request_size"], str(len(body_content)))
206-
self.assertEqual(http_request["protocol"], "HTTP/1.1")
207-
self.assertEqual(set(http_request.keys()), set(_HTTP_REQUEST_FIELDS))
179+
self.assertEqual(http_request.request_method, "PUT")
180+
self.assertEqual(http_request.request_url, expected_path)
181+
self.assertEqual(http_request.user_agent, expected_agent)
182+
self.assertEqual(http_request.referer, expected_referrer)
183+
self.assertEqual(http_request.remote_ip, "127.0.0.1")
184+
self.assertEqual(http_request.request_size, len(body_content))
185+
self.assertEqual(http_request.protocol, "HTTP/1.1")
208186

209187
def test_http_request_sparse(self):
210188
from django.test import RequestFactory
@@ -215,13 +193,10 @@ def test_http_request_sparse(self):
215193
middleware = request.RequestMiddleware(None)
216194
middleware.process_request(django_request)
217195
http_request, trace_id = self._call_fut()
218-
self.assertEqual(http_request["request_method"], "PUT")
219-
self.assertEqual(http_request["request_url"], expected_path)
220-
self.assertIsNone(http_request["referer"])
221-
self.assertEqual(http_request["remote_ip"], "127.0.0.1")
222-
self.assertIsNone(http_request["request_size"])
223-
self.assertEqual(http_request["protocol"], "HTTP/1.1")
224-
self.assertEqual(set(http_request.keys()), set(_HTTP_REQUEST_FIELDS))
196+
self.assertEqual(http_request.request_method, "PUT")
197+
self.assertEqual(http_request.request_url, expected_path)
198+
self.assertEqual(http_request.remote_ip, "127.0.0.1")
199+
self.assertEqual(http_request.protocol, "HTTP/1.1")
225200

226201

227202
class Test_get_request_data(unittest.TestCase):

0 commit comments

Comments
 (0)