From ad13efa19bab8d21e33c740ed3da2360f0cdbe67 Mon Sep 17 00:00:00 2001 From: rob salmond Date: Tue, 22 Jan 2019 16:33:12 -0700 Subject: [PATCH 1/6] only attempt to set the span status when a current span is available --- .../trace/ext/flask/flask_middleware.py | 23 ++++++++++--------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/opencensus/trace/ext/flask/flask_middleware.py b/opencensus/trace/ext/flask/flask_middleware.py index 8c9ddf1cc..1ff2b738d 100644 --- a/opencensus/trace/ext/flask/flask_middleware.py +++ b/opencensus/trace/ext/flask/flask_middleware.py @@ -260,18 +260,19 @@ def _teardown_request(self, exception): if exception is not None: span = execution_context.get_current_span() - span.status = status.Status( - code=code_pb2.UNKNOWN, - message=str(exception) - ) - # try attaching the stack trace to the span, only populated if - # the app has 'PROPAGATE_EXCEPTIONS', 'DEBUG', or 'TESTING' - # enabled - exc_type, _, exc_traceback = sys.exc_info() - if exc_traceback is not None: - span.stack_trace = stack_trace.StackTrace.from_traceback( - exc_traceback + if span is not None: + span.status = status.Status( + code=code_pb2.UNKNOWN, + message=str(exception) ) + # try attaching the stack trace to the span, only populated if + # the app has 'PROPAGATE_EXCEPTIONS', 'DEBUG', or 'TESTING' + # enabled + exc_type, _, exc_traceback = sys.exc_info() + if exc_traceback is not None: + span.stack_trace = stack_trace.StackTrace.from_traceback( + exc_traceback + ) tracer.end_span() tracer.finish() From aeebe0e944306eab6ad63c9aff59173521763251 Mon Sep 17 00:00:00 2001 From: Rob Salmond Date: Wed, 23 Jan 2019 00:10:33 -0700 Subject: [PATCH 2/6] add a test which attempts to capture exception data in a span which does not exist --- .../trace/ext/flask/test_flask_middleware.py | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/tests/unit/trace/ext/flask/test_flask_middleware.py b/tests/unit/trace/ext/flask/test_flask_middleware.py index fcf2f38cb..87c7ee58f 100644 --- a/tests/unit/trace/ext/flask/test_flask_middleware.py +++ b/tests/unit/trace/ext/flask/test_flask_middleware.py @@ -35,6 +35,8 @@ from opencensus.trace.tracers import base from opencensus.trace.tracers import noop_tracer from opencensus.trace.blank_span import BlankSpan +from opencensus.trace.span_context import SpanContext +from opencensus.trace.trace_options import TraceOptions class TestFlaskMiddleware(unittest.TestCase): @@ -471,3 +473,23 @@ def test_teardown_include_exception_and_traceback(self): ) self.assertIsNotNone(exported_spandata.stack_trace.stack_trace_hash_id) self.assertNotEqual(exported_spandata.stack_trace.stack_frames, []) + + def test_teardown_include_exception_and_traceback_span_disabled(self): + sampler = always_off.AlwaysOffSampler() + app = self.create_app() + app.config['TESTING'] = True + middleware = flask_middleware.FlaskMiddleware(app=app, sampler=sampler) + + original_method = middleware.propagator.from_headers + + def nope(*args, **kwargs): + trace_options = TraceOptions() + trace_options.set_enabled(False) + return SpanContext(trace_options=trace_options) + + middleware.propagator.from_headers = nope + + with self.assertRaises(Exception): + app.test_client().get('/error') + + middleware.propagator.from_headers = original_method From 724d96961e168a0a6af0c6976a6cf7d617c98dee Mon Sep 17 00:00:00 2001 From: Chris Kleinknecht Date: Wed, 23 Jan 2019 13:37:21 -0800 Subject: [PATCH 3/6] Lint fixes --- opencensus/trace/ext/flask/flask_middleware.py | 12 +++++++----- tests/unit/trace/ext/flask/test_flask_middleware.py | 2 +- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/opencensus/trace/ext/flask/flask_middleware.py b/opencensus/trace/ext/flask/flask_middleware.py index 1ff2b738d..2a3db49c5 100644 --- a/opencensus/trace/ext/flask/flask_middleware.py +++ b/opencensus/trace/ext/flask/flask_middleware.py @@ -265,13 +265,15 @@ def _teardown_request(self, exception): code=code_pb2.UNKNOWN, message=str(exception) ) - # try attaching the stack trace to the span, only populated if - # the app has 'PROPAGATE_EXCEPTIONS', 'DEBUG', or 'TESTING' - # enabled + # try attaching the stack trace to the span, only populated + # if the app has 'PROPAGATE_EXCEPTIONS', 'DEBUG', or + # 'TESTING' enabled exc_type, _, exc_traceback = sys.exc_info() if exc_traceback is not None: - span.stack_trace = stack_trace.StackTrace.from_traceback( - exc_traceback + span.stack_trace = ( + stack_trace.StackTrace.from_traceback( + exc_traceback + ) ) tracer.end_span() diff --git a/tests/unit/trace/ext/flask/test_flask_middleware.py b/tests/unit/trace/ext/flask/test_flask_middleware.py index 87c7ee58f..61a544ed1 100644 --- a/tests/unit/trace/ext/flask/test_flask_middleware.py +++ b/tests/unit/trace/ext/flask/test_flask_middleware.py @@ -473,7 +473,7 @@ def test_teardown_include_exception_and_traceback(self): ) self.assertIsNotNone(exported_spandata.stack_trace.stack_trace_hash_id) self.assertNotEqual(exported_spandata.stack_trace.stack_frames, []) - + def test_teardown_include_exception_and_traceback_span_disabled(self): sampler = always_off.AlwaysOffSampler() app = self.create_app() From a20b39816d9bf29e0295b29f6f6c2f4599236b9d Mon Sep 17 00:00:00 2001 From: Chris Kleinknecht Date: Wed, 23 Jan 2019 16:17:28 -0800 Subject: [PATCH 4/6] Add TODO --- tests/unit/trace/ext/flask/test_flask_middleware.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/unit/trace/ext/flask/test_flask_middleware.py b/tests/unit/trace/ext/flask/test_flask_middleware.py index 61a544ed1..c56022322 100644 --- a/tests/unit/trace/ext/flask/test_flask_middleware.py +++ b/tests/unit/trace/ext/flask/test_flask_middleware.py @@ -480,6 +480,7 @@ def test_teardown_include_exception_and_traceback_span_disabled(self): app.config['TESTING'] = True middleware = flask_middleware.FlaskMiddleware(app=app, sampler=sampler) + # TODO: send trace options in header (#465) original_method = middleware.propagator.from_headers def nope(*args, **kwargs): From 8bb8e06b6f365fc4a6da88bdc34ded6eff8ff140 Mon Sep 17 00:00:00 2001 From: Chris Kleinknecht Date: Wed, 23 Jan 2019 16:23:59 -0800 Subject: [PATCH 5/6] Don't mask base Exceptions in test --- tests/unit/trace/ext/flask/test_flask_middleware.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/tests/unit/trace/ext/flask/test_flask_middleware.py b/tests/unit/trace/ext/flask/test_flask_middleware.py index c56022322..b24062228 100644 --- a/tests/unit/trace/ext/flask/test_flask_middleware.py +++ b/tests/unit/trace/ext/flask/test_flask_middleware.py @@ -39,6 +39,10 @@ from opencensus.trace.trace_options import TraceOptions +class FlaskTestException(Exception): + pass + + class TestFlaskMiddleware(unittest.TestCase): @staticmethod @@ -55,7 +59,7 @@ def health_check(): @app.route('/error') def error(): - raise Exception('error') + raise FlaskTestException('error') return app @@ -460,7 +464,7 @@ def test_teardown_include_exception_and_traceback(self): app = self.create_app() app.config['TESTING'] = True flask_middleware.FlaskMiddleware(app=app, exporter=mock_exporter) - with self.assertRaises(Exception): + with self.assertRaises(FlaskTestException): app.test_client().get('/error') exported_spandata = mock_exporter.export.call_args[0][0][0] @@ -490,7 +494,7 @@ def nope(*args, **kwargs): middleware.propagator.from_headers = nope - with self.assertRaises(Exception): + with self.assertRaises(FlaskTestException): app.test_client().get('/error') middleware.propagator.from_headers = original_method From e7cd6797e13fe40c4c72735490f2420e2b0aa0d6 Mon Sep 17 00:00:00 2001 From: Chris Kleinknecht Date: Wed, 23 Jan 2019 16:24:23 -0800 Subject: [PATCH 6/6] Clean up imports --- .../trace/ext/flask/test_flask_middleware.py | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/tests/unit/trace/ext/flask/test_flask_middleware.py b/tests/unit/trace/ext/flask/test_flask_middleware.py index b24062228..3f1aad18c 100644 --- a/tests/unit/trace/ext/flask/test_flask_middleware.py +++ b/tests/unit/trace/ext/flask/test_flask_middleware.py @@ -17,26 +17,28 @@ import unittest +from google.rpc import code_pb2 import flask import mock -from google.rpc import code_pb2 from opencensus.trace import execution_context -from opencensus.trace import span_data from opencensus.trace import span as span_module +from opencensus.trace import span_data from opencensus.trace import stack_trace from opencensus.trace import status -from opencensus.trace.exporters import print_exporter, stackdriver_exporter, \ - zipkin_exporter, jaeger_exporter +from opencensus.trace.blank_span import BlankSpan +from opencensus.trace.exporters import jaeger_exporter +from opencensus.trace.exporters import print_exporter +from opencensus.trace.exporters import stackdriver_exporter +from opencensus.trace.exporters import zipkin_exporter from opencensus.trace.exporters.ocagent import trace_exporter from opencensus.trace.ext.flask import flask_middleware from opencensus.trace.propagation import google_cloud_format from opencensus.trace.samplers import always_off, always_on, ProbabilitySampler -from opencensus.trace.tracers import base -from opencensus.trace.tracers import noop_tracer -from opencensus.trace.blank_span import BlankSpan from opencensus.trace.span_context import SpanContext from opencensus.trace.trace_options import TraceOptions +from opencensus.trace.tracers import base +from opencensus.trace.tracers import noop_tracer class FlaskTestException(Exception):