Do not validate instrumentation, if the whole SDK is disabled.

[profuel]

Do not validate instrumentation, if the whole SDK is disabled.

[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]

• ✅login: profuel / (b6e032b)

The committers listed above are authorized under a signed CLA.

[codecov]

Codecov Report

Attention: Patch coverage is 50.00000% with 1 line in your changes missing coverage. Please review.

Project coverage is 73.36%. Comparing base (4a021fb) to head (b6e032b).

Files with missing lines	Patch %	Lines
src/SDK/Sdk.php	50.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #1453      +/-   ##
============================================
+ Coverage     73.26%   73.36%   +0.09%     
- Complexity     2685     2686       +1     
============================================
  Files           387      387              
  Lines          8009     8011       +2     
============================================
+ Hits           5868     5877       +9     
+ Misses         2141     2134       -7     

Flag	Coverage Δ
8.1	72.95% <50.00%> (-0.04%)	⬇️
8.2	73.18% <50.00%> (+<0.01%)	⬆️
8.3	73.21% <50.00%> (+0.01%)	⬆️
8.4	?
8.5	73.25% <50.00%> (+0.06%)	⬆️

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

Files with missing lines	Coverage Δ
src/SDK/Sdk.php	95.23% <50.00%> (-4.77%)	⬇️

... 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 4a021fb...b6e032b. Read the comment docs.

[brettmc]

This needs some thought. Per spec, disabling the SDK should still allow propagators: https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/configuration/sdk-environment-variables.md#general-sdk-configuration
We have auto-instrumentations for http clients, and would expect that they would still propagate distributed trace headers if the SDK is disabled. I think this would break that?

[profuel]

That's an interesting detail, I was missing.
Could you please help me understand - does it mean that this code is not correct?
https://github.com/spryker/otel-rabbit-mq-instrumentation/blob/master/_register.php
And we have to use similar logic to this :

$disabledInstrumentations = Configuration::getList(Variables::OTEL_PHP_DISABLED_INSTRUMENTATIONS);

return [self::OTEL_PHP_DISABLED_INSTRUMENTATIONS_ALL] === $disabledInstrumentations || in_array($name, $disabledInstrumentations);

Thank you for your help!

[brettmc]

does it mean that this code is not correct

I think it probably doesn't matter for rabbitmq (it's not performing distributed tracing). Our current thinking is that if the SDK is disabled, then when you fetch tracers/meters/loggers from the various providers, you'll get a no-op instance.

[brettmc]

@profuel thanks for your contribution, however we won't progress with this change due to the propagator thing - it would break some of our instrumentations according to spec.