open-telemetry / opentelemetry-php

The OpenTelemetry PHP Library
https://opentelemetry.io/docs/instrumentation/php/
Apache License 2.0
750 stars 186 forks source link

Typed static property OpenTelemetry\Context\Context::$spanContextKey must not be accessed before initialization #1247

Closed dhempel closed 8 months ago

dhempel commented 8 months ago

Describe your environment

php -v
PHP 8.2.15 (cli) (built: Jan 20 2024 14:15:02) (NTS)
Copyright (c) The PHP Group
Zend Engine v4.2.15, Copyright (c) Zend Technologies
    with Zend OPcache v8.2.15, Copyright (c), by Zend Technologies
    with Xdebug v3.3.1, Copyright (c) 2002-2023, by Derick Rethans

php --ri=opentelemetry
opentelemetry
opentelemetry hooks => enabled
extension version => 1.0.1
Directive => Local Value => Master Value
opentelemetry.conflicts => no value => no value
opentelemetry.validate_hook_functions => On => On

Composer-Packages
    "open-telemetry/sdk": "^1.0",
    "open-telemetry/transport-grpc": "^1.0",
    "open-telemetry/exporter-otlp": "^1.0",
    "php-http/guzzle7-adapter": "^1.0"

Steps to reproduce Autoloading for open-telemetry/context will throw the error above, which I converted into a stack trace

PHP Fatal error:  Uncaught Exception in /tmp/vendor/open-telemetry/context/Context.php:47
Stack trace:
#0 /tmp/vendor/open-telemetry/context/fiber/initialize_fiber_handler.php(19): OpenTelemetry\Context\Context::setStorage()
#1 /tmp/vendor/composer/autoload_real.php(53): require('...')
#2 /tmp/vendor/composer/autoload_real.php(36): composerRequire162d17fff590185c1d3968e76312fbb5()
#3 /tmp/vendor/autoload.php(12): ComposerAutoloaderInit162d17fff590185c1d3968e76312fbb5::getLoader()
#4 /tmp/vendor/phpunit/phpunit/phpunit(96): require('...')
#5 /tmp/vendor/bin/phpunit(123): include('...')
#6 {main}
  thrown in /tmp/vendor/open-telemetry/context/Context.php on line 47

Fatal error: Uncaught Exception in /tmp/vendor/open-telemetry/context/Context.php on line 47

What is the expected behavior? No error and working unittest with standard boostrapping

What is the actual behavior? Typed static property OpenTelemetry\Context\Context::$spanContextKey must not be accessed before initialization

Additional context Suggestion:

bobstrecansky commented 8 months ago

@dhempel - what's your intent for using SpanContextKey before initialization? Curious as to how you found this.

dhempel commented 8 months ago

@bobstrecansky Hi, I don't want to modify the initialization. I'm also surprised and thought it might be a PHP 8.2 relevant issue when autoloading the package open-telemetry/context, as there the script fiber/initialize_fiber_handler.php is running, which executes the function Context::setStorage which leads to this error.

dhempel commented 8 months ago

@bobstrecansky: Hi again, maybe it is caused by my unit tests which use @backupStaticAttributes enabled in this case.

dhempel commented 8 months ago

@bobstrecansky Otherwise i can't reproduce this issue within a simple cli script.

Nevay commented 8 months ago

Context::$spanContextKey is initialized on Context construction during the first call to Context::getRoot() (which is also called by Context::getCurrent()); the static property is only accessed in instance methods -> it should not be possible to be uninitialized under normal circumstances. Context::setStorage() should not access the context at all.

Which version of PHPUnit and sebastian/global-state are you using / can you provide a PHPUnit setup that reproduces this issue? Normally @backupStaticAttributes should skip uninitialized properties.

dhempel commented 8 months ago

We are still using an old version of Phpunit, which is probably related to the error:

name     : phpunit/phpunit
versions : * 8.5.36 
....
sebastian/global-state ^3.0.0
dhempel commented 8 months ago

I will provide a setup asap to reproduce this.

dhempel commented 8 months ago

Hello everyone, I can't easily reproduce it and I don't have the time to provide the simple example.