Skip to content
This repository was archived by the owner on Sep 17, 2025. It is now read-only.

Fixed requests contrib to raise original exceptions #771

Merged
reyang merged 3 commits into from
Aug 22, 2019
Merged

Fixed requests contrib to raise original exceptions #771

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4399dd7
Fixed requests contrib to raise original exceptions
victoraugustolls Aug 22, 2019
7c014b2
Update CHANGELOG.md
victoraugustolls Aug 22, 2019
4dd5ad9
Update CHANGELOG.md
victoraugustolls Aug 22, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
# Changelog

## Unreleased
- Updated `django`, `flask`, `httplib`, `requests` and `pyramid` modules.
- Updated `django`, `flask`, `httplib`, `requests` and `pyramid` modules
([#755](https://github.com/census-instrumentation/opencensus-python/pull/755))
- Added `http code` to `grpc code` status code mapping on `utils`
([#746](https://github.com/census-instrumentation/opencensus-python/pull/746))
- Updated `requests` module
([#771](https://github.com/census-instrumentation/opencensus-python/pull/771))
Copy link
Copy Markdown
Contributor

@lzchen lzchen Aug 22, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to update the CHANGELOG for the core sdk?

Copy link
Copy Markdown
Contributor Author

@victoraugustolls victoraugustolls Aug 22, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is just for tracking for the next release, so we don’t forget to update some module. I remember someone suggested this recently, but I can’t remember in which PR. It does not need to go into the release changelog (in my opinion), but this way we know what needs to be updated !

Copy link
Copy Markdown
Contributor

@lzchen lzchen Aug 22, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. As long as we are consistent, I feel like it should be fine. So far it seems like we only update the changelog if there are changes in the actual package but your argument for it being a "release reminder" is valid :).


## 0.7.2
Released 2019-08-16
Expand Down
2 changes: 2 additions & 0 deletions contrib/opencensus-ext-requests/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
([#746](https://github.com/census-instrumentation/opencensus-python/pull/746))
- Fixed span name
([#746](https://github.com/census-instrumentation/opencensus-python/pull/746))
- Fixed exception handling
([#771](https://github.com/census-instrumentation/opencensus-python/pull/771))

## 0.7.1
Released 2019-08-06
Expand Down
6 changes: 6 additions & 0 deletions contrib/opencensus-ext-requests/opencensus/ext/requests/trace.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,10 +105,13 @@ def call(url, *args, **kwargs):
result = requests_func(url, *args, **kwargs)
except requests.Timeout:
_span.set_status(exceptions_status.TIMEOUT)
raise
except requests.URLRequired:
_span.set_status(exceptions_status.INVALID_URL)
raise
except Exception as e:
_span.set_status(exceptions_status.unknown(e))
raise
else:
# Add the status code to attributes
_tracer.add_attribute_to_current_span(
Expand Down Expand Up @@ -178,10 +181,13 @@ def wrap_session_request(wrapped, instance, args, kwargs):
result = wrapped(*args, **kwargs)
except requests.Timeout:
_span.set_status(exceptions_status.TIMEOUT)
raise
except requests.URLRequired:
_span.set_status(exceptions_status.INVALID_URL)
raise
except Exception as e:
_span.set_status(exceptions_status.unknown(e))
raise
else:
# Add the status code to attributes
_tracer.add_attribute_to_current_span(
Expand Down
76 changes: 53 additions & 23 deletions contrib/opencensus-ext-requests/tests/test_requests_trace.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,8 @@ def test_wrap_requests_timeout(self):
url = 'http://localhost:8080/test'

with patch, patch_thread:
wrapped(url)
with self.assertRaises(requests.Timeout):
wrapped(url)

expected_attributes = {
'http.host': 'localhost:8080',
Expand Down Expand Up @@ -288,7 +289,8 @@ def test_wrap_requests_invalid_url(self):
url = 'http://localhost:8080/test'

with patch, patch_thread:
wrapped(url)
with self.assertRaises(requests.URLRequired):
wrapped(url)

expected_attributes = {
'http.host': 'localhost:8080',
Expand All @@ -308,6 +310,7 @@ def test_wrap_requests_invalid_url(self):
expected_status.__dict__,
mock_tracer.current_span.status.__dict__
)
self.assertRaises(requests.URLRequired, mock_func)

def test_wrap_requests_exception(self):
mock_return = mock.Mock()
Expand All @@ -334,7 +337,8 @@ def test_wrap_requests_exception(self):
url = 'http://localhost:8080/test'

with patch, patch_thread:
wrapped(url)
with self.assertRaises(requests.TooManyRedirects):
wrapped(url)

expected_attributes = {
'http.host': 'localhost:8080',
Expand All @@ -354,6 +358,7 @@ def test_wrap_requests_exception(self):
expected_status.__dict__,
mock_tracer.current_span.status.__dict__
)
self.assertRaises(requests.TooManyRedirects, mock_func)

def test_wrap_session_request(self):
wrapped = mock.Mock(return_value=mock.Mock(status_code=200))
Expand All @@ -376,8 +381,10 @@ def test_wrap_session_request(self):
kwargs = {}

with patch, patch_thread:
trace.wrap_session_request(wrapped, 'Session.request',
(request_method, url), kwargs)
trace.wrap_session_request(
wrapped, 'Session.request',
(request_method, url), kwargs
)

expected_attributes = {
'http.host': 'localhost:8080',
Expand Down Expand Up @@ -427,8 +434,10 @@ def wrapped(*args, **kwargs):
request_method = 'POST'

with patch_tracer, patch_attr, patch_thread:
trace.wrap_session_request(wrapped, 'Session.request',
(request_method, url), {})
trace.wrap_session_request(
wrapped, 'Session.request',
(request_method, url), {}
)

expected_name = '/'
self.assertEqual(expected_name, mock_tracer.current_span.name)
Expand Down Expand Up @@ -458,8 +467,11 @@ def wrapped(*args, **kwargs):
request_method = 'POST'

with patch_tracer, patch_attr, patch_thread:
trace.wrap_session_request(wrapped, 'Session.request',
(request_method, url), {})
trace.wrap_session_request(
wrapped, 'Session.request',
(request_method, url), {}
)

self.assertEqual(None, mock_tracer.current_span)

def test_wrap_session_request_exporter_thread(self):
Expand Down Expand Up @@ -487,8 +499,11 @@ def wrapped(*args, **kwargs):
request_method = 'POST'

with patch_tracer, patch_attr, patch_thread:
trace.wrap_session_request(wrapped, 'Session.request',
(request_method, url), {})
trace.wrap_session_request(
wrapped, 'Session.request',
(request_method, url), {}
)

self.assertEqual(None, mock_tracer.current_span)

def test_header_is_passed_in(self):
Expand All @@ -511,8 +526,10 @@ def test_header_is_passed_in(self):
kwargs = {}

with patch, patch_thread:
trace.wrap_session_request(wrapped, 'Session.request',
(request_method, url), kwargs)
trace.wrap_session_request(
wrapped, 'Session.request',
(request_method, url), kwargs
)

self.assertEqual(kwargs['headers']['x-trace'], 'some-value')

Expand All @@ -536,8 +553,10 @@ def test_headers_are_preserved(self):
kwargs = {'headers': {'key': 'value'}}

with patch, patch_thread:
trace.wrap_session_request(wrapped, 'Session.request',
(request_method, url), kwargs)
trace.wrap_session_request(
wrapped, 'Session.request',
(request_method, url), kwargs
)

self.assertEqual(kwargs['headers']['key'], 'value')
self.assertEqual(kwargs['headers']['x-trace'], 'some-value')
Expand All @@ -563,8 +582,10 @@ def test_tracer_headers_are_overwritten(self):
kwargs = {'headers': {'x-trace': 'original-value'}}

with patch, patch_thread:
trace.wrap_session_request(wrapped, 'Session.request',
(request_method, url), kwargs)
trace.wrap_session_request(
wrapped, 'Session.request',
(request_method, url), kwargs
)

self.assertEqual(kwargs['headers']['x-trace'], 'some-value')

Expand All @@ -590,8 +611,11 @@ def test_wrap_session_request_timeout(self):
kwargs = {}

with patch, patch_thread:
trace.wrap_session_request(wrapped, 'Session.request',
(request_method, url), kwargs)
with self.assertRaises(requests.Timeout):
trace.wrap_session_request(
wrapped, 'Session.request',
(request_method, url), kwargs
)

expected_attributes = {
'http.host': 'localhost:8080',
Expand Down Expand Up @@ -635,8 +659,11 @@ def test_wrap_session_request_invalid_url(self):
kwargs = {}

with patch, patch_thread:
trace.wrap_session_request(wrapped, 'Session.request',
(request_method, url), kwargs)
with self.assertRaises(requests.URLRequired):
trace.wrap_session_request(
wrapped, 'Session.request',
(request_method, url), kwargs
)

expected_attributes = {
'http.host': 'localhost:8080',
Expand Down Expand Up @@ -680,8 +707,11 @@ def test_wrap_session_request_exception(self):
kwargs = {}

with patch, patch_thread:
trace.wrap_session_request(wrapped, 'Session.request',
(request_method, url), kwargs)
with self.assertRaises(requests.TooManyRedirects):
trace.wrap_session_request(
wrapped, 'Session.request',
(request_method, url), kwargs
)

expected_attributes = {
'http.host': 'localhost:8080',
Expand Down