Fix environment name casing issue (#1861)

getsentry · Aug 19, 2022 · 9f98ec1 · 9f98ec1
1 parent 432a8f9
commit 9f98ec1
Show file tree

Hide file tree

Showing 11 changed files with 111 additions and 65 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -20,6 +20,7 @@
 - Fix logging loop with Serilog sentry ([#1828](https://github.com/getsentry/sentry-dotnet/pull/1828))
 - Skip attachment if stream is empty ([#1854](https://github.com/getsentry/sentry-dotnet/pull/1854))
 - Allow some mobile options to be modified from defaults ([#1857](https://github.com/getsentry/sentry-dotnet/pull/1857))
+- Fix environment name casing issue ([#1861](https://github.com/getsentry/sentry-dotnet/pull/1861))
 
 ## 3.20.1
 

diff --git a/SentryAspNetCore.slnf b/SentryAspNetCore.slnf
@@ -0,0 +1,16 @@
+{
+  "solution": {
+    "path": "Sentry.sln",
+    "projects": [
+      "src\\Sentry\\Sentry.csproj",
+      "src\\Sentry.AspNetCore\\Sentry.AspNetCore.csproj",
+      "src\\Sentry.AspNetCore.Grpc\\Sentry.AspNetCore.Grpc.csproj",
+      "src\\Sentry.Extensions.Logging\\Sentry.Extensions.Logging.csproj",
+      "test\\Sentry.Testing\\Sentry.Testing.csproj",
+      "test\\Sentry.Tests\\Sentry.Tests.csproj",
+      "test\\Sentry.AspNetCore.Tests\\Sentry.AspNetCore.Tests.csproj",
+      "test\\Sentry.AspNetCore.Grpc.Tests\\Sentry.AspNetCore.Grpc.Tests.csproj",
+      "test\\Sentry.Extensions.Logging.Tests\\Sentry.Extensions.Logging.Tests.csproj",
+    ]
+  }
+}
diff --git a/src/Sentry.AspNetCore/SentryAspNetCoreOptions.cs b/src/Sentry.AspNetCore/SentryAspNetCoreOptions.cs
@@ -1,7 +1,14 @@
+using Microsoft.AspNetCore.Hosting;
 using Microsoft.AspNetCore.Http;
 using Sentry.Extensibility;
 using Sentry.Extensions.Logging;
 
+#if NETSTANDARD2_0
+using IWebHostEnvironment = Microsoft.AspNetCore.Hosting.IHostingEnvironment;
+#else
+using Microsoft.Extensions.Hosting;
+#endif
+
 namespace Sentry.AspNetCore;
 
 /// <summary>
@@ -70,4 +77,51 @@ public SentryAspNetCoreOptions()
         // Don't report Environment.UserName as the user.
         IsEnvironmentUser = false;
     }
+
+    internal void SetEnvironment(IWebHostEnvironment hostingEnvironment)
+    {
+        // Set environment from AspNetCore hosting environment name, if not set already
+        // Note: The SettingLocator will take care of the default behavior and assignment, which takes precedence.
+        //       We only need to do anything here if nothing was found by the locator.
+        if (SettingLocator.GetEnvironment(useDefaultIfNotFound: false) is not null)
+        {
+            return;
+        }
+
+        if (AdjustStandardEnvironmentNameCasing)
+        {
+            // NOTE: Sentry prefers to have its environment setting to be all lower case.
+            //       .NET Core sets the ENV variable to 'Production' (upper case P),
+            //       'Development' (upper case D) or 'Staging' (upper case S) 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.IsProduction())
+            {
+                Environment = Internal.Constants.ProductionEnvironmentSetting;
+            }
+            else if (hostingEnvironment.IsStaging())
+            {
+                Environment = Internal.Constants.StagingEnvironmentSetting;
+            }
+            else if (hostingEnvironment.IsDevelopment())
+            {
+                Environment = Internal.Constants.DevelopmentEnvironmentSetting;
+            }
+            else
+            {
+                // Use the value set by the developer.
+                Environment = hostingEnvironment.EnvironmentName;
+            }
+        }
+        else
+        {
+            Environment = hostingEnvironment.EnvironmentName;
+        }
+    }
 }
diff --git a/src/Sentry.AspNetCore/SentryAspNetCoreOptionsSetup.cs b/src/Sentry.AspNetCore/SentryAspNetCoreOptionsSetup.cs
@@ -17,16 +17,25 @@ namespace Sentry.AspNetCore
     /// </summary>
     public class SentryAspNetCoreOptionsSetup : ConfigureFromConfigurationOptions<SentryAspNetCoreOptions>
     {
-        private readonly IHostingEnvironment _hostingEnvironment;
+        /// <summary>
+        /// Creates a new instance of <see cref="SentryAspNetCoreOptionsSetup"/>.
+        /// </summary>
+        public SentryAspNetCoreOptionsSetup(
+            ILoggerProviderConfiguration<SentryAspNetCoreLoggerProvider> providerConfiguration)
+            : base(providerConfiguration.Configuration)
+        {
+        }
 
         /// <summary>
         /// Creates a new instance of <see cref="SentryAspNetCoreOptionsSetup"/>.
         /// </summary>
+        [Obsolete("Use constructor with no IHostingEnvironment")]
         public SentryAspNetCoreOptionsSetup(
             ILoggerProviderConfiguration<SentryAspNetCoreLoggerProvider> providerConfiguration,
             IHostingEnvironment hostingEnvironment)
             : base(providerConfiguration.Configuration)
-            => _hostingEnvironment = hostingEnvironment;
+        {
+        }
 
         /// <summary>
         /// Configures the <see cref="SentryAspNetCoreOptions"/>.
@@ -35,48 +44,6 @@ public override void Configure(SentryAspNetCoreOptions options)
         {
             base.Configure(options);
 
-            // Set environment from AspNetCore hosting environment name, if not set already
-            // Note: The SettingLocator will take care of the default behavior and assignment, which takes precedence.
-            //       We only need to do anything here if nothing was found by the locator.
-            if (options.SettingLocator.GetEnvironment(useDefaultIfNotFound: false) is null)
-            {
-                if (!options.AdjustStandardEnvironmentNameCasing)
-                {
-                    options.Environment = _hostingEnvironment.EnvironmentName;
-                }
-                else
-                {
-                    // NOTE: Sentry prefers to have its environment setting to be all lower case.
-                    //       .NET Core sets the ENV variable to 'Production' (upper case P),
-                    //       'Development' (upper case D) or 'Staging' (upper case S) 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.IsProduction())
-                    {
-                        options.Environment = Internal.Constants.ProductionEnvironmentSetting;
-                    }
-                    else if (_hostingEnvironment.IsStaging())
-                    {
-                        options.Environment = Internal.Constants.StagingEnvironmentSetting;
-                    }
-                    else if (_hostingEnvironment.IsDevelopment())
-                    {
-                        options.Environment = Internal.Constants.DevelopmentEnvironmentSetting;
-                    }
-                    else
-                    {
-                        // Use the value set by the developer.
-                        options.Environment = _hostingEnvironment.EnvironmentName;
-                    }
-                }
-            }
-
             options.AddLogEntryFilter((category, _, eventId, _)
                 // https://github.com/aspnet/KestrelHttpServer/blob/0aff4a0440c2f393c0b98e9046a8e66e30a56cb0/src/Kestrel.Core/Internal/Infrastructure/KestrelTrace.cs#L33
                 // 13 = Application unhandled exception, which is captured by the middleware so the LogError of kestrel ends up as a duplicate with less info

diff --git a/src/Sentry.AspNetCore/SentryWebHostBuilderExtensions.cs b/src/Sentry.AspNetCore/SentryWebHostBuilderExtensions.cs
@@ -49,7 +49,13 @@ public static IWebHostBuilder UseSentry(
         this IWebHostBuilder builder,
         Action<WebHostBuilderContext, SentryAspNetCoreOptions>? configureOptions)
         => builder.UseSentry((context, sentryBuilder) =>
-            sentryBuilder.AddSentryOptions(options => configureOptions?.Invoke(context, options)));
+        {
+            sentryBuilder.AddSentryOptions(options =>
+            {
+                configureOptions?.Invoke(context, options);
+                options.SetEnvironment(context.HostingEnvironment);
+            });
+        });
 
     /// <summary>
     /// Uses Sentry integration.
@@ -90,6 +96,7 @@ public static IWebHostBuilder UseSentry(
 
             var sentryBuilder = logging.Services.AddSentry();
             configureSentry?.Invoke(context, sentryBuilder);
+
         });
 
         _ = builder.ConfigureServices(c => _ = c.AddTransient<IStartupFilter, SentryStartupFilter>());

diff --git a/test/Sentry.AspNetCore.Tests/ApiApprovalTests.Run.Core2_1.verified.txt b/test/Sentry.AspNetCore.Tests/ApiApprovalTests.Run.Core2_1.verified.txt
@@ -49,6 +49,8 @@ namespace Sentry.AspNetCore
     }
     public class SentryAspNetCoreOptionsSetup : Microsoft.Extensions.Options.ConfigureFromConfigurationOptions<Sentry.AspNetCore.SentryAspNetCoreOptions>
     {
+        public SentryAspNetCoreOptionsSetup(Microsoft.Extensions.Logging.Configuration.ILoggerProviderConfiguration<Sentry.AspNetCore.SentryAspNetCoreLoggerProvider> providerConfiguration) { }
+        [System.Obsolete("Use constructor with no IHostingEnvironment")]
         public SentryAspNetCoreOptionsSetup(Microsoft.Extensions.Logging.Configuration.ILoggerProviderConfiguration<Sentry.AspNetCore.SentryAspNetCoreLoggerProvider> providerConfiguration, Microsoft.AspNetCore.Hosting.IHostingEnvironment hostingEnvironment) { }
         public override void Configure(Sentry.AspNetCore.SentryAspNetCoreOptions options) { }
     }

diff --git a/test/Sentry.AspNetCore.Tests/ApiApprovalTests.Run.Core3_1.verified.txt b/test/Sentry.AspNetCore.Tests/ApiApprovalTests.Run.Core3_1.verified.txt
@@ -49,6 +49,8 @@ namespace Sentry.AspNetCore
     }
     public class SentryAspNetCoreOptionsSetup : Microsoft.Extensions.Options.ConfigureFromConfigurationOptions<Sentry.AspNetCore.SentryAspNetCoreOptions>
     {
+        public SentryAspNetCoreOptionsSetup(Microsoft.Extensions.Logging.Configuration.ILoggerProviderConfiguration<Sentry.AspNetCore.SentryAspNetCoreLoggerProvider> providerConfiguration) { }
+        [System.Obsolete("Use constructor with no IHostingEnvironment")]
         public SentryAspNetCoreOptionsSetup(Microsoft.Extensions.Logging.Configuration.ILoggerProviderConfiguration<Sentry.AspNetCore.SentryAspNetCoreLoggerProvider> providerConfiguration, Microsoft.AspNetCore.Hosting.IWebHostEnvironment hostingEnvironment) { }
         public override void Configure(Sentry.AspNetCore.SentryAspNetCoreOptions options) { }
     }

diff --git a/test/Sentry.AspNetCore.Tests/ApiApprovalTests.Run.DotNet4_6.verified.txt b/test/Sentry.AspNetCore.Tests/ApiApprovalTests.Run.DotNet4_6.verified.txt
@@ -49,6 +49,8 @@ namespace Sentry.AspNetCore
     }
     public class SentryAspNetCoreOptionsSetup : Microsoft.Extensions.Options.ConfigureFromConfigurationOptions<Sentry.AspNetCore.SentryAspNetCoreOptions>
     {
+        public SentryAspNetCoreOptionsSetup(Microsoft.Extensions.Logging.Configuration.ILoggerProviderConfiguration<Sentry.AspNetCore.SentryAspNetCoreLoggerProvider> providerConfiguration) { }
+        [System.Obsolete("Use constructor with no IHostingEnvironment")]
         public SentryAspNetCoreOptionsSetup(Microsoft.Extensions.Logging.Configuration.ILoggerProviderConfiguration<Sentry.AspNetCore.SentryAspNetCoreLoggerProvider> providerConfiguration, Microsoft.AspNetCore.Hosting.IHostingEnvironment hostingEnvironment) { }
         public override void Configure(Sentry.AspNetCore.SentryAspNetCoreOptions options) { }
     }

diff --git a/test/Sentry.AspNetCore.Tests/ApiApprovalTests.Run.DotNet5_0.verified.txt b/test/Sentry.AspNetCore.Tests/ApiApprovalTests.Run.DotNet5_0.verified.txt
@@ -49,6 +49,8 @@ namespace Sentry.AspNetCore
     }
     public class SentryAspNetCoreOptionsSetup : Microsoft.Extensions.Options.ConfigureFromConfigurationOptions<Sentry.AspNetCore.SentryAspNetCoreOptions>
     {
+        public SentryAspNetCoreOptionsSetup(Microsoft.Extensions.Logging.Configuration.ILoggerProviderConfiguration<Sentry.AspNetCore.SentryAspNetCoreLoggerProvider> providerConfiguration) { }
+        [System.Obsolete("Use constructor with no IHostingEnvironment")]
         public SentryAspNetCoreOptionsSetup(Microsoft.Extensions.Logging.Configuration.ILoggerProviderConfiguration<Sentry.AspNetCore.SentryAspNetCoreLoggerProvider> providerConfiguration, Microsoft.AspNetCore.Hosting.IWebHostEnvironment hostingEnvironment) { }
         public override void Configure(Sentry.AspNetCore.SentryAspNetCoreOptions options) { }
     }

diff --git a/test/Sentry.AspNetCore.Tests/ApiApprovalTests.Run.DotNet6_0.verified.txt b/test/Sentry.AspNetCore.Tests/ApiApprovalTests.Run.DotNet6_0.verified.txt
@@ -49,6 +49,8 @@ namespace Sentry.AspNetCore
     }
     public class SentryAspNetCoreOptionsSetup : Microsoft.Extensions.Options.ConfigureFromConfigurationOptions<Sentry.AspNetCore.SentryAspNetCoreOptions>
     {
+        public SentryAspNetCoreOptionsSetup(Microsoft.Extensions.Logging.Configuration.ILoggerProviderConfiguration<Sentry.AspNetCore.SentryAspNetCoreLoggerProvider> providerConfiguration) { }
+        [System.Obsolete("Use constructor with no IHostingEnvironment")]
         public SentryAspNetCoreOptionsSetup(Microsoft.Extensions.Logging.Configuration.ILoggerProviderConfiguration<Sentry.AspNetCore.SentryAspNetCoreLoggerProvider> providerConfiguration, Microsoft.AspNetCore.Hosting.IWebHostEnvironment hostingEnvironment) { }
         public override void Configure(Sentry.AspNetCore.SentryAspNetCoreOptions options) { }
     }

diff --git a/test/Sentry.AspNetCore.Tests/SentryAspNetCoreOptionsSetupTests.cs b/test/Sentry.AspNetCore.Tests/SentryAspNetCoreOptionsSetupTests.cs
@@ -1,19 +1,19 @@
-#if NETCOREAPP2_1 || NET461
-using IHostingEnvironment = Microsoft.AspNetCore.Hosting.IHostingEnvironment;
-#else
-using IHostingEnvironment = Microsoft.AspNetCore.Hosting.IWebHostEnvironment;
-#endif
 using Microsoft.Extensions.Logging;
 using Microsoft.Extensions.Logging.Configuration;
 using Sentry.Testing;
 
+#if NETCOREAPP3_1_OR_GREATER
+using IHostingEnvironment = Microsoft.AspNetCore.Hosting.IWebHostEnvironment;
+#else
+using IHostingEnvironment = Microsoft.AspNetCore.Hosting.IHostingEnvironment;
+#endif
+
 namespace Sentry.AspNetCore.Tests;
 
 public class SentryAspNetCoreOptionsSetupTests
 {
     private readonly SentryAspNetCoreOptionsSetup _sut = new(
-        Substitute.For<ILoggerProviderConfiguration<SentryAspNetCoreLoggerProvider>>(),
-        Substitute.For<IHostingEnvironment>());
+        Substitute.For<ILoggerProviderConfiguration<SentryAspNetCoreLoggerProvider>>());
 
     private readonly SentryAspNetCoreOptions _target = new();
 
@@ -55,16 +55,10 @@ public void Filters_Environment_CustomOrASPNETEnvironment_Set(string environment
         // 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);
+        _target.SetEnvironment(hostingEnvironment);
 
         // Assert.
         Assert.Equal(expectedEnvironment, _target.Environment);
@@ -85,15 +79,11 @@ public void Filters_Environment_AdjustStandardEnvironmentNameCasing_AffectsSentr
         var hostingEnvironment = Substitute.For<IHostingEnvironment>();
         hostingEnvironment.EnvironmentName = hostingEnvironmentSetting;
 
-        var sut = new SentryAspNetCoreOptionsSetup(
-            Substitute.For<ILoggerProviderConfiguration<SentryAspNetCoreLoggerProvider>>(),
-            hostingEnvironment);
-
         _target.Environment = environment;
         _target.AdjustStandardEnvironmentNameCasing = adjustStandardEnvironmentNameCasingSetting;
 
         // Act.
-        sut.Configure(_target);
+        _target.SetEnvironment(hostingEnvironment);
 
         // Assert.
         Assert.Equal(expectedEnvironment, _target.Environment);
@@ -105,10 +95,11 @@ public void Filters_Environment_AdjustStandardEnvironmentNameCasing_AffectsSentr
     public void Filters_Environment_SentryEnvironment_Set(string environment)
     {
         // Arrange.
+        var hostingEnvironment = Substitute.For<IHostingEnvironment>();
         _target.FakeSettings().EnvironmentVariables[Internal.Constants.EnvironmentEnvironmentVariable] = environment;
 
         // Act.
-        _sut.Configure(_target);
+        _target.SetEnvironment(hostingEnvironment);
 
         // Assert.
         Assert.Equal(environment, _target.Environment);