feat: Allow OTEL_PHP_DEBUG_SCOPES_DISABLED as en environment variable

[PROFeNoM]

Hi 👋

While doing some tests and trying to set OTEL_PHP_DEBUG_SCOPES_DISABLED to 1 as an environment variable to disable the use of DebugScope, it wouldn't work. This is because it wasn't considered.

I guess it could have also made sense to use some kind of a ResolverInterface, considering both INIs and env var.

Also, my personal opinion on this env var is that it could make sense to have it enabled by default (in the next major, being a breaking change), considering that the use of debug_backtrace has a non-negligible performance impact.

[welcome]

Thanks for opening your first pull request! If you haven't yet signed our Contributor License Agreement (CLA), then please do so that we can accept your contribution. A link should appear shortly in this PR if you have not already signed one.

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: PROFeNoM / name: Alexandre Choura (5c72145, e22c8fc, b076c40, 5d1a7f5, cfb6476, 618f517)

[Nevay]

Disabling via environment variable should work as expected (unless variables_order is configured to not populate $_SERVER):

OTEL_PHP_DEBUG_SCOPES_DISABLED=1 php -r 'require "vendor/autoload.php"; echo OpenTelemetry\Context\Context::getCurrent()->activate()::class, PHP_EOL;'

---

Assuming that you are using putenv() to set the environment variable as done in the testcase - putenv() is not threadsafe, for example vlucas/phpdotenv and symfony/dotenv won't populate it by default:

Using getenv() and putenv() is strongly discouraged due to the fact that these functions are not thread safe, however it is still possible to instruct PHP dotenv to use these functions.

Beware that putenv() is not thread safe, that's why it's not enabled by default
// don't check existence with getenv() because of thread safety issues

If we want to support putenv() / getenv(), we should IMO use the order $_SERVER > getenv() > ini_get().

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 83.08%. Comparing base (b20c45d) to head (b076c40).
Report is 1 commits behind head on main.

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #1237      +/-   ##
============================================
- Coverage     83.09%   83.08%   -0.02%     
+ Complexity     2275     2274       -1     
============================================
  Files           285      285              
  Lines          6460     6456       -4     
============================================
- Hits           5368     5364       -4     
  Misses         1092     1092              

Flag	Coverage Δ
7.4	81.72% <100.00%> (-0.02%)	⬇️
8.0	82.98% <100.00%> (-0.02%)	⬇️
8.1	83.14% <100.00%> (-0.02%)	⬇️
8.2	83.14% <100.00%> (-0.02%)	⬇️
8.3	83.14% <100.00%> (-0.02%)	⬇️
8.4	83.14% <100.00%> (-0.02%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files	Coverage Δ
src/Context/Context.php	82.60% <100.00%> (+0.38%)	⬆️

... and 4 files with indirect coverage changes

---

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b20c45d...b076c40. Read the comment docs.

[brettmc]

To be consistent with how the SDK checks for variables, can you change the order to be $_SERVER, getenv, ini_get

[brettmc]

Also, my personal opinion on this env var is that it could make sense to have it enabled by default (in the next major, being a breaking change), considering that the use of debug_backtrace has a non-negligible performance impact.

I think the idea is that because it's inside an assertion, when run with a production configuration that code will not be run. So in effect, you couldn't use DebugScope in a non-assertion environment even if you wanted to.

[brettmc]

Can you add a test to prove what happens if the variable is falsey? I'm assuming that disabled=false === enabled, so there DebugScope should be used in this case.