Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conditionally create server spans for flask #828

Merged
merged 7 commits into from
Dec 20, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

- `opentelemetry-instrumentation-tornado` Add support instrumentation for Tornado 5.1.1
([#812](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/812))

- `opentelemetry-instrumentation-flask` Flask: Conditionally create SERVER spans
([#828](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/828))

## [1.7.1-0.26b1](https://github.com/open-telemetry/opentelemetry-python/releases/tag/v1.7.0-0.26b0) - 2021-11-11

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -176,15 +176,24 @@ def _before_request():
return
flask_request_environ = flask.request.environ
span_name = get_default_span_name()
token = context.attach(
extract(flask_request_environ, getter=otel_wsgi.wsgi_getter)
)

token = ctx = span_kind = None

if trace.get_current_span() is trace.INVALID_SPAN:
ctx = extract(flask_request_environ, getter=otel_wsgi.wsgi_getter)
token = context.attach(ctx)
span_kind = trace.SpanKind.SERVER
else:
ctx = context.get_current()
span_kind = trace.SpanKind.INTERNAL

Copy link
Contributor

@lzchen lzchen Dec 9, 2021

Choose a reason for hiding this comment

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

If span is found in current context, I believe you'll have to set the current context as the parent. Currently, ctx would be None.

Copy link
Member Author

@ashu658 ashu658 Dec 9, 2021

Choose a reason for hiding this comment

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

Done.
I think it was passing earlier as tracer.start_span() makes current span as parent if context passed is none.
@lzchen Should we set parent explicitly for readability or let start_span() handle it?

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh yes you are correct. Either way is fine with me.

span = tracer.start_span(
span_name,
kind=trace.SpanKind.SERVER,
ctx,
kind=span_kind,
start_time=flask_request_environ.get(_ENVIRON_STARTTIME_KEY),
)

if request_hook:
request_hook(span, flask_request_environ)

Expand Down Expand Up @@ -229,7 +238,9 @@ def _teardown_request(exc):
activation.__exit__(
type(exc), exc, getattr(exc, "__traceback__", None)
)
context.detach(flask.request.environ.get(_ENVIRON_TOKEN))

if flask.request.environ.get(_ENVIRON_TOKEN, None):
context.detach(flask.request.environ.get(_ENVIRON_TOKEN))

return _teardown_request

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
get_global_response_propagator,
set_global_response_propagator,
)
from opentelemetry.instrumentation.wsgi import OpenTelemetryMiddleware
from opentelemetry.sdk.resources import Resource
from opentelemetry.semconv.trace import SpanAttributes
from opentelemetry.test.test_base import TestBase
Expand Down Expand Up @@ -413,3 +414,31 @@ def test_custom_span_name(self):
span_list[0].resource.attributes["service.name"],
"flask-api-no-app",
)


class TestProgrammaticWrappedWithOtherFramework(
InstrumentationTest, TestBase, WsgiTestBase
):
def setUp(self):
super().setUp()

self.app = Flask(__name__)
self.app.wsgi_app = OpenTelemetryMiddleware(self.app.wsgi_app)
FlaskInstrumentor().instrument_app(self.app)
self._common_initialization()

def tearDown(self) -> None:
super().tearDown()
with self.disable_logging():
FlaskInstrumentor().uninstrument_app(self.app)

def test_mark_span_internal_in_presence_of_span_from_other_framework(self):
resp = self.client.get("/hello/123")
self.assertEqual(200, resp.status_code)
resp.get_data()
span_list = self.memory_exporter.get_finished_spans()
self.assertEqual(trace.SpanKind.INTERNAL, span_list[0].kind)
self.assertEqual(trace.SpanKind.SERVER, span_list[1].kind)
self.assertEqual(
span_list[0].parent.span_id, span_list[1].context.span_id
)