Skip to content

Commit 9369fdd

Browse files
CagriYoncaCagriYonca
committed
fix: Response class doesn't have exception attribute. Fixes: #819
Signed-off-by: Cagri Yonca <[email protected]>
1 parent 6bc03da commit 9369fdd

File tree

3 files changed

+107
-33
lines changed

3 files changed

+107
-33
lines changed

‎src/instana/instrumentation/pyramid.py‎

Lines changed: 19 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,28 +1,28 @@
11
# (c) Copyright IBM Corp. 2021
22
# (c) Copyright Instana Inc. 2020
33

4+
45
try:
6+
from typing import TYPE_CHECKING, Any, Callable, Dict, Optional, Tuple, Union
7+
8+
import wrapt
9+
from opentelemetry.semconv.trace import SpanAttributes
10+
from pyramid.config import Configurator
511
from pyramid.httpexceptions import HTTPException
612
from pyramid.path import caller_package
713
from pyramid.settings import aslist
814
from pyramid.tweens import EXCVIEW
9-
from pyramid.config import Configurator
10-
from typing import TYPE_CHECKING, Dict, Any, Callable, Tuple
11-
import wrapt
12-
13-
from opentelemetry.semconv.trace import SpanAttributes
14-
from opentelemetry.trace import SpanKind
1515

1616
from instana.log import logger
17-
from instana.singletons import tracer, agent
17+
from instana.propagators.format import Format
18+
from instana.singletons import agent, tracer
1819
from instana.util.secrets import strip_secrets_from_query
1920
from instana.util.traceutils import extract_custom_headers
20-
from instana.propagators.format import Format
2121

2222
if TYPE_CHECKING:
23+
from pyramid.registry import Registry
2324
from pyramid.request import Request
2425
from pyramid.response import Response
25-
from pyramid.registry import Registry
2626

2727
class InstanaTweenFactory(object):
2828
"""A factory that provides Instana instrumentation tween for Pyramid apps"""
@@ -32,11 +32,11 @@ def __init__(
3232
) -> None:
3333
self.handler = handler
3434

35-
def __call__(self, request: "Request") -> "Response":
35+
def __call__(self, request: "Request") -> Optional["Response"]:
3636
ctx = tracer.extract(Format.HTTP_HEADERS, dict(request.headers))
3737

3838
with tracer.start_as_current_span("wsgi", span_context=ctx) as span:
39-
span.set_attribute("http.host", request.host)
39+
span.set_attribute(SpanAttributes.HTTP_HOST, request.host)
4040
span.set_attribute(SpanAttributes.HTTP_METHOD, request.method)
4141
span.set_attribute(SpanAttributes.HTTP_URL, request.path)
4242

@@ -57,9 +57,7 @@ def __call__(self, request: "Request") -> "Response":
5757
span.set_attribute(
5858
"http.path_tpl", request.matched_route.pattern
5959
)
60-
6160
extract_custom_headers(span, response.headers)
62-
6361
tracer.inject(span.context, Format.HTTP_HEADERS, response.headers)
6462
except HTTPException as e:
6563
response = e
@@ -69,7 +67,6 @@ def __call__(self, request: "Request") -> "Response":
6967
except BaseException as e:
7068
span.set_attribute(SpanAttributes.HTTP_STATUS_CODE, 500)
7169
span.record_exception(e)
72-
7370
logger.debug(
7471
"Pyramid InstanaTweenFactory BaseException: ", exc_info=True
7572
)
@@ -78,16 +75,18 @@ def __call__(self, request: "Request") -> "Response":
7875
span.set_attribute(
7976
SpanAttributes.HTTP_STATUS_CODE, response.status_int
8077
)
81-
82-
if 500 <= response.status_int:
83-
if response.exception:
84-
span.record_exception(response.exception)
85-
span.assure_errored()
86-
78+
if response.status_code >= 500:
79+
handle_exception(span, response)
8780
return response
8881

8982
INSTANA_TWEEN = __name__ + ".InstanaTweenFactory"
9083

84+
def handle_exception(span, response: Union["Response", HTTPException]) -> None:
85+
if isinstance(response, HTTPException):
86+
span.record_exception(response.exception)
87+
else:
88+
span.record_exception(response.body)
89+
9190
# implicit tween ordering
9291
def includeme(config: Configurator) -> None:
9392
logger.debug("Instrumenting pyramid")

‎tests/apps/pyramid/pyramid_app/app.py‎

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
# (c) Copyright IBM Corp. 2021
22
# (c) Copyright Instana Inc. 2020
33

