Skip to content

Commit

Permalink
Use is_recording flag in jinja, celery, esearch, falcon instrumentati…
Browse files Browse the repository at this point in the history
…ons (#1241)
  • Loading branch information
lzchen authored Oct 14, 2020
1 parent c782f14 commit 535c2e6
Show file tree
Hide file tree
Showing 11 changed files with 160 additions and 67 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -148,10 +148,11 @@ def _trace_postrun(*args, **kwargs):
return

# request context tags
span.set_attribute(_TASK_TAG_KEY, _TASK_RUN)
utils.set_attributes_from_context(span, kwargs)
utils.set_attributes_from_context(span, task.request)
span.set_attribute(_TASK_NAME_KEY, task.name)
if span.is_recording():
span.set_attribute(_TASK_TAG_KEY, _TASK_RUN)
utils.set_attributes_from_context(span, kwargs)
utils.set_attributes_from_context(span, task.request)
span.set_attribute(_TASK_NAME_KEY, task.name)

activation.__exit__(None, None, None)
utils.detach_span(task, task_id)
Expand All @@ -169,10 +170,11 @@ def _trace_before_publish(self, *args, **kwargs):
)

# apply some attributes here because most of the data is not available
span.set_attribute(_TASK_TAG_KEY, _TASK_APPLY_ASYNC)
span.set_attribute(_MESSAGE_ID_ATTRIBUTE_NAME, task_id)
span.set_attribute(_TASK_NAME_KEY, task.name)
utils.set_attributes_from_context(span, kwargs)
if span.is_recording():
span.set_attribute(_TASK_TAG_KEY, _TASK_APPLY_ASYNC)
span.set_attribute(_MESSAGE_ID_ATTRIBUTE_NAME, task_id)
span.set_attribute(_TASK_NAME_KEY, task.name)
utils.set_attributes_from_context(span, kwargs)

activation = self._tracer.use_span(span, end_on_exit=True)
activation.__enter__()
Expand Down Expand Up @@ -209,7 +211,7 @@ def _trace_failure(*args, **kwargs):

# retrieve and pass exception info to activation
span, _ = utils.retrieve_span(task, task_id)
if span is None:
if span is None or not span.is_recording():
return

status_kwargs = {"canonical_code": StatusCanonicalCode.UNKNOWN}
Expand Down Expand Up @@ -238,7 +240,7 @@ def _trace_retry(*args, **kwargs):
return

span, _ = utils.retrieve_span(task, task_id)
if span is None:
if span is None or not span.is_recording():
return

# Add retry reason metadata to span
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@
# pylint:disable=too-many-branches
def set_attributes_from_context(span, context):
"""Helper to extract meta values from a Celery Context"""
if not span.is_recording():
return
for key in CELERY_CONTEXT_ATTRIBUTES:
value = context.get(key)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,30 @@ def test_set_attributes_from_context(self):
)
self.assertNotIn("custom_meta", span.attributes)

def test_set_attributes_not_recording(self):
# it should extract only relevant keys
context = {
"correlation_id": "44b7f305",
"delivery_info": {"eager": True},
"eta": "soon",
"expires": "later",
"hostname": "localhost",
"id": "44b7f305",
"reply_to": "44b7f305",
"retries": 4,
"timelimit": ("now", "later"),
"custom_meta": "custom_value",
"routing_key": "celery",
}

mock_span = mock.Mock()
mock_span.is_recording.return_value = False
utils.set_attributes_from_context(mock_span, context)
self.assertFalse(mock_span.is_recording())
self.assertTrue(mock_span.is_recording.called)
self.assertFalse(mock_span.set_attribute.called)
self.assertFalse(mock_span.set_status.called)

def test_set_attributes_from_context_empty_keys(self):
# it should not extract empty keys
context = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ def _uninstrument(self, **kwargs):


def _wrap_perform_request(tracer, span_name_prefix):
# pylint: disable=R0912
def wrapper(wrapped, _, args, kwargs):
method = url = None
try:
Expand All @@ -125,26 +126,27 @@ def wrapper(wrapped, _, args, kwargs):
params = kwargs.get("params", {})
body = kwargs.get("body", None)

attributes = {
"component": "elasticsearch-py",
"db.type": "elasticsearch",
}

if url:
attributes["elasticsearch.url"] = url
if method:
attributes["elasticsearch.method"] = method
if body:
attributes["db.statement"] = str(body)
if params:
attributes["elasticsearch.params"] = str(params)

