From 1624938ab48b63c1fa6e98037d74976dbc8186da Mon Sep 17 00:00:00 2001
From: Marty Tippin <120425148+tippmar-nr@users.noreply.github.com>
Date: Thu, 15 Jun 2023 11:05:31 -0500
Subject: [PATCH] fix: Cache the AgentEnabled setting value. (#1723)

---
 .../Configuration/DefaultConfiguration.cs     | 24 ++++++++++----
 .../DefaultConfigurationTests.cs              | 33 ++++++++++++++++++-
 2 files changed, 50 insertions(+), 7 deletions(-)

diff --git a/src/Agent/NewRelic/Agent/Core/Configuration/DefaultConfiguration.cs b/src/Agent/NewRelic/Agent/Core/Configuration/DefaultConfiguration.cs
index 7a0c044413..26ac375b1a 100644
--- a/src/Agent/NewRelic/Agent/Core/Configuration/DefaultConfiguration.cs
+++ b/src/Agent/NewRelic/Agent/Core/Configuration/DefaultConfiguration.cs
@@ -183,17 +183,28 @@ private int TryGetAppSettingAsIntWithDefault(string key, int defaultValue)
 
         public object AgentRunId { get { return _serverConfiguration.AgentRunId; } }
 
+        // protected to allow unit test wrapper to manipulate
+        protected static bool? _agentEnabledAppSettingParsed;
+        protected static bool _appSettingAgentEnabled;
+        private static readonly object _lockObj = new object();
+
+
         public virtual bool AgentEnabled
         {
             get
             {
-                var agentEnabledAsString = _configurationManagerStatic.GetAppSetting("NewRelic.AgentEnabled");
-
-                bool agentEnabled;
-                if (!bool.TryParse(agentEnabledAsString, out agentEnabled))
-                    return _localConfiguration.agentEnabled;
+                // read from app setting one time only and cache the result
+                if (!_agentEnabledAppSettingParsed.HasValue)
+                {
+                    lock (_lockObj)
+                    {
+                        _agentEnabledAppSettingParsed ??= bool.TryParse(_configurationManagerStatic.GetAppSetting("NewRelic.AgentEnabled"),
+                            out _appSettingAgentEnabled);
+                    }
+                }
 
-                return agentEnabled;
+                // read from local config if we couldn't parse from app settings
+                return _agentEnabledAppSettingParsed.Value ? _appSettingAgentEnabled : _localConfiguration.agentEnabled;
             }
         }
 
@@ -2681,6 +2692,7 @@ private void LogDisabledPropertyUse(string disabledPropertyName, string newPrope
             TryGetAppSettingAsIntWithDefault("SqlStatementCacheCapacity", DefaultSqlStatementCacheCapacity)).Value;
 
         private bool? _codeLevelMetricsEnabled;
+
         public bool CodeLevelMetricsEnabled
         {
             get
diff --git a/tests/Agent/UnitTests/Core.UnitTest/Configuration/DefaultConfigurationTests.cs b/tests/Agent/UnitTests/Core.UnitTest/Configuration/DefaultConfigurationTests.cs
index d897522add..fc01d09290 100644
--- a/tests/Agent/UnitTests/Core.UnitTest/Configuration/DefaultConfigurationTests.cs
+++ b/tests/Agent/UnitTests/Core.UnitTest/Configuration/DefaultConfigurationTests.cs
@@ -20,6 +20,12 @@ internal class TestableDefaultConfiguration : DefaultConfiguration
     {
         public TestableDefaultConfiguration(IEnvironment environment, configuration localConfig, ServerConfiguration serverConfig, RunTimeConfiguration runTimeConfiguration, SecurityPoliciesConfiguration securityPoliciesConfiguration, IProcessStatic processStatic, IHttpRuntimeStatic httpRuntimeStatic, IConfigurationManagerStatic configurationManagerStatic, IDnsStatic dnsStatic)
             : base(environment, localConfig, serverConfig, runTimeConfiguration, securityPoliciesConfiguration, processStatic, httpRuntimeStatic, configurationManagerStatic, dnsStatic) { }
+
+        public static void ResetStatics()
+        {
+            _agentEnabledAppSettingParsed = null;
+            _appSettingAgentEnabled = false;
+        }
     }
 
     [TestFixture, Category("Configuration")]
@@ -50,15 +56,40 @@ public void SetUp()
             _dnsStatic = Mock.Create<IDnsStatic>();
 
             _defaultConfig = new TestableDefaultConfiguration(_environment, _localConfig, _serverConfig, _runTimeConfig, _securityPoliciesConfiguration, _processStatic, _httpRuntimeStatic, _configurationManagerStatic, _dnsStatic);
+
+            TestableDefaultConfiguration.ResetStatics();
         }
 
         [Test]
         public void AgentEnabledShouldPassThroughToLocalConfig()
         {
             Assert.IsTrue(_defaultConfig.AgentEnabled);
+
+            _localConfig.agentEnabled = false;
+            Assert.IsFalse(_defaultConfig.AgentEnabled);
+
             _localConfig.agentEnabled = true;
             Assert.IsTrue(_defaultConfig.AgentEnabled);
-            _localConfig.agentEnabled = false;
+        }
+
+        [Test]
+        public void AgentEnabledShouldUseCachedAppSetting()
+        {
+            Mock.Arrange(() => _configurationManagerStatic.GetAppSetting("NewRelic.AgentEnabled")).Returns("false");
+
+            Assert.IsFalse(_defaultConfig.AgentEnabled);
+            Assert.IsFalse(_defaultConfig.AgentEnabled);
+
+            Mock.Assert(() => _configurationManagerStatic.GetAppSetting("NewRelic.AgentEnabled"), Occurs.Once());
+        }
+
+        [Test]
+        public void AgentEnabledShouldPreferAppSettingOverLocalConfig()
+        {
+            Mock.Arrange(() => _configurationManagerStatic.GetAppSetting("NewRelic.AgentEnabled")).Returns("false");
+
+            _localConfig.agentEnabled = true;
+
             Assert.IsFalse(_defaultConfig.AgentEnabled);
         }