From 742d232189ae5c94480711172d5db4219b20c384 Mon Sep 17 00:00:00 2001 From: Chris Ventura <45495992+nrcventura@users.noreply.github.com> Date: Fri, 1 Dec 2023 14:34:49 -0800 Subject: [PATCH] fix: Fix a crash that can occur when the profiler logs certain characters. (#1982) (#2109) --- src/Agent/NewRelic/Home/Home.csproj | 2 +- .../Profiler/CorProfilerCallbackImpl.h | 3 + .../Controllers/WeatherForecastController.cs | 19 ++++++ .../SmokeTestApp/SmokeTestApp.csproj | 1 + .../ContainerApplications/docker-compose.yml | 4 ++ .../LinuxUnicodeLogFileTestFixture.cs | 15 +++++ .../LinuxUnicodeLogFileTest.cs | 59 +++++++++++++++++++ 7 files changed, 102 insertions(+), 1 deletion(-) create mode 100644 tests/Agent/IntegrationTests/ContainerIntegrationTests/ContainerFixtures/LinuxUnicodeLogFileTestFixture.cs create mode 100644 tests/Agent/IntegrationTests/ContainerIntegrationTests/LinuxUnicodeLogFileTest.cs diff --git a/src/Agent/NewRelic/Home/Home.csproj b/src/Agent/NewRelic/Home/Home.csproj index 392fff2f8e..cd0a2a6772 100644 --- a/src/Agent/NewRelic/Home/Home.csproj +++ b/src/Agent/NewRelic/Home/Home.csproj @@ -13,7 +13,7 @@ - + diff --git a/src/Agent/NewRelic/Profiler/Profiler/CorProfilerCallbackImpl.h b/src/Agent/NewRelic/Profiler/Profiler/CorProfilerCallbackImpl.h index 6fc76d4c01..b283aa59c2 100644 --- a/src/Agent/NewRelic/Profiler/Profiler/CorProfilerCallbackImpl.h +++ b/src/Agent/NewRelic/Profiler/Profiler/CorProfilerCallbackImpl.h @@ -23,6 +23,7 @@ #include #include #include +#include #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)); nrlog::StdLog.get_dest().exceptions(std::wostream::failbit | std::wostream::badbit); LogInfo("Logger initialized."); } diff --git a/tests/Agent/IntegrationTests/ContainerApplications/SmokeTestApp/Controllers/WeatherForecastController.cs b/tests/Agent/IntegrationTests/ContainerApplications/SmokeTestApp/Controllers/WeatherForecastController.cs index 0c067ad542..b7e09856e8 100644 --- a/tests/Agent/IntegrationTests/ContainerApplications/SmokeTestApp/Controllers/WeatherForecastController.cs +++ b/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 logger) [HttpGet] public IEnumerable 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 Get() }) .ToArray(); } + + /// + /// 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. + /// + [Trace] + [MethodImpl(MethodImplOptions.NoOptimization | MethodImplOptions.NoInlining)] + private void MethodWithSpecialCharacter\u0435() + { + // This just similates doing a blocking external call + Thread.Sleep(100); + } } diff --git a/tests/Agent/IntegrationTests/ContainerApplications/SmokeTestApp/SmokeTestApp.csproj b/tests/Agent/IntegrationTests/ContainerApplications/SmokeTestApp/SmokeTestApp.csproj index c11e0f4f61..a229c8898e 100644 --- a/tests/Agent/IntegrationTests/ContainerApplications/SmokeTestApp/SmokeTestApp.csproj +++ b/tests/Agent/IntegrationTests/ContainerApplications/SmokeTestApp/SmokeTestApp.csproj @@ -9,6 +9,7 @@ + diff --git a/tests/Agent/IntegrationTests/ContainerApplications/docker-compose.yml b/tests/Agent/IntegrationTests/ContainerApplications/docker-compose.yml index c1045ddec8..d796a53053 100644 --- a/tests/Agent/IntegrationTests/ContainerApplications/docker-compose.yml +++ b/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: diff --git a/tests/Agent/IntegrationTests/ContainerIntegrationTests/ContainerFixtures/LinuxUnicodeLogFileTestFixture.cs b/tests/Agent/IntegrationTests/ContainerIntegrationTests/ContainerFixtures/LinuxUnicodeLogFileTestFixture.cs new file mode 100644 index 0000000000..3b4383a3dc --- /dev/null +++ b/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) { } + } +} diff --git a/tests/Agent/IntegrationTests/ContainerIntegrationTests/LinuxUnicodeLogFileTest.cs b/tests/Agent/IntegrationTests/ContainerIntegrationTests/LinuxUnicodeLogFileTest.cs new file mode 100644 index 0000000000..7d10f8ead4 --- /dev/null +++ b/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 +{ + /// + /// 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. + /// + public class LinuxUnicodeLogFileTest : NewRelicIntegrationTest + { + 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")); + } + } +}