Skip to content

Commit

Permalink
fix: Fix a crash that can occur when the profiler logs certain charac…
Browse files Browse the repository at this point in the history
…ters. (#1982) (#2109)
  • Loading branch information
nrcventura committed Dec 1, 2023
1 parent 2d8da68 commit 742d232
Show file tree
Hide file tree
Showing 7 changed files with 102 additions and 1 deletion.
2 changes: 1 addition & 1 deletion src/Agent/NewRelic/Home/Home.csproj
Expand Up @@ -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>
Expand Up @@ -23,6 +23,7 @@
#include <string>
#include <thread>
#include <utility>
#include <codecvt>

#ifdef PAL_STDCPP_COMPAT
#include "UnixSystemCalls.h"
Expand Down Expand Up @@ -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.");
}
Expand Down
Expand Up @@ -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;

Expand All @@ -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),
Expand All @@ -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);
}
}
Expand Up @@ -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>
Expand Up @@ -67,6 +67,10 @@ services:
service: smoketestapp
build:
dockerfile: SmokeTestApp/Dockerfile.amazon
LinuxUnicodeLogFileApp:
extends:
file: docker-compose-smoketestapp.yml
service: smoketestapp

networks:
default:
Expand Down
@@ -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) { }
}
}
@@ -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"));
}
}
}

0 comments on commit 742d232

Please sign in to comment.