Skip to content

Use is_recording flag in requests instrumentation #1087

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

Merged
merged 16 commits into from
Sep 14, 2020
Original file line number Diff line number Diff line change
Expand Up @@ -121,39 +121,45 @@ def _instrumented_requests_call(
with get_tracer(
__name__, __version__, tracer_provider
).start_as_current_span(span_name, kind=SpanKind.CLIENT) as span:
span.set_attribute("component", "http")
span.set_attribute("http.method", method.upper())
span.set_attribute("http.url", url)
if span.is_recording():
span.set_attribute("component", "http")
span.set_attribute("http.method", method.upper())
span.set_attribute("http.url", url)

headers = get_or_create_headers()
propagators.inject(type(headers).__setitem__, headers)
headers = get_or_create_headers()
propagators.inject(type(headers).__setitem__, headers)

token = context.attach(
context.set_value(_SUPPRESS_REQUESTS_INSTRUMENTATION_KEY, True)
)
try:
result = call_wrapped() # *** PROCEED
except Exception as exc: # pylint: disable=W0703
exception = exc
result = getattr(exc, "response", None)
finally:
context.detach(token)

if exception is not None:
span.set_status(
Status(_exception_to_canonical_code(exception))
)
span.record_exception(exception)

if result is not None:
span.set_attribute("http.status_code", result.status_code)
span.set_attribute("http.status_text", result.reason)
span.set_status(
Status(http_status_to_canonical_code(result.status_code))
token = context.attach(
context.set_value(_SUPPRESS_REQUESTS_INSTRUMENTATION_KEY, True)
)

if span_callback is not None:
span_callback(span, result)
try:
result = call_wrapped() # *** PROCEED
except Exception as exc: # pylint: disable=W0703
exception = exc
result = getattr(exc, "response", None)
finally:
context.detach(token)

if exception is not None:
span.set_status(
Status(_exception_to_canonical_code(exception))
)
span.record_exception(exception)

if result is not None:
span.set_attribute("http.status_code", result.status_code)
span.set_attribute("http.status_text", result.reason)
span.set_status(
Status(http_status_to_canonical_code(result.status_code))
)

if span_callback is not None:
span_callback(span, result)
else:
try:
result = call_wrapped() # *** PROCEED
except Exception as exc: # pylint: disable=W0703
exception = exc

if exception is not None:
raise exception.with_traceback(exception.__traceback__)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,19 @@ def test_suppress_instrumentation(self):

self.assert_span(num_spans=0)

def test_not_recording(self):
RequestsInstrumentor().uninstrument()
RequestsInstrumentor().instrument(
tracer_provider=self.original_tracer_provider
)

result = self.perform_request(self.URL)
self.assertEqual(result.text, "Hello!")

span = self.assert_span(None, 0)
Copy link
Member

@aabmass aabmass Sep 11, 2020

Choose a reason for hiding this comment

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

Looks to me like this would pass regardless of the code change since the result isn't actually different, which is fine if this behavior wasn't checked before.

But also patching (I think) opentelemetry.trace.INVALID_SPAN and then checking that is_recording() was called, set_attribute() was never called, etc. would be good to verify this PR's behavior

with self.assertRaises(AttributeError):
attr = span.attributes

def test_distributed_context(self):
previous_propagator = propagators.get_global_httptextformat()
try:
Expand Down
1 change: 1 addition & 0 deletions tests/util/setup.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ package_dir=
packages=find_namespace:
install_requires =
opentelemetry-api
opentelemetry-sdk

[options.extras_require]
test = flask~=1.0
Expand Down