4-
from wsgiref.simple_server import make_server
5-
from pyramid.config import Configurator
64
import logging
5+
from wsgiref.simple_server import make_server
76

8-
from pyramid.response import Response
97
import pyramid.httpexceptions as exc
8+
from pyramid.config import Configurator
9+
from pyramid.response import Response
1010

1111
from tests.helpers import testenv
1212

@@ -25,6 +25,10 @@ def please_fail(request):
2525
raise exc.HTTPInternalServerError("internal error")
2626

2727

28+
def fail_with_http_exception(request):
29+
raise exc.HTTPException("bad request")
30+
31+
2832
def tableflip(request):
2933
raise BaseException("fake exception")
3034

@@ -39,6 +43,10 @@ def hello_user(request):
3943
return Response(f"Hello {user}!")
4044

4145

46+
def return_error_response(request):
47+
return Response("Error", status=500)
48+
49+
4250
app = None
4351
settings = {
4452
"pyramid.tweens": "tests.apps.pyramid.pyramid_utils.tweens.timing_tween_factory",
@@ -48,12 +56,16 @@ def hello_user(request):
4856
config.add_view(hello_world, route_name="hello")
4957
config.add_route("fail", "/500")
5058
config.add_view(please_fail, route_name="fail")
59+
config.add_route("fail_with_http_exception", "/fail_with_http_exception")
60+
config.add_view(fail_with_http_exception, route_name="fail_with_http_exception")
5161
config.add_route("crash", "/exception")
5262
config.add_view(tableflip, route_name="crash")
5363
config.add_route("response_headers", "/response_headers")
5464
config.add_view(response_headers, route_name="response_headers")
5565
config.add_route("hello_user", "/hello_user/{user}")
5666
config.add_view(hello_user, route_name="hello_user")
67+
config.add_route(name="return_error_response", pattern="/return_error_response")
68+
config.add_view(return_error_response, route_name="return_error_response")
5769
app = config.make_wsgi_app()
5870

5971
pyramid_server = make_server("127.0.0.1", testenv["pyramid_port"], app)

‎tests/frameworks/test_pyramid.py‎

Lines changed: 73 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,16 @@
11
# (c) Copyright IBM Corp. 2021
22
# (c) Copyright Instana Inc. 2020
33

4+
from typing import Generator
5+
46
import pytest
57
import urllib3
6-
from typing import Generator
78

9+
import tests.apps.pyramid.pyramid_app # noqa: F401
10+
from instana.singletons import agent, tracer
11+
from instana.span.span import get_current_span
812
from instana.util.ids import hex_id
9-
import tests.apps.pyramid.pyramid_app
1013
from tests.helpers import testenv
11-
from instana.singletons import tracer, agent
12-
from instana.span.span import get_current_span
1314

1415

1516
class TestPyramid:
@@ -77,7 +78,9 @@ def test_get_request(self) -> None:
7778

7879
# wsgi
7980
assert pyramid_span.n == "wsgi"
80-
assert pyramid_span.data["http"]["host"] == "127.0.0.1:" + str(testenv["pyramid_port"])
81+
assert pyramid_span.data["http"]["host"] == "127.0.0.1:" + str(
82+
testenv["pyramid_port"]
83+
)
8184
assert pyramid_span.data["http"]["url"] == "/"
8285
assert pyramid_span.data["http"]["method"] == "GET"
8386
assert pyramid_span.data["http"]["status"] == 200
@@ -161,7 +164,9 @@ def test_500(self) -> None:
161164

162165
# wsgi
163166
assert pyramid_span.n == "wsgi"
164-
assert pyramid_span.data["http"]["host"] == "127.0.0.1:" + str(testenv["pyramid_port"])
167+
assert pyramid_span.data["http"]["host"] == "127.0.0.1:" + str(
168+
testenv["pyramid_port"]
169+
)
165170
assert pyramid_span.data["http"]["url"] == "/500"
166171
assert pyramid_span.data["http"]["method"] == "GET"
167172
assert pyramid_span.data["http"]["status"] == 500
@@ -178,6 +183,56 @@ def test_500(self) -> None:
178183
assert type(urllib3_span.stack) is list
179184
assert len(urllib3_span.stack) > 1
180185

186+
def test_return_error_response(self) -> None:
187+
with tracer.start_as_current_span("test"):
188+
response = self.http.request(
189+
"GET", testenv["pyramid_server"] + "/return_error_response"
190+
)
191+
192+
spans = self.recorder.queued_spans()
193+
assert len(spans) == 3
194+
195+
pyramid_span = spans[0]
196+
197+
assert response.status == 500
198+
199+
assert pyramid_span.n == "wsgi"
200+
assert pyramid_span.data["http"]["host"] == "127.0.0.1:" + str(
201+
testenv["pyramid_port"]
202+
)
203+
assert pyramid_span.data["http"]["url"] == "/return_error_response"
204+
assert pyramid_span.data["http"]["method"] == "GET"
205+
assert pyramid_span.data["http"]["status"] == 500
206+
assert pyramid_span.data["http"]["error"] == "b'Error'"
207+
assert pyramid_span.data["http"]["path_tpl"] == "/return_error_response"
208+
209+
assert pyramid_span.ec == 1
210+
211+
def test_fail_with_http_exception(self) -> None:
212+
with tracer.start_as_current_span("test"):
213+
response = self.http.request(
214+
"GET", testenv["pyramid_server"] + "/fail_with_http_exception"
215+
)
216+
217+
spans = self.recorder.queued_spans()
218+
assert len(spans) == 3
219+
220+
pyramid_span = spans[0]
221+
222+
assert response.status == 520
223+
224+
assert pyramid_span.n == "wsgi"
225+
assert pyramid_span.data["http"]["host"] == "127.0.0.1:" + str(
226+
testenv["pyramid_port"]
227+
)
228+
assert pyramid_span.data["http"]["url"] == "/fail_with_http_exception"
229+
assert pyramid_span.data["http"]["method"] == "GET"
230+
assert pyramid_span.data["http"]["status"] == 520
231+
assert pyramid_span.data["http"]["error"] == "bad request"
232+
assert pyramid_span.data["http"]["path_tpl"] == "/fail_with_http_exception"
233+
234+
assert pyramid_span.ec == 1
235+
181236
def test_exception(self) -> None:
182237
with tracer.start_as_current_span("test"):
183238
response = self.http.request(
@@ -211,7 +266,9 @@ def test_exception(self) -> None:
211266

212267
# wsgi
213268
assert pyramid_span.n == "wsgi"
214-
assert pyramid_span.data["http"]["host"] == "127.0.0.1:" + str(testenv["pyramid_port"])
269+
assert pyramid_span.data["http"]["host"] == "127.0.0.1:" + str(
270+
testenv["pyramid_port"]
271+
)
215272
assert pyramid_span.data["http"]["url"] == "/exception"
216273
assert pyramid_span.data["http"]["method"] == "GET"
217274
assert pyramid_span.data["http"]["status"] == 500
@@ -270,7 +327,9 @@ def test_response_header_capture(self) -> None:
270327

271328
# wsgi
272329
assert pyramid_span.n == "wsgi"
273-
assert pyramid_span.data["http"]["host"] == "127.0.0.1:" + str(testenv["pyramid_port"])
330+
assert pyramid_span.data["http"]["host"] == "127.0.0.1:" + str(
331+
testenv["pyramid_port"]
332+
)
274333
assert pyramid_span.data["http"]["url"] == "/response_headers"
275334
assert pyramid_span.data["http"]["method"] == "GET"
276335
assert pyramid_span.data["http"]["status"] == 200
@@ -341,7 +400,9 @@ def test_request_header_capture(self) -> None:
341400

342401
# wsgi
343402
assert pyramid_span.n == "wsgi"
344-
assert pyramid_span.data["http"]["host"] == "127.0.0.1:" + str(testenv["pyramid_port"])
403+
assert pyramid_span.data["http"]["host"] == "127.0.0.1:" + str(
404+
testenv["pyramid_port"]
405+
)
345406
assert pyramid_span.data["http"]["url"] == "/"
346407
assert pyramid_span.data["http"]["method"] == "GET"
347408
assert pyramid_span.data["http"]["status"] == 200
@@ -419,7 +480,9 @@ def test_scrub_secret_path_template(self) -> None:
419480

420481
# wsgi
421482
assert pyramid_span.n == "wsgi"
422-
assert pyramid_span.data["http"]["host"] == "127.0.0.1:" + str(testenv["pyramid_port"])
483+
assert pyramid_span.data["http"]["host"] == "127.0.0.1:" + str(
484+
testenv["pyramid_port"]
485+
)
423486
assert pyramid_span.data["http"]["url"] == "/hello_user/oswald"
424487
assert pyramid_span.data["http"]["method"] == "GET"
425488
assert pyramid_span.data["http"]["status"] == 200

0 commit comments

Comments
 (0)