From e88c887f2a5859d0b5186d7935dbe36a3b00fb2a Mon Sep 17 00:00:00 2001 From: Pau Freixes Date: Thu, 10 May 2018 16:26:14 +0200 Subject: [PATCH 1/4] New flag `raise_if_not_subsegment` for Aiottp client Without setting this flag as False, any HTTP call done with the Aiohttp Client when there is no an active segment will fail with the SegmentNotFoundException exception. This issue can be annoying for this systems that do not create explicitly the segements and relay on the sampled incomming requests, which create automatically segments just for a sub set of the incomming request. --- aws_xray_sdk/ext/aiohttp/client.py | 46 ++++++++++++++++++++++++------ tests/ext/aiohttp/test_client.py | 23 +++++++++++++++ 2 files changed, 61 insertions(+), 8 deletions(-) diff --git a/aws_xray_sdk/ext/aiohttp/client.py b/aws_xray_sdk/ext/aiohttp/client.py index ed9fbf32..53d1d9be 100644 --- a/aws_xray_sdk/ext/aiohttp/client.py +++ b/aws_xray_sdk/ext/aiohttp/client.py @@ -9,6 +9,7 @@ from aws_xray_sdk.core import xray_recorder from aws_xray_sdk.core.models import http from aws_xray_sdk.ext.util import inject_trace_header, strip_url +from aws_xray_sdk.core.exceptions.exceptions import SegmentNotFoundException # All aiohttp calls will entail outgoing HTTP requests, only in some ad-hoc # exceptions the namespace will be flip back to local. @@ -23,20 +24,41 @@ async def begin_subsegment(session, trace_config_ctx, params): name = trace_config_ctx.name if trace_config_ctx.name else strip_url(str(params.url)) - subsegment = xray_recorder.begin_subsegment(name, REMOTE_NAMESPACE) + + try: + subsegment = xray_recorder.begin_subsegment(name, REMOTE_NAMESPACE) + except SegmentNotFoundException: + if not trace_config_ctx.raise_if_not_subsegment: + return + raise + subsegment.put_http_meta(http.METHOD, params.method) subsegment.put_http_meta(http.URL, params.url.human_repr()) inject_trace_header(params.headers, subsegment) async def end_subsegment(session, trace_config_ctx, params): - subsegment = xray_recorder.current_subsegment() + + try: + subsegment = xray_recorder.current_subsegment() + except SegmentNotFoundException: + if not trace_config_ctx.raise_if_not_subsegment: + return + raise + subsegment.put_http_meta(http.STATUS, params.response.status) xray_recorder.end_subsegment() async def end_subsegment_with_exception(session, trace_config_ctx, params): - subsegment = xray_recorder.current_subsegment() + + try: + subsegment = xray_recorder.current_subsegment() + except SegmentNotFoundException: + if not trace_config_ctx.raise_if_not_subsegment: + return + raise + subsegment.add_exception( params.exception, traceback.extract_stack(limit=xray_recorder._max_trace_back) @@ -48,16 +70,24 @@ async def end_subsegment_with_exception(session, trace_config_ctx, params): xray_recorder.end_subsegment() -def aws_xray_trace_config(name=None): +def aws_xray_trace_config(name=None, raise_if_not_subsegment=True): """ :param name: name used to identify the subsegment, with None internally the URL will be used as identifier. + :param raise_if_not_subsegment: boolean, raise an exception so stopping the request if + the subsegment can't be created. True by default. Use + False to let the request move ahead and just skip the trace. :returns: TraceConfig. """ - trace_config = aiohttp.TraceConfig( - trace_config_ctx_factory=lambda trace_request_ctx: SimpleNamespace(name=name, - trace_request_ctx=trace_request_ctx) - ) + + def _trace_config_ctx_factory(trace_request_ctx): + return SimpleNamespace( + name=name, + raise_if_not_subsegment=raise_if_not_subsegment, + trace_request_ctx=trace_request_ctx + ) + + trace_config = aiohttp.TraceConfig(trace_config_ctx_factory=_trace_config_ctx_factory) trace_config.on_request_start.append(begin_subsegment) trace_config.on_request_end.append(end_subsegment) trace_config.on_request_exception.append(end_subsegment_with_exception) diff --git a/tests/ext/aiohttp/test_client.py b/tests/ext/aiohttp/test_client.py index 1a0db0b7..5c9e2780 100644 --- a/tests/ext/aiohttp/test_client.py +++ b/tests/ext/aiohttp/test_client.py @@ -3,6 +3,7 @@ from aws_xray_sdk.core import xray_recorder from aws_xray_sdk.core.async_context import AsyncContext +from aws_xray_sdk.core.exceptions.exceptions import SegmentNotFoundException from aws_xray_sdk.ext.util import strip_url from aws_xray_sdk.ext.aiohttp.client import aws_xray_trace_config from aws_xray_sdk.ext.aiohttp.client import REMOTE_NAMESPACE, LOCAL_NAMESPACE @@ -130,3 +131,25 @@ async def test_invalid_url(loop, recorder): exception = subsegment.cause['exceptions'][0] assert exception.type == 'ClientConnectorError' + + +async def test_no_segment_raise(loop, recorder): + trace_config = aws_xray_trace_config() + status_code = 200 + url = 'http://{}/status/{}?foo=bar'.format(BASE_URL, status_code) + with pytest.raises(SegmentNotFoundException): + async with ClientSession(loop=loop, trace_configs=[trace_config]) as session: + async with session.get(url): + pass + + +async def test_no_segment_not_raise(loop, recorder): + trace_config = aws_xray_trace_config(raise_if_not_subsegment=False) + status_code = 200 + url = 'http://{}/status/{}?foo=bar'.format(BASE_URL, status_code) + async with ClientSession(loop=loop, trace_configs=[trace_config]) as session: + async with session.get(url) as resp: + status_received = resp.status + + # Just check that the request was done correctly + assert status_received == status_code From 35e6b8d2f8ea0e42d65d12690a4cd0040a578809 Mon Sep 17 00:00:00 2001 From: Pau Freixes Date: Thu, 10 May 2018 16:35:52 +0200 Subject: [PATCH 2/4] New entry within the CHANGELOG --- CHANGELOG.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 13d290f1..f45291bc 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -5,6 +5,7 @@ CHANGELOG unreleased ========== * feature: Added Sqlalchemy parameterized query capture. `PR34 `_. +* bugfix: Added new `raise_if_not_subsegment` parameter for Aiohttp Client tracing `PR58 `_. 1.0 === From 2708a3a333316d2f0fc4fc28368956e363ad4044 Mon Sep 17 00:00:00 2001 From: Pau Freixes Date: Thu, 10 May 2018 23:27:27 +0200 Subject: [PATCH 3/4] Refactorized a bit the code --- aws_xray_sdk/ext/aiohttp/client.py | 36 ++++++++++++------------------ 1 file changed, 14 insertions(+), 22 deletions(-) diff --git a/aws_xray_sdk/ext/aiohttp/client.py b/aws_xray_sdk/ext/aiohttp/client.py index 53d1d9be..0158733a 100644 --- a/aws_xray_sdk/ext/aiohttp/client.py +++ b/aws_xray_sdk/ext/aiohttp/client.py @@ -24,41 +24,33 @@ async def begin_subsegment(session, trace_config_ctx, params): name = trace_config_ctx.name if trace_config_ctx.name else strip_url(str(params.url)) - try: subsegment = xray_recorder.begin_subsegment(name, REMOTE_NAMESPACE) except SegmentNotFoundException: - if not trace_config_ctx.raise_if_not_subsegment: - return - raise - - subsegment.put_http_meta(http.METHOD, params.method) - subsegment.put_http_meta(http.URL, params.url.human_repr()) - inject_trace_header(params.headers, subsegment) + if trace_config_ctx.raise_if_not_subsegment: + raise + trace_config_ctx.give_up = True + else: + trace_config_ctx.give_up = False + subsegment.put_http_meta(http.METHOD, params.method) + subsegment.put_http_meta(http.URL, params.url.human_repr()) + inject_trace_header(params.headers, subsegment) async def end_subsegment(session, trace_config_ctx, params): + if trace_config_ctx.give_up: + return - try: - subsegment = xray_recorder.current_subsegment() - except SegmentNotFoundException: - if not trace_config_ctx.raise_if_not_subsegment: - return - raise - + subsegment = xray_recorder.current_subsegment() subsegment.put_http_meta(http.STATUS, params.response.status) xray_recorder.end_subsegment() async def end_subsegment_with_exception(session, trace_config_ctx, params): + if trace_config_ctx.give_up: + return - try: - subsegment = xray_recorder.current_subsegment() - except SegmentNotFoundException: - if not trace_config_ctx.raise_if_not_subsegment: - return - raise - + subsegment = xray_recorder.current_subsegment() subsegment.add_exception( params.exception, traceback.extract_stack(limit=xray_recorder._max_trace_back) From 1b2c51576e4d7904dd6b9f5fca70ac9dca97b467 Mon Sep 17 00:00:00 2001 From: Pau Freixes Date: Sat, 12 May 2018 08:36:47 +0200 Subject: [PATCH 4/4] Give up when there is no open segment and LOG_ERROR is configured for context_missing --- aws_xray_sdk/ext/aiohttp/client.py | 16 +++++----------- tests/ext/aiohttp/test_client.py | 4 +++- 2 files changed, 8 insertions(+), 12 deletions(-) diff --git a/aws_xray_sdk/ext/aiohttp/client.py b/aws_xray_sdk/ext/aiohttp/client.py index 0158733a..4843bfeb 100644 --- a/aws_xray_sdk/ext/aiohttp/client.py +++ b/aws_xray_sdk/ext/aiohttp/client.py @@ -9,7 +9,6 @@ from aws_xray_sdk.core import xray_recorder from aws_xray_sdk.core.models import http from aws_xray_sdk.ext.util import inject_trace_header, strip_url -from aws_xray_sdk.core.exceptions.exceptions import SegmentNotFoundException # All aiohttp calls will entail outgoing HTTP requests, only in some ad-hoc # exceptions the namespace will be flip back to local. @@ -24,11 +23,10 @@ async def begin_subsegment(session, trace_config_ctx, params): name = trace_config_ctx.name if trace_config_ctx.name else strip_url(str(params.url)) - try: - subsegment = xray_recorder.begin_subsegment(name, REMOTE_NAMESPACE) - except SegmentNotFoundException: - if trace_config_ctx.raise_if_not_subsegment: - raise + subsegment = xray_recorder.begin_subsegment(name, REMOTE_NAMESPACE) + + # No-op if subsegment is `None` due to `LOG_ERROR`. + if not subsegment: trace_config_ctx.give_up = True else: trace_config_ctx.give_up = False @@ -62,20 +60,16 @@ async def end_subsegment_with_exception(session, trace_config_ctx, params): xray_recorder.end_subsegment() -def aws_xray_trace_config(name=None, raise_if_not_subsegment=True): +def aws_xray_trace_config(name=None): """ :param name: name used to identify the subsegment, with None internally the URL will be used as identifier. - :param raise_if_not_subsegment: boolean, raise an exception so stopping the request if - the subsegment can't be created. True by default. Use - False to let the request move ahead and just skip the trace. :returns: TraceConfig. """ def _trace_config_ctx_factory(trace_request_ctx): return SimpleNamespace( name=name, - raise_if_not_subsegment=raise_if_not_subsegment, trace_request_ctx=trace_request_ctx ) diff --git a/tests/ext/aiohttp/test_client.py b/tests/ext/aiohttp/test_client.py index 5c9e2780..36ffa138 100644 --- a/tests/ext/aiohttp/test_client.py +++ b/tests/ext/aiohttp/test_client.py @@ -134,6 +134,7 @@ async def test_invalid_url(loop, recorder): async def test_no_segment_raise(loop, recorder): + xray_recorder.configure(context_missing='RUNTIME_ERROR') trace_config = aws_xray_trace_config() status_code = 200 url = 'http://{}/status/{}?foo=bar'.format(BASE_URL, status_code) @@ -144,7 +145,8 @@ async def test_no_segment_raise(loop, recorder): async def test_no_segment_not_raise(loop, recorder): - trace_config = aws_xray_trace_config(raise_if_not_subsegment=False) + xray_recorder.configure(context_missing='LOG_ERROR') + trace_config = aws_xray_trace_config() status_code = 200 url = 'http://{}/status/{}?foo=bar'.format(BASE_URL, status_code) async with ClientSession(loop=loop, trace_configs=[trace_config]) as session: