Disable log forwarding if no samples should be stored (#1097)

* Disable log forwarding at the configuration layer if the max samples stored is ever 0. * changelog * Fix artifactbuilder * Potential fix for deprecated instrumentation being loaded * Refactor deprecated instrumentation xml impl, add unit test * Update checked in profiler * Code Review Feedback * Remove extra whitespace
newrelic · May 19, 2022 · 2260d87 · 2260d87
1 parent add709a
commit 2260d87
Show file tree

Hide file tree

Showing 11 changed files with 71 additions and 10 deletions.
diff --git a/build/ArtifactBuilder/CoreAgentComponents.cs b/build/ArtifactBuilder/CoreAgentComponents.cs
@@ -33,7 +33,8 @@ protected override void CreateAgentComponents()
                 $@"{SourceHomeBuilderPath}\extensions\NewRelic.Providers.Wrapper.AspNetCore.dll",
                 $@"{SourceHomeBuilderPath}\extensions\NewRelic.Providers.Wrapper.CosmosDb.dll",
                 $@"{SourceHomeBuilderPath}\extensions\NewRelic.Providers.Wrapper.HttpClient.dll",
-                $@"{SourceHomeBuilderPath}\extensions\NewRelic.Providers.Wrapper.Logging.dll",
+                $@"{SourceHomeBuilderPath}\extensions\NewRelic.Providers.Wrapper.Log4NetLogging.dll",
+                $@"{SourceHomeBuilderPath}\extensions\NewRelic.Providers.Wrapper.SerilogLogging.dll",
                 $@"{SourceHomeBuilderPath}\extensions\NewRelic.Providers.Wrapper.MicrosoftExtensionsLogging.dll",
                 $@"{SourceHomeBuilderPath}\extensions\NewRelic.Providers.Wrapper.MongoDb26.dll",
                 $@"{SourceHomeBuilderPath}\extensions\NewRelic.Providers.Wrapper.RabbitMq.dll",
@@ -47,7 +48,8 @@ protected override void CreateAgentComponents()
                 $@"{SourceHomeBuilderPath}\extensions\NewRelic.Providers.Wrapper.AspNetCore.Instrumentation.xml",
                 $@"{SourceHomeBuilderPath}\extensions\NewRelic.Providers.Wrapper.CosmosDb.Instrumentation.xml",
                 $@"{SourceHomeBuilderPath}\extensions\NewRelic.Providers.Wrapper.HttpClient.Instrumentation.xml",
-                $@"{SourceHomeBuilderPath}\extensions\NewRelic.Providers.Wrapper.Logging.Instrumentation.xml",
+                $@"{SourceHomeBuilderPath}\extensions\NewRelic.Providers.Wrapper.Log4NetLogging.Instrumentation.xml",
+                $@"{SourceHomeBuilderPath}\extensions\NewRelic.Providers.Wrapper.SerilogLogging.Instrumentation.xml",
                 $@"{SourceHomeBuilderPath}\extensions\NewRelic.Providers.Wrapper.MicrosoftExtensionsLogging.Instrumentation.xml",
                 $@"{SourceHomeBuilderPath}\extensions\NewRelic.Providers.Wrapper.Misc.Instrumentation.xml",
                 $@"{SourceHomeBuilderPath}\extensions\NewRelic.Providers.Wrapper.MongoDb26.Instrumentation.xml",

diff --git a/build/ArtifactBuilder/FrameworkAgentComponents.cs b/build/ArtifactBuilder/FrameworkAgentComponents.cs
@@ -40,7 +40,8 @@ protected override void CreateAgentComponents()
                 $@"{SourceHomeBuilderPath}\extensions\NewRelic.Providers.Wrapper.Couchbase.dll",
                 $@"{SourceHomeBuilderPath}\extensions\NewRelic.Providers.Wrapper.HttpClient.dll",
                 $@"{SourceHomeBuilderPath}\extensions\NewRelic.Providers.Wrapper.HttpWebRequest.dll",
-                $@"{SourceHomeBuilderPath}\extensions\NewRelic.Providers.Wrapper.Logging.dll",
+                $@"{SourceHomeBuilderPath}\extensions\NewRelic.Providers.Wrapper.Log4NetLogging.dll",
+                $@"{SourceHomeBuilderPath}\extensions\NewRelic.Providers.Wrapper.SerilogLogging.dll",
                 $@"{SourceHomeBuilderPath}\extensions\NewRelic.Providers.Wrapper.MongoDb.dll",
                 $@"{SourceHomeBuilderPath}\extensions\NewRelic.Providers.Wrapper.MongoDb26.dll",
                 $@"{SourceHomeBuilderPath}\extensions\NewRelic.Providers.Wrapper.Msmq.dll",
@@ -71,7 +72,8 @@ protected override void CreateAgentComponents()
                 $@"{SourceHomeBuilderPath}\extensions\NewRelic.Providers.Wrapper.Couchbase.Instrumentation.xml",
                 $@"{SourceHomeBuilderPath}\extensions\NewRelic.Providers.Wrapper.HttpClient.Instrumentation.xml",
                 $@"{SourceHomeBuilderPath}\extensions\NewRelic.Providers.Wrapper.HttpWebRequest.Instrumentation.xml",
-                $@"{SourceHomeBuilderPath}\extensions\NewRelic.Providers.Wrapper.Logging.Instrumentation.xml",
+                $@"{SourceHomeBuilderPath}\extensions\NewRelic.Providers.Wrapper.Log4NetLogging.Instrumentation.xml",
+                $@"{SourceHomeBuilderPath}\extensions\NewRelic.Providers.Wrapper.SerilogLogging.Instrumentation.xml",
                 $@"{SourceHomeBuilderPath}\extensions\NewRelic.Providers.Wrapper.MongoDb.Instrumentation.xml",
                 $@"{SourceHomeBuilderPath}\extensions\NewRelic.Providers.Wrapper.MongoDb26.Instrumentation.xml",
                 $@"{SourceHomeBuilderPath}\extensions\NewRelic.Providers.Wrapper.Msmq.Instrumentation.xml",

diff --git a/src/Agent/CHANGELOG.md b/src/Agent/CHANGELOG.md
@@ -7,8 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ## [Unreleased] changes
 
 ### New Features
+* Adds an internal list of deprecated instrumentation xml files which will cause the profiler to ignore deprecated instrumentation. This feature avoids an issue where orphaned deprecated log forwarding instrumentation could conflict with newer instrumentation. ([#1051](https://github.com/newrelic/newrelic-dotnet-agent/pull/1097))
 
 ### Fixes
+* Fixes an [issue with log forwarding](https://github.com/newrelic/newrelic-dotnet-agent/issues/1088) where an agent could momentarily forward logs even if the feature had been disabled at an account level. ([#1051](https://github.com/newrelic/newrelic-dotnet-agent/pull/1097))
 
 ## [9.8.0] - 2022-05-05
 

diff --git a/src/Agent/NewRelic/Agent/Core/Configuration/DefaultConfiguration.cs b/src/Agent/NewRelic/Agent/Core/Configuration/DefaultConfiguration.cs
@@ -1856,6 +1856,7 @@ public virtual bool LogEventCollectorEnabled
             get
             {
                 return ApplicationLoggingEnabled &&
+                    LogEventsMaxSamplesStored > 0 &&
                     !SecurityPoliciesTokenExists &&
                     HighSecurityModeOverrides(false,
                     EnvironmentOverrides(_localConfiguration.applicationLogging.forwarding.enabled, "NEW_RELIC_APPLICATION_LOGGING_FORWARDING_ENABLED"));

diff --git a/src/Agent/NewRelic/Profiler/Configuration/InstrumentationConfiguration.h b/src/Agent/NewRelic/Profiler/Configuration/InstrumentationConfiguration.h
@@ -25,14 +25,20 @@ namespace NewRelic { namespace Profiler { namespace Configuration
         InstrumentationConfiguration(InstrumentationXmlSetPtr instrumentationXmls) :
             _instrumentationPointsSet(new InstrumentationPointSet())
         {
-
             // pull instrumentation points from every xml string
             for (auto instrumentationXml : *instrumentationXmls)
             {
                 try
                 {
-                    LogDebug(L"Parsing instrumentation file '", instrumentationXml.first);
-                    GetInstrumentationPoints(instrumentationXml.second);
+                    if (InstrumentationXmlIsDeprecated(instrumentationXml.first))
+                    {
+                        LogWarn("Deprecated instrumentation file being ignored: ", instrumentationXml.first);
+                    }
+                    else
+                    {
+                        LogDebug(L"Parsing instrumentation file '", instrumentationXml.first);
+                        GetInstrumentationPoints(instrumentationXml.second);
+                    }                    
                 }
                 catch (...)
                 {
@@ -42,9 +48,7 @@ namespace NewRelic { namespace Profiler { namespace Configuration
                     continue;
                 }
             }
-
             LogInfo("Identified ", _instrumentationPointsSet->size(), " Instrumentation points in .xml files");
-
         }
 
         InstrumentationConfiguration(InstrumentationPointSetPtr instrumentationPoints) :
@@ -86,6 +90,16 @@ namespace NewRelic { namespace Profiler { namespace Configuration
 
     private:
 
+        static bool InstrumentationXmlIsDeprecated(xstring_t instrumentationXmlFilePath)
+        {
+            bool returnValue = false;
+            if (NewRelic::Profiler::Strings::ContainsCaseInsensitive(instrumentationXmlFilePath, _X("NewRelic.Providers.Wrapper.Logging.Instrumentation.xml")))
+            {
+                returnValue = true;
+            }
+            return returnValue;
+        }
+
         InstrumentationPointPtr TryGetInstrumentationPoint(
             const xstring_t& assemblyName,
             const xstring_t& className,

diff --git a/src/Agent/NewRelic/Profiler/ConfigurationTest/InstrumentationConfigurationTest.cpp b/src/Agent/NewRelic/Profiler/ConfigurationTest/InstrumentationConfigurationTest.cpp
@@ -73,6 +73,26 @@ namespace NewRelic { namespace Profiler { namespace Configuration { namespace Te
             Assert::IsFalse(instrumentationPoint == nullptr);
         }
 
+        TEST_METHOD(deprecated_logforwarding_xml_is_ignored)
+        {
+            InstrumentationXmlSetPtr xmlSet(new InstrumentationXmlSet());
+            xmlSet->emplace(L"NewRelic.Providers.Wrapper.Logging.Instrumentation.xml", L"\
+                <?xml version=\"1.0\" encoding=\"utf-8\"?>\
+                <extension>\
+                    <instrumentation>\
+                        <tracerFactory>\
+                            <match assemblyName=\"MyAssembly\" className=\"MyNamespace.MyClass\">\
+                                <exactMethodMatcher methodName=\"MyMethod\"/>\
+                            </match>\
+                        </tracerFactory>\
+                    </instrumentation>\
+                </extension>\
+                ");
+            InstrumentationConfiguration instrumentation(xmlSet);
+            auto instrumentationPoint = instrumentation.TryGetInstrumentationPoint(std::make_shared<MethodRewriter::Test::MockFunction>());
+            Assert::IsTrue(instrumentationPoint == nullptr);
+        }
+
         TEST_METHOD(no_assembly_match)
         {
             InstrumentationXmlSetPtr xmlSet(new InstrumentationXmlSet());

diff --git a/src/Agent/_profilerBuild/linux-arm64-release/libNewRelicProfiler.so b/src/Agent/_profilerBuild/linux-arm64-release/libNewRelicProfiler.so
diff --git a/src/Agent/_profilerBuild/linux-x64-release/libNewRelicProfiler.so b/src/Agent/_profilerBuild/linux-x64-release/libNewRelicProfiler.so
diff --git a/src/Agent/_profilerBuild/x64-Release/NewRelic.Profiler.dll b/src/Agent/_profilerBuild/x64-Release/NewRelic.Profiler.dll
diff --git a/src/Agent/_profilerBuild/x86-Release/NewRelic.Profiler.dll b/src/Agent/_profilerBuild/x86-Release/NewRelic.Profiler.dll
diff --git a/tests/Agent/UnitTests/Core.UnitTest/Configuration/DefaultConfigurationTests.cs b/tests/Agent/UnitTests/Core.UnitTest/Configuration/DefaultConfigurationTests.cs
@@ -2658,7 +2658,27 @@ public void ApplicationLogging_ForwardingEnabled_IsOverriddenByHighSecurityMode(
         }
 
         [Test]
-        public void ApplicationLogging_LocalDecordingEnabled_IsFalseInLocalConfigByDefault()
+        public void ApplicationLogging_ForwardingEnabled_IsOverriddenWhenNoSpanEventsAllowed_ByLocalConfig()
+        {
+            _localConfig.applicationLogging.forwarding.maxSamplesStored = 0;
+
+            Assert.IsFalse(_defaultConfig.LogEventCollectorEnabled);
+        }
+
+        [Test]
+        public void ApplicationLogging_ForwardingEnabled_IsOverriddenWhenNoSpanEventsAllowed_ByServer()
+        {
+            _serverConfig.EventHarvestConfig = new EventHarvestConfig
+            {
+                ReportPeriodMs = 5000,
+                HarvestLimits = new Dictionary<string, int> { { EventHarvestConfig.LogEventHarvestLimitKey, 0 } }
+            };
+
+            Assert.IsFalse(_defaultConfig.LogEventCollectorEnabled);
+        }
+
+        [Test]
+        public void ApplicationLogging_LocalDecoratingEnabled_IsFalseInLocalConfigByDefault()
         {
             Assert.IsFalse(_defaultConfig.LogDecoratorEnabled);
         }