with tracer.start_as_current_span(
op_name, kind=SpanKind.CLIENT, attributes=attributes
op_name, kind=SpanKind.CLIENT,
) as span:
if span.is_recording():
attributes = {
"component": "elasticsearch-py",
"db.type": "elasticsearch",
}
if url:
attributes["elasticsearch.url"] = url
if method:
attributes["elasticsearch.method"] = method
if body:
attributes["db.statement"] = str(body)
if params:
attributes["elasticsearch.params"] = str(params)
for key, value in attributes.items():
span.set_attribute(key, value)
try:
rv = wrapped(*args, **kwargs)
if isinstance(rv, dict):
if isinstance(rv, dict) and span.is_recording():
for member in _ATTRIBUTES_FROM_RESULT:
if member in rv:
span.set_attribute(
Expand All @@ -153,11 +155,12 @@ def wrapper(wrapped, _, args, kwargs):
)
return rv
except Exception as ex: # pylint: disable=broad-except
if isinstance(ex, elasticsearch.exceptions.NotFoundError):
status = StatusCanonicalCode.NOT_FOUND
else:
status = StatusCanonicalCode.UNKNOWN
span.set_status(Status(status, str(ex)))
if span.is_recording():
if isinstance(ex, elasticsearch.exceptions.NotFoundError):
status = StatusCanonicalCode.NOT_FOUND
else:
status = StatusCanonicalCode.UNKNOWN
span.set_status(Status(status, str(ex)))
raise ex

return wrapper
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,24 @@ def test_instrumentor(self, request_mock):
spans_list = self.get_ordered_finished_spans()
self.assertEqual(len(spans_list), 1)

def test_span_not_recording(self, request_mock):
request_mock.return_value = (1, {}, {})
mock_tracer = mock.Mock()
mock_span = mock.Mock()
mock_span.is_recording.return_value = False
mock_tracer.start_span.return_value = mock_span
mock_tracer.use_span.return_value.__enter__ = mock_span
mock_tracer.use_span.return_value.__exit__ = mock_span
with mock.patch("opentelemetry.trace.get_tracer") as tracer:
tracer.return_value = mock_tracer
Elasticsearch()
self.assertFalse(mock_span.is_recording())
self.assertTrue(mock_span.is_recording.called)
self.assertFalse(mock_span.set_attribute.called)
self.assertFalse(mock_span.set_status.called)

ElasticsearchInstrumentor().uninstrument()

def test_prefix_arg(self, request_mock):
prefix = "prefix-from-env"
ElasticsearchInstrumentor().uninstrument()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -117,13 +117,16 @@ def __call__(self, env, start_response):
token = context.attach(
propagators.extract(otel_wsgi.get_header_from_environ, env)
)
attributes = otel_wsgi.collect_request_attributes(env)
span = self._tracer.start_span(
otel_wsgi.get_default_span_name(env),
kind=trace.SpanKind.SERVER,
attributes=attributes,
start_time=start_time,
)
if span.is_recording():
attributes = otel_wsgi.collect_request_attributes(env)
for key, value in attributes.items():
span.set_attribute(key, value)

activation = self._tracer.use_span(span, end_on_exit=True)
activation.__enter__()
env[_ENVIRON_SPAN_KEY] = span
Expand Down Expand Up @@ -162,7 +165,7 @@ def __init__(self, tracer=None, traced_request_attrs=None):

def process_request(self, req, resp):
span = req.env.get(_ENVIRON_SPAN_KEY)
if not span:
if not span or not span.is_recording():
return

attributes = extract_attributes_from_object(
Expand All @@ -173,7 +176,7 @@ def process_request(self, req, resp):

def process_resource(self, req, resp, resource, params):
span = req.env.get(_ENVIRON_SPAN_KEY)
if not span:
if not span or not span.is_recording():
return

resource_name = resource.__class__.__name__
Expand All @@ -186,7 +189,7 @@ def process_response(
self, req, resp, resource, req_succeeded=None
): # pylint:disable=R0201
span = req.env.get(_ENVIRON_SPAN_KEY)
if not span:
if not span or not span.is_recording():
return

status = resp.status
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
# See the License for the specific language governing permissions and
# limitations under the License.

from unittest.mock import patch
from unittest.mock import Mock, patch

from falcon import testing

Expand Down Expand Up @@ -188,3 +188,18 @@ def test_traced_request_attributes(self):
span = self.memory_exporter.get_finished_spans()[0]
self.assertIn("query_string", span.attributes)
self.assertEqual(span.attributes["query_string"], "q=abc")

def test_traced_not_recording(self):
mock_tracer = Mock()
mock_span = Mock()
mock_span.is_recording.return_value = False
mock_tracer.start_span.return_value = mock_span
mock_tracer.use_span.return_value.__enter__ = mock_span
mock_tracer.use_span.return_value.__exit__ = mock_span
with patch("opentelemetry.trace.get_tracer") as tracer:
tracer.return_value = mock_tracer
self.client().simulate_get(path="/hello?q=abc")
self.assertFalse(mock_span.is_recording())
self.assertTrue(mock_span.is_recording.called)
self.assertFalse(mock_span.set_attribute.called)
self.assertFalse(mock_span.set_status.called)
Original file line number Diff line number Diff line change
Expand Up @@ -78,39 +78,44 @@ def wrapper(wrapped, instance, args, kwargs):
def _wrap_render(tracer, wrapped, instance, args, kwargs):
"""Wrap `Template.render()` or `Template.generate()`
"""
template_name = instance.name or DEFAULT_TEMPLATE_NAME
attributes = {ATTRIBUTE_JINJA2_TEMPLATE_NAME: template_name}
with tracer.start_as_current_span(
"jinja2.render", kind=SpanKind.INTERNAL, attributes=attributes
):
"jinja2.render", kind=SpanKind.INTERNAL,
) as span:
if span.is_recording():
template_name = instance.name or DEFAULT_TEMPLATE_NAME
span.set_attribute(ATTRIBUTE_JINJA2_TEMPLATE_NAME, template_name)
return wrapped(*args, **kwargs)


