Skip to content
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

[opentelemetry-php-contrib] laravel: When loading configuration files, SDK ignores laravel's .env #1436

Open
gmhafiz opened this issue Nov 24, 2024 · 7 comments
Labels
bug Something isn't working

Comments

@gmhafiz
Copy link

gmhafiz commented Nov 24, 2024

Describe your environment

OS: linux debian 12
php: 8.2
laravel: v11
open-telemetry/opentelemetry-auto-laravel: 0.0.28
open-telemetry/sdk: 1.1.2

Steps to reproduce

This is after a normal laravel installation instrumented with open-telemetry/opentelemetry-auto-laravel package. There are two ways of running laravel. Method 1 works with php artisan serve. Method 2 using php-fpm does not read environment variable.

Get it and make a copy of .env file.

git clone [email protected]:gmhafiz/laravel-otlp.git
cd laravel-otlp
cp .env.example .env

Method 1:

Run with

php artisan migrate
php artisan db:seed
php artisan serve

Monitor log for traceID

tail -f storage/logs/laravel.log

Perform a request

curl -v localhost:8000/api/users

Method 2

Run laravel using nginx and php-fpm

docker build -t otlp/laravel .
docker exec -it otlp-laravel php artisan migrate
docker exec -it otlp-laravel php artisan db:seed
docker run --name otlp-laravel --rm -p 8111:80 otlp/laravel

Perform a request

curl -v localhost:8111/api/users

What is the expected behavior?

In both methods, traceID is present. For example.

{"datetime":"2024-11-22T08:27:29.243748+00:00","traceID":"9de4bd0cfce131835f531731e950a397","level":200,"message":"random amount is ","context":[15],"extra":{"hostname":"debianDesktop","app":"Laravel","env":"local"}}

What is the actual behavior?
Method 1 has traceID, but in method 2, otel is not enabled (uses noop) and traceID becomes 0000000000000000000000000000000

Additional context

Even though both methods have OTEL_PHP_AUTOLOAD_ENABLED=true set in .env file, only in the first method otel is enabled.

I am unsure the mechanism of php artisan serve is picking up values from .env

In method 2, what I think happened was the SDK tries to load values using both $_SERVER (from EnvironmentResolver.php) and getenv(). And OTEL_PHP_AUTOLOAD_ENABLED variable is false at this point for some reason.

What I have done, or try to attempt:

  1. I have tried adding required variables directly into server/vhost.conf like below and it works. but I am unsure about hard coding values like these.
location ~ \.php$ {
    ...
    fastcgi_param OTEL_PHP_AUTOLOAD_ENABLED "true";
    fastcgi_param OTEL_SERVICE_NAME "laravel";
    fastcgi_param OTEL_EXPORTER_OTLP_ENDPOINT "http://localhost:4318";
    fastcgi_param OTEL_EXPORTER_OTLP_PROTOCOL "http/json";
    fastcgi_param OTEL_TRACES_EXPORTER "console";
    fastcgi_param OTEL_METRIC_EXPORTER "none";
    fastcgi_param OTEL_LOGS_EXPORTER "console";
    fastcgi_param OTEL_TRACES_SAMPLER_ARG "1";
}
  1. I'm sure adding those variables to server/php.ini will work too because of PhpIniResolver.php, as advised by https://opentelemetry.io/docs/zero-code/php/#phpini-configuration. I want to avoid this method as it will affect all php applications.

While both 1) and 2) works, ideally I want this package to load values using environment variables. But no otel variables are loaded because values set in .env file are not read by the SDK.

Something could be done to open-telemetry/opentelemetry-auto-laravel package to load values from laravel's .env using helper function such as env().

Looking around, many laravel packages publish a config file under config directory. For example in config/database.php, DB_CONNECTION is read using env() helper function.

# config/database.php
<?php

return [
    'default' => env('DB_CONNECTION', 'sqlite'),
]

Laravel reads this config using a config() magic function and uses {{filename.key}} magic string as parameter. For example calling config('database.default') returns the default value set in .env which is 'sqlite'.

Suggestion

Perhaps open-telemetry/opentelemetry-auto-laravel could also publish a config file and then allow it to load those values on top of both EnvironmentResolver.php and PhpIniResolver.php loaded by open-telemetry/sdk package.

This new config file should be limited only for open-telemetry/opentelemetry-auto-laravel package. What do you think?

@gmhafiz gmhafiz added the bug Something isn't working label Nov 24, 2024
@gmhafiz gmhafiz changed the title laravel: When loading configuration files, SDK ignores laravel's .env [opentelemetry-php-contrib] laravel: When loading configuration files, SDK ignores laravel's .env Nov 24, 2024
@ChrisLightfootWild
Copy link
Contributor

