fix: Fix a crash that can occur when the profiler logs certain characters. (#1982) (#2109)

Author: nrcventura

src/Agent/NewRelic/Home/Home.csproj

@@ -13,7 +13,7 @@
   </Target>
 
   <ItemGroup>
-    <PackageReference Include="NewRelic.Agent.Internal.Profiler" Version="10.18.0.15"/>
+    <PackageReference Include="NewRelic.Agent.Internal.Profiler" Version="10.20.0.8"/>
   </ItemGroup>
 
 </Project>

src/Agent/NewRelic/Profiler/Profiler/CorProfilerCallbackImpl.h

@@ -23,6 +23,7 @@
 #include <string>
 #include <thread>
 #include <utility>
+#include <codecvt>
 
 #ifdef PAL_STDCPP_COMPAT
 #include "UnixSystemCalls.h"
@@ -1091,6 +1092,8 @@ namespace NewRelic { namespace Profiler {
                     xstring_t logfilename(nrlog::DefaultFileLogLocation(_systemCalls).GetPathAndFileName());
                     std::string wlogfilename(std::begin(logfilename), std::end(logfilename));
                     nrlog::StdLog.get_dest().open(wlogfilename);
+                    // Imbue with locale and codecvt facet is used to allow the log file to write non-ascii chars to the log
+                    nrlog::StdLog.get_dest().imbue(std::locale(std::locale::classic(), new std::codecvt_utf8<wchar_t>));
                     nrlog::StdLog.get_dest().exceptions(std::wostream::failbit | std::wostream::badbit);
                     LogInfo("Logger initialized.");
                 }

tests/Agent/IntegrationTests/ContainerApplications/SmokeTestApp/Controllers/WeatherForecastController.cs

@@ -4,8 +4,11 @@
 using System;
 using System.Collections.Generic;
 using System.Linq;
+using System.Runtime.CompilerServices;
+using System.Threading;
 using Microsoft.AspNetCore.Mvc;
 using Microsoft.Extensions.Logging;
+using NewRelic.Api.Agent;
 
 namespace ContainerizedAspNetCoreApp.Controllers;
 
@@ -28,6 +31,9 @@ public WeatherForecastController(ILogger<WeatherForecastController> logger)
     [HttpGet]
     public IEnumerable<WeatherForecast> Get()
     {
+        // This name of this method call contains characters outside of the ascii
+        // range of characters.
+        MethodWithSpecialCharacter\u0435();
         return Enumerable.Range(1, 5).Select(index => new WeatherForecast
             {
                 Date = DateTime.Now.AddDays(index),
@@ -36,4 +42,17 @@ public IEnumerable<WeatherForecast> Get()
             })
             .ToArray();
     }
+
+    /// <summary>
+    /// The е in Mеthod is not a normal e character, it is char code \u0435.
+    /// This method is used to validate that we support instrumenting methods
+    /// with these types of names, and that the profiler does not crash.
+    /// </summary>
+    [Trace]
+    [MethodImpl(MethodImplOptions.NoOptimization | MethodImplOptions.NoInlining)]
+    private void MethodWithSpecialCharacter\u0435()
+    {
+        // This just similates doing a blocking external call
+        Thread.Sleep(100);
+    }
 }

tests/Agent/IntegrationTests/ContainerApplications/SmokeTestApp/SmokeTestApp.csproj

@@ -9,6 +9,7 @@
 
   <ItemGroup>
     <PackageReference Include="Microsoft.VisualStudio.Azure.Containers.Tools.Targets" Version="1.17.2" />
+    <PackageReference Include="NewRelic.Agent.Api" Version="10.20.0" />
   </ItemGroup>
 
 </Project>

tests/Agent/IntegrationTests/ContainerApplications/docker-compose.yml

@@ -67,6 +67,10 @@ services:
             service: smoketestapp
         build:
             dockerfile: SmokeTestApp/Dockerfile.amazon
+    LinuxUnicodeLogFileApp:
+        extends:
+            file: docker-compose-smoketestapp.yml
+            service: smoketestapp
 
 networks:
     default:

tests/Agent/IntegrationTests/ContainerIntegrationTests/ContainerFixtures/LinuxUnicodeLogFileTestFixture.cs

@@ -0,0 +1,15 @@
+// Copyright 2020 New Relic, Inc. All rights reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+namespace NewRelic.Agent.ContainerIntegrationTests.ContainerFixtures
+{
+    public class LinuxUnicodeLogFileTestFixture : LinuxSmokeTestFixtureBase
+    {
+        private static readonly string Dockerfile = "SmokeTestApp/Dockerfile";
+        private static readonly string ApplicationDirectoryName = "LinuxUnicodeLogfileTestApp";
+        private static readonly ContainerApplication.Architecture Architecture = ContainerApplication.Architecture.X64;
+        private static readonly string DistroTag = "jammy";
+
+        public LinuxUnicodeLogFileTestFixture() : base(ApplicationDirectoryName, DistroTag, Architecture, Dockerfile) { }
+    }
+}

tests/Agent/IntegrationTests/ContainerIntegrationTests/LinuxUnicodeLogFileTest.cs

@@ -0,0 +1,59 @@
+// Copyright 2020 New Relic, Inc. All rights reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+using NewRelic.Agent.IntegrationTestHelpers;
+using Xunit.Abstractions;
+using Xunit;
+using NewRelic.Agent.ContainerIntegrationTests.ContainerFixtures;
+using System;
+
+namespace ContainerIntegrationTests
+{
+    /// <summary>
+    /// This test is meant to prevent any regressions from occurring when profiler log lines containing
+    /// character codes outside of the ascii range are written to log files. Older profiler versions
+    /// could trigger an error or crash when this happened. Before the profiler change, no transactions
+    /// would be created by the test application, with the profiler change, the test transaction should be
+    /// created successfully.
+    /// </summary>
+    public class LinuxUnicodeLogFileTest : NewRelicIntegrationTest<DebianX64SmokeTestFixture>
+    {
+        private readonly DebianX64SmokeTestFixture _fixture;
+
+        public LinuxUnicodeLogFileTest(DebianX64SmokeTestFixture fixture, ITestOutputHelper output) : base(fixture)
+        {
+            _fixture = fixture;
+            _fixture.TestLogger = output;
+
+            _fixture.Actions(setupConfiguration: () =>
+            {
+                var configModifier = new NewRelicConfigModifier(_fixture.DestinationNewRelicConfigFilePath);
+                configModifier.ConfigureFasterMetricsHarvestCycle(10);
+                // The original problem only seemed to occur with some of the finest level log lines
+                // and it did not occur with console logs.
+                configModifier.SetLogLevel("finest");
+            },
+                exerciseApplication: () =>
+                {
+                    _fixture.ExerciseApplication();
+
+                    _fixture.Delay(11); // wait long enough to ensure a metric harvest occurs after we exercise the app
+                    _fixture.AgentLog.WaitForLogLine(AgentLogBase.HarvestFinishedLogLineRegex, TimeSpan.FromSeconds(11));
+
+                    // shut down the container and wait for the agent log to see it
+                    _fixture.ShutdownRemoteApplication();
+                    _fixture.AgentLog.WaitForLogLine(AgentLogBase.ShutdownLogLineRegex, TimeSpan.FromSeconds(10));
+                });
+
+            _fixture.Initialize();
+        }
+
+        [Fact]
+        public void Test()
+        {
+            var actualMetrics = _fixture.AgentLog.GetMetrics();
+
+            Assert.Contains(actualMetrics, m => m.MetricSpec.Name.Equals("WebTransaction/MVC/WeatherForecast/Get"));
+        }
+    }
+}