Support for SENTRY_ENVIRONMENT environment variable

[NiclasvanEyk]

Motivation

By default the PHP-SDK uses the value of the SENTRY_ENVIRONMENT environment variable. In the Laravel-SDK this behaviour is overridden, as Laravel implements its own logic to set the environment.

This makes a lot of sense, but there are certain scenarios where one might wants to have different values for the Sentry environment and the Application environment. For example the @production-directive for Blade can only be properly used, if you set the Laravel environment to 'production'.

As an example, we often have the scenario at work, that we deploy a software release in multiple environments, but each environment runs in "production"-mode. We adjust the environment for Sentry to 'customer-abc', 'customer-xyz' for each deployment individually, while the Laravel environment is set to'production' for all deployments.

We do this each time, by adding

'environment' => env('SENTRY_ENVIRONMENT'),

to our config/sentry.php-file every time, but I feel like this could be also useful for other users.

Contents

This PR adjusts the defaults to respect the value of the SENTRY_ENVIRONMENT environment variable while still using the Laravel environment as a fallback. No breaking changes are introduced and this way this SDK behaves more like the underlying PHP-SDK.

[stayallive]

A env() call doesn't work in combination with config caching, so this solution would not work after php artisan config:cache is used.

I think this might be very specific to your application since most applications don't have another environment and/or would use tags to differentiate between tenants in a multi-tenant situation.

I'm not against this change (if changed so it would work with config caching) but I'm thinking this might be a little too specific for your use case to add as a default. I might be open to add this to the config/sentry.php template file instead which would also solve the config cache issue. What do you think?

[stayallive]

After reading the docs and your comment a bit better.

Let's add this to the default config/sentry.php instead and keep the fallback on the Laravel app environment like you've done. This we way honor the SENTRY_ENVIRONMENT correctly and it works with config caching and we have a fallback on a Laravel specific value. Sounds good?

This would have to look a little something like this I think:

    $options = \array_merge(
        [
            'prefixes' => [$basePath],
            'in_app_exclude' => ["{$basePath}/vendor"],
        ],
        $userConfig
    );

    $options['environment'] = $options['environment'] ?? $this->app->environment(),

[NiclasvanEyk]

You're right, I always forget about config caching and its behaviour with the env-function.

I also like your suggestion, as it also improves the discoverability of this feature. Will change it soon.

By the way I was a bit surprised that you still run the tests on older versions of Laravel. I first used the Illuminate\Support\Env-class for the retrieval of the environment variable value, but the tests failed for Laravel 5.1, so all other test runs got aborted.

In the readme you state:

Laravel >= 7.x.x on PHP >= 7.2 is supported starting from 1.7.0

so why do you still run the tests for older versions on the master branch? Just asking out of curiosity.

[stayallive]

We still support Laravel 5.0+ and PHP 7.1+ (read the 2 lines before it) we don’t only support Laravel 7. It’s meant to say we started supporting Laravel 7 on version 1.7.0.

[NiclasvanEyk]

Ah, thanks for the clarification, I did not read that part correctly.

[NiclasvanEyk]

Let me know if there are still changes that need to be made, like the wording of the comment in the config file, as this is user facing.
Otherwise I would be done for now with the changes.

[stayallive]

@NiclasvanEyk sorry, I thought you had not pushed yet 😄

I've made a few tiny adjustments to this feature, should all work the same still but possibly makes the working a bit more clear.

Let me know if you think otherwise!

[stayallive]

Thanks for making this contribution, much appreciated 👍

[stayallive]

I'm not sure why this was never merged, thanks again!