The instrumentation is bootstrapped before Laravel, so the .env configuration will not work.

@gmhafiz
Copy link
Author

gmhafiz commented Nov 26, 2024

Got it.

I asked around and https://github.com/vlucas/phpdotenv package has been suggested. It is already been used by laravel too.

I added these lines before EnvironmentResolver() so values .env are injected into $_ENV.

$dotenv = Dotenv::createImmutable($_ENV['HOME']);
$dotenv->load();

Once the values are loaded, EnvironmentResolver() works as usual.

One thing I am unsure is how universal is the key of HOME from $_ENV. In my case it is /var/www

@ChrisLightfootWild
Copy link
Contributor

Configuration must be provided via $_ENV or php.ini. Please see the zero-code docs.

You need to move your OTEL_* configuration outside of your application code, which is what dotenv essentially is.

@gdaszuta
Copy link

gdaszuta commented Nov 28, 2024

EDIT: I was wrong, it's possible to use boolean variables in php.ini, they just need to be quoted, so
OTEL_PHP_AUTOLOAD_ENABLED=true won't work, but OTEL_PHP_AUTOLOAD_ENABLED="true" should work.


Just mentioning, that because #1442 it's partially impossible to instrument laravel now in a meaningful way without some tinkering.

OpenTelemetry instrumentation is loaded and configured in composer autoloader, so it's either env or php.ini, and php.ini doesn't work right now – there is a possibility to create a separate autoloader just for vlucas/phpdotenv to configure opentelemetry in a way that's consistent with Laravel way, but this isn't trivial as the Dotenv class itself has it's fair share of dependencies, so it would be reinventing a wheel.

The issue with environment variables is, that they aren't passed as they are in some cases (cronjobs aren't passing env at all while spawning child process https://github.com/laravel/framework/blob/11.x/src/Illuminate/Console/Scheduling/Event.php#L198), I cannot be 100% sure about Horizon (although it should work), and some component in tests (AFAIR testbench or phpunit) while passing variables is casting boolean to string of value "1" while spawning tests in a separate process.

Only 100% working solution for all Laravel components right now is to prepare a separate initialization script with contents similar to https://github.com/open-telemetry/opentelemetry-php/blob/main/examples/load_config_env.php#L11 and include it at the begining of public/index.php and artisan files (anywhere before require __DIR__.'/vendor/autoload.php';). I guess you could put there a snippet that would read and partially parse your .env file to extract OTEL_* variables and put them in a runtime environment to achieve at least some consistency.

@ChrisLightfootWild
Copy link
Contributor

EDIT: I was wrong, it's possible to use boolean variables in php.ini, they just need to be quoted, so OTEL_PHP_AUTOLOAD_ENABLED=true won't work, but OTEL_PHP_AUTOLOAD_ENABLED="true" should work.

Just mentioning, that because #1442 it's partially impossible to instrument laravel now in a meaningful way without some tinkering.

OpenTelemetry instrumentation is loaded and configured in composer autoloader, so it's either env or php.ini, and php.ini doesn't work right now – there is a possibility to create a separate autoloader just for vlucas/phpdotenv to configure opentelemetry in a way that's consistent with Laravel way, but this isn't trivial as the Dotenv class itself has it's fair share of dependencies, so it would be reinventing a wheel.

The issue with environment variables is, that they aren't passed as they are in some cases (cronjobs aren't passing env at all while spawning child process https://github.com/laravel/framework/blob/11.x/src/Illuminate/Console/Scheduling/Event.php#L198), I cannot be 100% sure about Horizon (although it should work), and some component in tests (AFAIR testbench or phpunit) while passing variables is casting boolean to string of value "1" while spawning tests in a separate process.

Only 100% working solution for all Laravel components right now is to prepare a separate initialization script with contents similar to https://github.com/open-telemetry/opentelemetry-php/blob/main/examples/load_config_env.php#L11 and include it at the begining of public/index.php and artisan files (anywhere before require __DIR__.'/vendor/autoload.php';). I guess you could put there a snippet that would read and partially parse your .env file to extract OTEL_* variables and put them in a runtime environment to achieve at least some consistency.

This doesn't sound like an issue with instrumentation.

If you are calling cron, you typically need to have already dumped your environment out. That way, the spawned process should be able to load from there.

printenv > /etc/environment in an entrypoint, for example.

@bobstrecansky
Copy link
Collaborator

@gmhafiz - is this resolved?

@gmhafiz
Copy link
Author

gmhafiz commented Dec 7, 2024

@gmhafiz - is this resolved?

For production, our option is to hardcode the required variables into nginx virtual host file in order to not to contaminate other applications.

for dev environment, this is less of an issue because many of use use docker compose to set the required variables.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants