Fix RuntimeError: Working outside of request context.

[benjaminsky]

Description

Check for flask.request to avoid RuntimeError: Working outside of request context. error.

Type of change

• Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

Validated locally.

Does This PR Require a Core Repo Change?

• No.

Checklist:

• Followed the style guidelines of this project
• Changelogs have been updated

[srikanthccv]

When does this happen? I would expect it to be present.

[owais]

Same question as @lonewolf3739. Is this happening because you are using the instrumentation outside a real request-response cycle (some mock/test env) or are we missing something here as instrumentation authors?

[benjaminsky]

I ran into this issue instrumenting a flask app that’s also instrumented with New Relic. I think that’s the cause. I can dig into it further but it is a real environment. It’s an ancient version of flask too though - 0.12.1. Ben Campbell

…

________________________________ From: Owais Lone ***@***.***> Sent: Tuesday, October 12, 2021 5:22:09 PM To: open-telemetry/opentelemetry-python-contrib ***@***.***> Cc: Ben Campbell ***@***.***>; Author ***@***.***> Subject: Re: [open-telemetry/opentelemetry-python-contrib] Fix RuntimeError: Working outside of request context. (#734) Same question as @lonewolf3739<https://github.com/lonewolf3739>. Is this happening because you are using the instrumentation outside a real request-response cycle (some mock/test env) or are we missing something here as instrumentation authors? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub<#734 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AAMJ5LRK4LR2FE7BHYSP34LUGTGLDANCNFSM5F34EMCQ>. Triage notifications on the go with GitHub Mobile for iOS<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.

[srikanthccv]

OpenTelemetry flask instrumentation requires >= 1.0. Why are trying to instrument with both otel and new relic? I think you should dig in further and find the the actual issue.

[owais]

• OpenTelemetry supports flask 1.0 and newer as @lonewolf3739 pointed out.
• We do not support double instrumenting with multiple agents. Behavior under such conditions is undefined at best.

That said, this change itself seems harmless even if unnecessary for supported versions of Flask. I'm inclined to accept this especially if we can specifically describe a situation where this fails.

[benjaminsky]

After digging in, this is caused by the New Relic instrumentation in my case. That said, I'm getting an uncaught exception and given the desire for instrumentation never to throw errors, this feels like a pretty reasonable bulletproofing change. We do wish to retain both New Relic and otel when we go live. I know this isn't supported but with this change it works.

[owais]

It sounds like another 3rd party library (NR) is changing a Flask in way that break it's public API or expected behavior. I'm inclined to not merge PRs like this in general but this specific patch seems harmless and I think I can live with it if it unblocks you. That said, I'd like to give @open-telemetry/python-approvers a chance to share their opinion on this as well.

[lzchen]

@owais @benjaminsky @lonewolf3739

The change itself seems harmless enough so approving, but we don't want to set the precedence for the future to change instrumentations based off of undefined/unsupported behaviour. We can deal with this on a case by case basis however since it doesn't seem like it occurs that often.