ASPNET.Core default Environment settings are cleaned, if used. (#554)

Co-authored-by: Brian Surowiec <[email protected]>
Co-authored-by: Bruno Garcia <[email protected]>

CHANGELOG.md

@@ -1,6 +1,7 @@
 # unreleased
 
 * Set the Environment setting to 'production' if none was provided. (#550) @PureKrome
+* ASPNET.Core hosting environment is set to 'production' / 'development' (notice lower casing) if no custom options.Enviroment is set. (#554) @PureKrome
 * Add most popular libraries to InAppExclude #555 (@bruno-garcia)
 
 # 3.0.0-alpha.1

src/Sentry.AspNetCore/Constants.cs

@@ -4,5 +4,19 @@ internal static class Constants
     {
         // See: https://github.com/getsentry/sentry-release-registry
         public const string SdkName = "sentry.dotnet.aspnetcore";
+
+        public static string ASPNETCoreProductionEnvironmentName =>
+#if NETSTANDARD2_0
+             "Production";
+#else
+             Microsoft.Extensions.Hosting.Environments.Production;
+#endif
+
+        public static string ASPNETCoreDevelopmentEnvironmentName =>
+#if NETSTANDARD2_0
+             "Development";
+#else
+             Microsoft.Extensions.Hosting.Environments.Development;
+#endif
     }
 }

src/Sentry.AspNetCore/SentryAspNetCoreOptionsSetup.cs

@@ -3,6 +3,7 @@
 using Microsoft.Extensions.Options;
 using Sentry.Extensions.Logging;
 using Sentry.Internal;
+using Microsoft.Extensions.Hosting;
 #if NETSTANDARD2_0
 using IHostingEnvironment = Microsoft.AspNetCore.Hosting.IHostingEnvironment;
 #else
@@ -25,10 +26,42 @@ public override void Configure(SentryAspNetCoreOptions options)
         {
             base.Configure(options);
 
-            // Don't override user defined value
-            options.Environment ??=
-                EnvironmentLocator.Locate() // Sentry specific environment takes precedence #92
-                ?? _hostingEnvironment.EnvironmentName;
+            // Don't override user defined value.
+            if (string.IsNullOrWhiteSpace(options.Environment))
+            {
+                var locatedEnvironment = EnvironmentLocator.Locate();
+                if (!string.IsNullOrWhiteSpace(locatedEnvironment))
+                {
+                    // Sentry specific environment takes precedence #92.
+                    options.Environment = locatedEnvironment;
+                }
+                else
+                {
+                    // NOTE: Sentry prefers to have it's environment setting to be all lower case.
+                    //       .NET Core sets the ENV variable to 'Production' (upper case P) or
+                    //       'Development' (upper case D) which conflicts with the Sentry recommendation.
+                    //       As such, we'll be kind and override those values, here ... if applicable.
+                    // Assumption: The Hosting Environment is always set.
+                    //             If not set by a developer, then the framework will auto set it.
+                    //             Alternatively, developers might set this to a CUSTOM value, which we
+                    //             need to respect (especially the case-sensitivity).
+                    //             REF: https://docs.microsoft.com/en-us/aspnet/core/fundamentals/environments
+
+                    if (_hostingEnvironment.EnvironmentName.Equals(Constants.ASPNETCoreProductionEnvironmentName))
+                    {
+                        options.Environment = Internal.Constants.ProductionEnvironmentSetting;
+                    }
+                    else if (_hostingEnvironment.EnvironmentName.Equals(Constants.ASPNETCoreDevelopmentEnvironmentName))
+                    {
+                        options.Environment = Internal.Constants.DevelopmentEnvironmentSetting;
+                    }
+                    else
+                    {
+                        // Use the value set by the developer.
+                        options.Environment = _hostingEnvironment.EnvironmentName;
+                    }
+                }
+            }
 
             options.AddLogEntryFilter((category, level, eventId, exception)
                 // https://github.com/aspnet/KestrelHttpServer/blob/0aff4a0440c2f393c0b98e9046a8e66e30a56cb0/src/Kestrel.Core/Internal/Infrastructure/KestrelTrace.cs#L33

src/Sentry/Internal/Constants.cs

@@ -24,7 +24,9 @@ internal static class Constants
         /// Default Sentry environment setting.
         /// </summary>
         /// <remarks>Best Sentry practice is to always try and have a value for this setting.</remarks>
-        public const string DefaultEnvironmentSetting = "production";
+        public const string ProductionEnvironmentSetting = "production";
+
+        public const string DevelopmentEnvironmentSetting = "development";
 
         // See: https://github.com/getsentry/sentry-release-registry
         public const string SdkName = "sentry.dotnet";

src/Sentry/Internal/MainSentryEventProcessor.cs

@@ -143,7 +143,7 @@ public SentryEvent Process(SentryEvent @event)
                 var foundEnvironment = EnvironmentLocator.Locate();
                 @event.Environment = string.IsNullOrWhiteSpace(foundEnvironment)
                     ? string.IsNullOrWhiteSpace(_options.Environment)
-                        ? Constants.DefaultEnvironmentSetting
+                        ? Constants.ProductionEnvironmentSetting
                         : _options.Environment
                     : foundEnvironment;
             }

test/Sentry.AspNetCore.Tests/SentryAspNetCoreOptionsSetupTests.cs

@@ -8,6 +8,7 @@
 using Microsoft.Extensions.Logging.Configuration;
 using NSubstitute;
 using Xunit;
+using Sentry.Testing;
 
 namespace Sentry.AspNetCore.Tests
 {
@@ -39,5 +40,50 @@ public void Filters_KestrelEventId1_WithException_NotFiltered()
             _sut.Configure(_target);
             Assert.DoesNotContain(_target.Filters, f => f.Filter("Microsoft.AspNetCore.Server.Kestrel", LogLevel.Trace, 1, null));
         }
+
+        [Theory]
+        [InlineData("foo", "Production", "foo")] // Custom - set. Rest ignored.
+        [InlineData("Production", "Production", "Production")] // Custom - set. Rest ignored. NOTE: Custom value _happens_ to be the common ASPNET_ENVIRONMENT PROD setting.
+        [InlineData(null, "Production", "production")] // Custom - nothing set. ASPNET_ENVIRONMENT is default PROD.
+        [InlineData("", "Production", "production")] // Custom - nothing set. ASPNET_ENVIRONMENT is default PROD.
+        [InlineData(null, "Development", "development")] // Custom - nothing set. ASPNET_ENVIRONMENT is default DEV.
+        [InlineData("", "Development", "development")] // Custom - nothing set. ASPNET_ENVIRONMENT is default DEV.
+        [InlineData(null, "production", "production")] // Custom - nothing set. ASPNET_ENVIRONMENT is custom (notice lowercase 'p').
+        [InlineData(null, "development", "development")] // Custom - nothing set. ASPNET_ENVIRONMENT is custom (notice lowercase 'd').
+        public void Filters_Environment_CustomOrASPNETEnvironment_Set(string environment, string hostingEnvironmentSetting, string expectedEnvironment)
+        {
+            // Arrange.
+            var hostingEnvironment = Substitute.For<IHostingEnvironment>();
+            hostingEnvironment.EnvironmentName = hostingEnvironmentSetting;
+
+            var sut = new SentryAspNetCoreOptionsSetup(
+                Substitute.For<ILoggerProviderConfiguration<SentryAspNetCoreLoggerProvider>>(),
+                hostingEnvironment);
+
+            //const string environment = "some environment";
+            _target.Environment = environment;
+
+            // Act.
+            sut.Configure(_target);
+
+            // Assert.
+            Assert.Equal(expectedEnvironment, _target.Environment);
+        }
+
+        [Theory]
+        [InlineData("foo")] // Random setting.
+        [InlineData("Production")] // Custom setting which is the same as ASPNET_ENVIRONMENT. But because this is manually set, don't change it.
+        public void Filters_Environment_SentryEnvironment_Set(string environment)
+        {
+            // Arrange.
+            EnvironmentVariableGuard.WithVariable(Internal.Constants.EnvironmentEnvironmentVariable, environment, () =>
+            {
+                // Act.
+                _sut.Configure(_target);
+
+                // Assert.
+                Assert.Equal(environment, _target.Environment);
+            });
+        }
     }
 }

test/Sentry.Tests/Internals/MainSentryEventProcessorTests.cs

@@ -165,9 +165,9 @@ public void Process_NoReleaseOnOptions_SameAsCachedVersion()
         }
 
         [Theory]
