Skip to content

Commit d74261d

Browse files
mathewcmathewc
committed
Removing SettingsManager caching
1 parent b4313d4 commit d74261d

File tree

4 files changed

+111
-117
lines changed

4 files changed

+111
-117
lines changed

‎src/WebJobs.Script.WebHost/App_Start/WebHostResolver.cs‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,6 @@ internal void EnsureInitialized(WebHostSettings settings)
139139
if (_activeHostManager == null &&
140140
(_standbyHostManager == null || _settingsManager.ContainerReady))
141141
{
142-
_settingsManager.Reset();
143142
_specializationTimer?.Dispose();
144143
_specializationTimer = null;
145144

‎src/WebJobs.Script/Config/ScriptSettingsManager.cs‎

Lines changed: 13 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,12 @@
22
// Licensed under the MIT License. See License.txt in the project root for license information.
33

44
using System;
5-
using System.Collections.Concurrent;
65

76
namespace Microsoft.Azure.WebJobs.Script.Config
87
{
98
public class ScriptSettingsManager
109
{
1110
private static ScriptSettingsManager _instance = new ScriptSettingsManager();
12-
private readonly ConcurrentDictionary<string, string> _settingsCache = new ConcurrentDictionary<string, string>();
1311

1412
// for testing
1513
public ScriptSettingsManager()
@@ -40,19 +38,16 @@ public virtual string AzureWebsiteDefaultSubdomain
4038
{
4139
get
4240
{
43-
return _settingsCache.GetOrAdd(nameof(AzureWebsiteDefaultSubdomain), k =>
44-
{
45-
string siteHostName = GetSetting(EnvironmentSettingNames.AzureWebsiteHostName);
41+
string siteHostName = GetSetting(EnvironmentSettingNames.AzureWebsiteHostName);
4642

47-
int? periodIndex = siteHostName?.IndexOf('.');
43+
int? periodIndex = siteHostName?.IndexOf('.');
4844

49-
if (periodIndex != null && periodIndex > 0)
50-
{
51-
return siteHostName.Substring(0, periodIndex.Value);
52-
}
45+
if (periodIndex != null && periodIndex > 0)
46+
{
47+
return siteHostName.Substring(0, periodIndex.Value);
48+
}
5349

54-
return null;
55-
});
50+
return null;
5651
}
5752
}
5853

@@ -89,52 +84,28 @@ public virtual string InstanceId
8984

9085
public virtual string ApplicationInsightsInstrumentationKey
9186
{
92-
get => GetSettingFromCache(EnvironmentSettingNames.AppInsightsInstrumentationKey);
93-
set => UpdateSettingInCache(EnvironmentSettingNames.AppInsightsInstrumentationKey, value);
87+
get => Utility.GetSettingFromConfigOrEnvironment(EnvironmentSettingNames.AppInsightsInstrumentationKey);
88+
set => SetSetting(EnvironmentSettingNames.AppInsightsInstrumentationKey, value);
9489
}
9590

96-
private string GetSettingFromCache(string settingKey)
91+
public virtual string GetSetting(string settingKey)
9792
{
9893
if (string.IsNullOrEmpty(settingKey))
9994
{
10095
throw new ArgumentNullException(nameof(settingKey));
10196
}
10297

103-
return _settingsCache.GetOrAdd(settingKey, (key) => Utility.GetSettingFromConfigOrEnvironment(key));
98+
return Environment.GetEnvironmentVariable(settingKey);
10499
}
105100

106-
private void UpdateSettingInCache(string settingKey, string settingValue)
101+
public virtual void SetSetting(string settingKey, string settingValue)
107102
{
108103
if (string.IsNullOrEmpty(settingKey))
109104
{
110105
throw new ArgumentNullException(nameof(settingKey));
111106
}
112107

113-
_settingsCache.AddOrUpdate(settingKey, settingValue, (a, b) => settingValue);
114-
}
115-
116-
public virtual void Reset()
117-
{
118-
_settingsCache.Clear();
119-
}
120-
121-
public virtual string GetSetting(string settingKey)
122-
{
123-
string settingValue = null;
124-
if (!string.IsNullOrEmpty(settingKey))
125-
{
126-
settingValue = Environment.GetEnvironmentVariable(settingKey);
127-
}
128-
129-
return settingValue;
130-
}
131-
132-
public virtual void SetSetting(string settingKey, string settingValue)
133-
{
134-
if (!string.IsNullOrEmpty(settingKey))
135-
{
136-
Environment.SetEnvironmentVariable(settingKey, settingValue);
137-
}
108+
Environment.SetEnvironmentVariable(settingKey, settingValue);
138109
}
139110
}
140111
}

‎test/WebJobs.Script.Tests.Integration/ApplicationInsights/ApplicationInsightsTestFixture.cs‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
using System;
55
using System.Collections.Concurrent;
66
using System.Collections.Generic;
7+
using System.Configuration;
78
using System.IO;
89
using System.Net.Http;
910
using System.Web.Http;
@@ -41,7 +42,9 @@ public ApplicationInsightsTestFixture(string scriptRoot, string testId)
4142
var resolver = _config.DependencyResolver;
4243
var hostConfig = resolver.GetService<WebHostResolver>().GetScriptHostConfiguration(HostSettings);
4344

44-
_settingsManager.ApplicationInsightsInstrumentationKey = TestChannelLoggerFactoryBuilder.ApplicationInsightsKey;
45+
// configure via AppSettings, because the test project has configured an
46+
// empty value in app.config which we need to override
47+
ConfigurationManager.AppSettings[EnvironmentSettingNames.AppInsightsInstrumentationKey] = TestChannelLoggerFactoryBuilder.ApplicationInsightsKey;
4548

4649
InitializeConfig(hostConfig);
4750

@@ -74,6 +77,8 @@ protected void InitializeConfig(ScriptHostConfiguration config)
7477

7578
public void Dispose()
7679
{
80+
ConfigurationManager.AppSettings[EnvironmentSettingNames.AppInsightsInstrumentationKey] = string.Empty;
81+
7782
_httpServer?.Dispose();
7883
HttpClient?.Dispose();
7984
}

‎test/WebJobs.Script.Tests/ScriptHostTests.cs‎

Lines changed: 92 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -1266,109 +1266,128 @@ public void ConfigureLoggerFactory_Default()
12661266
[Fact]
12671267
public void ConfigureLoggerFactory_ApplicationInsights()
12681268
{
1269-
var config = new ScriptHostConfiguration();
1270-
var mockTraceFactory = new Mock<IFunctionTraceWriterFactory>(MockBehavior.Strict);
1271-
var loggerFactory = new TestLoggerFactory();
1272-
config.HostConfig.LoggerFactory = loggerFactory;
1269+
try
1270+
{
1271+
var config = new ScriptHostConfiguration();
1272+
var mockTraceFactory = new Mock<IFunctionTraceWriterFactory>(MockBehavior.Strict);
1273+
var loggerFactory = new TestLoggerFactory();
1274+
config.HostConfig.LoggerFactory = loggerFactory;
12731275

1274-
// Make sure no App Insights is configured
1275-
var settingsManager = ScriptSettingsManager.Instance;
1276-
settingsManager.ApplicationInsightsInstrumentationKey = "Some_Instrumentation_Key";
1276+
// configure via AppSettings, because the test project has configured an
1277+
// empty value in app.config which we need to override
1278+
var settingsManager = ScriptSettingsManager.Instance;
1279+
ConfigurationManager.AppSettings[EnvironmentSettingNames.AppInsightsInstrumentationKey] = "Some_Instrumentation_Key";
12771280

1278-
var metricsLogger = new TestMetricsLogger();
1279-
config.HostConfig.AddService<IMetricsLogger>(metricsLogger);
1281+
var metricsLogger = new TestMetricsLogger();
1282+
config.HostConfig.AddService<IMetricsLogger>(metricsLogger);
12801283

1281-
ScriptHost.ConfigureLoggerFactory(config, mockTraceFactory.Object, settingsManager, () => true);
1284+
ScriptHost.ConfigureLoggerFactory(config, mockTraceFactory.Object, settingsManager, () => true);
12821285

1283-
Assert.Equal(2, loggerFactory.Providers.Count);
1286+
Assert.Equal(2, loggerFactory.Providers.Count);
12841287

1285-
Assert.Equal(1, loggerFactory.Providers.OfType<FileLoggerProvider>().Count());
1288+
Assert.Equal(1, loggerFactory.Providers.OfType<FileLoggerProvider>().Count());
12861289

1287-
// The app insights logger is internal, so just check the name
1288-
ILoggerProvider appInsightsProvider = loggerFactory.Providers.Last();
1289-
Assert.Equal("ApplicationInsightsLoggerProvider", appInsightsProvider.GetType().Name);
1290+
// The app insights logger is internal, so just check the name
1291+
ILoggerProvider appInsightsProvider = loggerFactory.Providers.Last();
1292+
Assert.Equal("ApplicationInsightsLoggerProvider", appInsightsProvider.GetType().Name);
12901293

1291-
Assert.Equal(1, metricsLogger.LoggedEvents.Count);
1292-
Assert.Equal(MetricEventNames.ApplicationInsightsEnabled, metricsLogger.LoggedEvents[0]);
1294+
Assert.Equal(1, metricsLogger.LoggedEvents.Count);
1295+
Assert.Equal(MetricEventNames.ApplicationInsightsEnabled, metricsLogger.LoggedEvents[0]);
1296+
}
1297+
finally
1298+
{
1299+
// restore the empty value configured for tests in app.config
1300+
ConfigurationManager.AppSettings[EnvironmentSettingNames.AppInsightsInstrumentationKey] = string.Empty;
1301+
}
12931302
}
12941303

12951304
[Fact]
12961305
public void DefaultLoggerFactory_BeginScope()
12971306
{
1298-
var trace = new TestTraceWriter(TraceLevel.Info);
1299-
var config = new ScriptHostConfiguration();
1300-
var mockTraceFactory = new Mock<IFunctionTraceWriterFactory>(MockBehavior.Strict);
1301-
mockTraceFactory
1302-
.Setup(f => f.Create("Test", null))
1303-
.Returns(trace);
1304-
1305-
var channel = new TestTelemetryChannel();
1306-
var builder = new TestChannelLoggerFactoryBuilder(channel);
1307+
try
1308+
{
1309+
var trace = new TestTraceWriter(TraceLevel.Info);
1310+
var config = new ScriptHostConfiguration();
1311+
var mockTraceFactory = new Mock<IFunctionTraceWriterFactory>(MockBehavior.Strict);
1312+
mockTraceFactory
1313+
.Setup(f => f.Create("Test", null))
1314+
.Returns(trace);
13071315

1308-
config.LoggerFactoryBuilder = builder;
1309-
config.HostConfig.LoggerFactory = new LoggerFactory();
1316+
var channel = new TestTelemetryChannel();
1317+
var builder = new TestChannelLoggerFactoryBuilder(channel);
13101318

1311-
var settingsManager = ScriptSettingsManager.Instance;
1312-
settingsManager.ApplicationInsightsInstrumentationKey = TestChannelLoggerFactoryBuilder.ApplicationInsightsKey;
1319+
config.LoggerFactoryBuilder = builder;
1320+
config.HostConfig.LoggerFactory = new LoggerFactory();
13131321

1314-
ScriptHost.ConfigureLoggerFactory(config, mockTraceFactory.Object, settingsManager, () => true);
1322+
// configure via AppSettings, because the test project has configured an
1323+
// empty value in app.config which we need to override
1324+
var settingsManager = ScriptSettingsManager.Instance;
1325+
ConfigurationManager.AppSettings[EnvironmentSettingNames.AppInsightsInstrumentationKey] = TestChannelLoggerFactoryBuilder.ApplicationInsightsKey;
13151326

1316-
// Create a logger and try out the configured factory. We need to pretend that it is coming from a
1317-
// function, so set the function name and the category appropriately.
1318-
var logger = config.HostConfig.LoggerFactory.CreateLogger(LogCategories.Function);
1327+
ScriptHost.ConfigureLoggerFactory(config, mockTraceFactory.Object, settingsManager, () => true);
13191328

1320-
using (logger.BeginScope(new Dictionary<string, object>
1321-
{
1322-
[ScriptConstants.LoggerFunctionNameKey] = "Test"
1323-
}))
1324-
{
1325-
// Now log as if from within a function.
1329+
// Create a logger and try out the configured factory. We need to pretend that it is coming from a
1330+
// function, so set the function name and the category appropriately.
1331+
var logger = config.HostConfig.LoggerFactory.CreateLogger(LogCategories.Function);
13261332

1327-
// Test that both dictionaries and structured logs work as state
1328-
// and that nesting works as expected.
1329-
using (logger.BeginScope("{customKey1}", "customValue1"))
1333+
using (logger.BeginScope(new Dictionary<string, object>
13301334
{
1331-
logger.LogInformation("1");
1335+
[ScriptConstants.LoggerFunctionNameKey] = "Test"
1336+
}))
1337+
{
1338+
// Now log as if from within a function.
13321339

1333-
using (logger.BeginScope(new Dictionary<string, object>
1334-
{
1335-
["customKey2"] = "customValue2"
1336-
}))
1340+
// Test that both dictionaries and structured logs work as state
1341+
// and that nesting works as expected.
1342+
using (logger.BeginScope("{customKey1}", "customValue1"))
13371343
{
1338-
logger.LogInformation("2");
1339-
}
1344+
logger.LogInformation("1");
1345+
1346+
using (logger.BeginScope(new Dictionary<string, object>
1347+
{
1348+
["customKey2"] = "customValue2"
1349+
}))
1350+
{
1351+
logger.LogInformation("2");
1352+
}
13401353

1341-
logger.LogInformation("3");
1342-
}
1354+
logger.LogInformation("3");
1355+
}
13431356

1344-
using (logger.BeginScope("should not throw"))
1345-
{
1346-
logger.LogInformation("4");
1357+
using (logger.BeginScope("should not throw"))
1358+
{
1359+
logger.LogInformation("4");
1360+
}
13471361
}
1348-
}
13491362

1350-
Assert.Equal(4, trace.GetTraces().Count);
1351-
Assert.Equal(4, channel.Telemetries.Count);
1363+
Assert.Equal(4, trace.GetTraces().Count);
1364+
Assert.Equal(4, channel.Telemetries.Count);
13521365

1353-
var traces = channel.Telemetries.Cast<TraceTelemetry>().OrderBy(t => t.Message).ToArray();
1366+
var traces = channel.Telemetries.Cast<TraceTelemetry>().OrderBy(t => t.Message).ToArray();
13541367

1355-
// Every telemetry will have {originalFormat}, Category, Level, but we validate those elsewhere.
1356-
// We're only interested in the custom properties.
1357-
Assert.Equal("1", traces[0].Message);
1358-
Assert.Equal(4, traces[0].Properties.Count);
1359-
Assert.Equal("customValue1", traces[0].Properties["prop__customKey1"]);
1368+
// Every telemetry will have {originalFormat}, Category, Level, but we validate those elsewhere.
1369+
// We're only interested in the custom properties.
1370+
Assert.Equal("1", traces[0].Message);
1371+
Assert.Equal(4, traces[0].Properties.Count);
1372+
Assert.Equal("customValue1", traces[0].Properties["prop__customKey1"]);
13601373

1361-
Assert.Equal("2", traces[1].Message);
1362-
Assert.Equal(5, traces[1].Properties.Count);
1363-
Assert.Equal("customValue1", traces[1].Properties["prop__customKey1"]);
1364-
Assert.Equal("customValue2", traces[1].Properties["prop__customKey2"]);
1374+
Assert.Equal("2", traces[1].Message);
1375+
Assert.Equal(5, traces[1].Properties.Count);
1376+
Assert.Equal("customValue1", traces[1].Properties["prop__customKey1"]);
1377+
Assert.Equal("customValue2", traces[1].Properties["prop__customKey2"]);
13651378

1366-
Assert.Equal("3", traces[2].Message);
1367-
Assert.Equal(4, traces[2].Properties.Count);
1368-
Assert.Equal("customValue1", traces[2].Properties["prop__customKey1"]);
1379+
Assert.Equal("3", traces[2].Message);
1380+
Assert.Equal(4, traces[2].Properties.Count);
1381+
Assert.Equal("customValue1", traces[2].Properties["prop__customKey1"]);
13691382

1370-
Assert.Equal("4", traces[3].Message);
1371-
Assert.Equal(3, traces[3].Properties.Count);
1383+
Assert.Equal("4", traces[3].Message);
1384+
Assert.Equal(3, traces[3].Properties.Count);
1385+
}
1386+
finally
1387+
{
1388+
// restore the empty value configured for tests in app.config
1389+
ConfigurationManager.AppSettings[EnvironmentSettingNames.AppInsightsInstrumentationKey] = string.Empty;
1390+
}
13721391
}
13731392

13741393
[Fact]

0 commit comments

Comments
 (0)