From d7252780626540680defbbdf1387129a2eb1e417 Mon Sep 17 00:00:00 2001 From: Marty T <120425148+tippmar-nr@users.noreply.github.com> Date: Tue, 20 Aug 2024 14:43:45 -0500 Subject: [PATCH] chore: Update Serilog to latest (#2694) * Updated Serilog to latest everywhere * Fix ilrepack issues for serilog * Refactor GuidGenerator to work around ILRepack of System.Diagnostics.DiagnosticSource * Null checking * Visibility bypasser caching --- src/Agent/NewRelic/Agent/Core/Core.csproj | 19 ++-- .../Agent/Core/Utilities/GuidGenerator.cs | 90 +++++++++++++++---- .../NewRelic.Agent.TestUtilities.csproj | 6 +- 3 files changed, 88 insertions(+), 27 deletions(-) diff --git a/src/Agent/NewRelic/Agent/Core/Core.csproj b/src/Agent/NewRelic/Agent/Core/Core.csproj index 9a792198f..9c95c4286 100644 --- a/src/Agent/NewRelic/Agent/Core/Core.csproj +++ b/src/Agent/NewRelic/Agent/Core/Core.csproj @@ -43,12 +43,12 @@ all runtime; build; native; contentfiles; analyzers; buildtransitive - - - - + + + + - + all @@ -56,6 +56,7 @@ + @@ -79,7 +80,6 @@ - @@ -110,6 +110,9 @@ + + + @@ -132,8 +135,8 @@ - 18 - 15 + 20 + 17 diff --git a/src/Agent/NewRelic/Agent/Core/Utilities/GuidGenerator.cs b/src/Agent/NewRelic/Agent/Core/Utilities/GuidGenerator.cs index a3c3f5c12..617db6611 100644 --- a/src/Agent/NewRelic/Agent/Core/Utilities/GuidGenerator.cs +++ b/src/Agent/NewRelic/Agent/Core/Utilities/GuidGenerator.cs @@ -2,14 +2,14 @@ // SPDX-License-Identifier: Apache-2.0 using NewRelic.Agent.Extensions.Logging; +using NewRelic.Reflection; using System; -using System.Diagnostics; -using System.IO; using System.Security.Cryptography; using System.Threading; namespace NewRelic.Agent.Core.Utilities { + [NrExcludeFromCodeCoverage] public static class GuidGenerator { /// Our testing shows that RngCryptoServiceProvider library is threadsafe and is more performant @@ -17,6 +17,15 @@ public static class GuidGenerator private static readonly RNGCryptoServiceProvider RngCryptoServiceProvider = new RNGCryptoServiceProvider(); private static Func _traceGeneratorFunc = GetTraceIdFromCurrentActivity; + private static bool _initialized; + private static object _lockObj = new(); + private static bool _hasDiagnosticSourceReference; + + private static Func _fieldReadAccessor; + private static Func _valuePropertyAccessor; + private static Func _traceIdGetter; + private static Func _idFormatGetter; + /// /// Returns a newrelic style guid. /// https://source.datanerd.us/agents/agent-specs/blob/2ad6637ded7ec3784de40fbc88990e06525127b8/Cross-Application-Tracing-PORTED.md#guid @@ -33,20 +42,26 @@ public static string GenerateNewRelicTraceId() { try { - return _traceGeneratorFunc(); - } - catch (Exception ex) - { - // If the app does not reference System.Diagnostics.DiagnosticSource then Activity.Current will not be available. - // A FileNotFoundException occurs when System.Diagnostics.DiagnosticSource is unavailble. - - if (!(ex is FileNotFoundException)) + var retVal = _traceGeneratorFunc(); + if (retVal == null) { - Log.Warn(ex, "Unexpected exception type when attempting to generate a trace ID from Activity.Current"); + if (!_hasDiagnosticSourceReference) + { + // Fall back to using our standard method of generating traceIds if the application doesn't reference DiagnosticSource + Log.Info("No reference to DiagnosticSource; trace IDs will be generated using the standard generator"); + Interlocked.Exchange(ref _traceGeneratorFunc, GenerateTraceId); + return _traceGeneratorFunc(); + } + + // couldn't get a traceId from the current activity (maybe there wasn't one), so fallback to the standard generator for this request only + return GenerateTraceId(); } - // Fall back to using our standard method of generating traceIds. - Log.Info($"Trace IDs will be generated using the standard generator"); + return retVal; + } + catch (Exception e) + { + Log.Info(e, "Unexpected exception generating traceId using the current activity. Falling back to the standard generator"); Interlocked.Exchange(ref _traceGeneratorFunc, GenerateTraceId); return _traceGeneratorFunc(); } @@ -63,12 +78,55 @@ private static string GenerateTraceId() private static string GetTraceIdFromCurrentActivity() { - if (Activity.Current != default && Activity.Current.IdFormat == ActivityIdFormat.W3C) + // because we ILRepack System.Diagnostics.DiagnosticSource, we have to look for the app's reference to it (if there is one) + // and use reflection to get the trace id from the current activity + + // initialize one time + if (!_initialized) { - return Activity.Current.TraceId.ToString(); + lock (_lockObj) + { + if (!_initialized) + { + var assemblies = AppDomain.CurrentDomain.GetAssemblies(); + // find System.Diagnostics.DiagnosticSource + var diagnosticSourceAssembly = Array.Find(assemblies, a => a.FullName.StartsWith("System.Diagnostics.DiagnosticSource")); + if (diagnosticSourceAssembly != null) // customer app might not reference the assembly + { + _hasDiagnosticSourceReference = true; + + // find the Activity class + var activityType = diagnosticSourceAssembly.GetType("System.Diagnostics.Activity"); + _fieldReadAccessor = VisibilityBypasser.Instance.GenerateFieldReadAccessor(activityType, "s_current"); + } + + _initialized = true; + } + } } - return GenerateTraceId(); + if (!_hasDiagnosticSourceReference) + return null; + + var current = _fieldReadAccessor(null); // s_current is a static, so we don't need an object instance + if (current == null) + return null; + + // get the Value property + _valuePropertyAccessor ??= VisibilityBypasser.Instance.GeneratePropertyAccessor(current.GetType(), "Value"); + var value = _valuePropertyAccessor(current); + if (value == null) + return null; + + // get IdFormat property + _idFormatGetter ??= VisibilityBypasser.Instance.GeneratePropertyAccessor(value.GetType(), "IdFormat"); + var idFormat = _idFormatGetter(value); + if (idFormat == null || Enum.GetName(idFormat.GetType(), idFormat) != "W3C") // make sure it's in W3C trace id format + return null; + + // get TraceId property + _traceIdGetter ??= VisibilityBypasser.Instance.GeneratePropertyAccessor(value.GetType(), "TraceId"); + return _traceIdGetter(value).ToString(); } } } diff --git a/tests/Agent/UnitTests/NewRelic.Agent.TestUtilities/NewRelic.Agent.TestUtilities.csproj b/tests/Agent/UnitTests/NewRelic.Agent.TestUtilities/NewRelic.Agent.TestUtilities.csproj index 8243450d0..551f43090 100644 --- a/tests/Agent/UnitTests/NewRelic.Agent.TestUtilities/NewRelic.Agent.TestUtilities.csproj +++ b/tests/Agent/UnitTests/NewRelic.Agent.TestUtilities/NewRelic.Agent.TestUtilities.csproj @@ -14,9 +14,9 @@ all runtime; build; native; contentfiles; analyzers; buildtransitive - - - + + +