-        [InlineData(null, Constants.DefaultEnvironmentSetting)] // Missing: will get default value.
-        [InlineData("", Constants.DefaultEnvironmentSetting)] // Missing: will get default value.
-        [InlineData(" ", Constants.DefaultEnvironmentSetting)] // Missing: will get default value.
+        [InlineData(null, Constants.ProductionEnvironmentSetting)] // Missing: will get default value.
+        [InlineData("", Constants.ProductionEnvironmentSetting)] // Missing: will get default value.
+        [InlineData(" ", Constants.ProductionEnvironmentSetting)] // Missing: will get default value.
         [InlineData("a", "a")] // Provided: nothing will change.
         [InlineData("production", "production")] // Provided: nothing will change. (value has a lower case 'p', different to default value)
         [InlineData("aBcDe F !@#$ gHi", "aBcDe F !@#$ gHi")] // Provided: nothing will change. (Case check)
@@ -183,9 +183,9 @@ public void Process_EnvironmentOnOptions_SetToEvent(string environment, string e
         }
 
         [Theory]
-        [InlineData(null, Constants.DefaultEnvironmentSetting)] // Missing: will get default value.
-        [InlineData("", Constants.DefaultEnvironmentSetting)] // Missing: will get default value.
-        [InlineData(" ", Constants.DefaultEnvironmentSetting)] // Missing: will get default value.
+        [InlineData(null, Constants.ProductionEnvironmentSetting)] // Missing: will get default value.
+        [InlineData("", Constants.ProductionEnvironmentSetting)] // Missing: will get default value.
+        [InlineData(" ", Constants.ProductionEnvironmentSetting)] // Missing: will get default value.
         [InlineData("a", "a")] // Provided: nothing will change.
         [InlineData("Production", "Production")] // Provided: nothing will change. (value has a upper case 'p', different to default value)
         [InlineData("aBcDe F !@#$ gHi", "aBcDe F !@#$ gHi")] // Provided: nothing will change. (Case check)