@_with_tracer_wrapper
def _wrap_compile(tracer, wrapped, _, args, kwargs):
template_name = (
args[1] if len(args) > 1 else kwargs.get("name", DEFAULT_TEMPLATE_NAME)
)
attributes = {ATTRIBUTE_JINJA2_TEMPLATE_NAME: template_name}
with tracer.start_as_current_span(
"jinja2.compile", kind=SpanKind.INTERNAL, attributes=attributes
):
"jinja2.compile", kind=SpanKind.INTERNAL,
) as span:
if span.is_recording():
template_name = (
args[1]
if len(args) > 1
else kwargs.get("name", DEFAULT_TEMPLATE_NAME)
)
span.set_attribute(ATTRIBUTE_JINJA2_TEMPLATE_NAME, template_name)
return wrapped(*args, **kwargs)


@_with_tracer_wrapper
def _wrap_load_template(tracer, wrapped, _, args, kwargs):
template_name = kwargs.get("name", args[0])
attributes = {ATTRIBUTE_JINJA2_TEMPLATE_NAME: template_name}
with tracer.start_as_current_span(
"jinja2.load", kind=SpanKind.INTERNAL, attributes=attributes
"jinja2.load", kind=SpanKind.INTERNAL,
) as span:
if span.is_recording():
template_name = kwargs.get("name", args[0])
span.set_attribute(ATTRIBUTE_JINJA2_TEMPLATE_NAME, template_name)
template = None
try:
template = wrapped(*args, **kwargs)
return template
finally:
if template:
if template and span.is_recording():
span.set_attribute(
ATTRIBUTE_JINJA2_TEMPLATE_PATH, template.filename
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
# limitations under the License.

import os
from unittest import mock

import jinja2

Expand Down Expand Up @@ -53,6 +54,21 @@ def test_render_inline_template_with_root(self):
self.assertIs(template.parent, root.get_span_context())
self.assertIsNone(root.parent)

def test_render_not_recording(self):
mock_tracer = mock.Mock()
mock_span = mock.Mock()
mock_span.is_recording.return_value = False
mock_tracer.start_span.return_value = mock_span
mock_tracer.use_span.return_value.__enter__ = mock_span
mock_tracer.use_span.return_value.__exit__ = mock_span
with mock.patch("opentelemetry.trace.get_tracer") as tracer:
tracer.return_value = mock_tracer
jinja2.environment.Template("Hello {{name}}!")
self.assertFalse(mock_span.is_recording())
self.assertTrue(mock_span.is_recording.called)
self.assertFalse(mock_span.set_attribute.called)
self.assertFalse(mock_span.set_status.called)

def test_render_inline_template(self):
template = jinja2.environment.Template("Hello {{name}}!")
self.assertEqual(template.render(name="Jinja"), "Hello Jinja!")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,15 +118,16 @@ def _wrap_cmd(tracer, cmd, wrapped, instance, args, kwargs):
_CMD, kind=SpanKind.INTERNAL, attributes={}
) as span:
try:
if not args:
vals = ""
else:
vals = _get_query_string(args[0])
if span.is_recording():
if not args:
vals = ""
else:
vals = _get_query_string(args[0])

query = "{}{}{}".format(cmd, " " if vals else "", vals)
span.set_attribute(_RAWCMD, query)
query = "{}{}{}".format(cmd, " " if vals else "", vals)
span.set_attribute(_RAWCMD, query)

_set_connection_attributes(span, instance)
_set_connection_attributes(span, instance)
except Exception as ex: # pylint: disable=broad-except
logger.warning(
"Failed to set attributes for pymemcache span %s", str(ex)
Expand Down
Loading

0 comments on commit 535c2e6

Please sign in to comment.