Skip to content

Commit e8a0b4c

Browse files
author
Manikanta Nallagatla
committed
address comments
1 parent 4628bbe commit e8a0b4c

File tree

6 files changed

+48
-50
lines changed

6 files changed

+48
-50
lines changed

‎src/WebJobs.Script.WebHost/Diagnostics/AzureMonitorDiagnosticLogger.cs

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,15 +5,17 @@
55
using System.Collections.Generic;
66
using System.Diagnostics;
77
using System.IO;
8+
using System.Linq;
89
using Microsoft.Azure.WebJobs.Logging;
910
using Microsoft.Azure.WebJobs.Script.Configuration;
1011
using Microsoft.Extensions.Logging;
1112
using Microsoft.Extensions.Options;
1213
using Newtonsoft.Json;
14+
using static Microsoft.Azure.WebJobs.Script.Utility;
1315

1416
namespace Microsoft.Azure.WebJobs.Script.WebHost.Diagnostics
1517
{
16-
public class AzureMonitorDiagnosticLogger : ILogger
18+
public class AzureMonitorDiagnosticLogger : ILogger, IDisposable
1719
{
1820
internal const string AzureMonitorCategoryName = "FunctionAppLogs";
1921
internal const string AzureMonitorOperationName = "Microsoft.Web/sites/functions/log";
@@ -29,7 +31,9 @@ public class AzureMonitorDiagnosticLogger : ILogger
2931
private readonly IEventGenerator _eventGenerator;
3032
private readonly IEnvironment _environment;
3133
private readonly IExternalScopeProvider _scopeProvider;
34+
private readonly IDisposable _appServiceOptionsOnChangeListener;
3235
private AppServiceOptions _appServiceOptions;
36+
private static bool isAzureMonitorLogsEnabled;
3337

3438
public AzureMonitorDiagnosticLogger(string category, string hostInstanceId, IEventGenerator eventGenerator, IEnvironment environment, IExternalScopeProvider scopeProvider,
3539
HostNameProvider hostNameProvider, IOptionsMonitor<AppServiceOptions> appServiceOptionsMonitor)
@@ -42,8 +46,8 @@ public AzureMonitorDiagnosticLogger(string category, string hostInstanceId, IEve
4246
_hostNameProvider = hostNameProvider ?? throw new ArgumentNullException(nameof(hostNameProvider));
4347
_ = appServiceOptionsMonitor ?? throw new ArgumentNullException(nameof(appServiceOptionsMonitor));
4448

45-
appServiceOptionsMonitor.OnChange(newOptions => _appServiceOptions = newOptions);
46-
_appServiceOptions = appServiceOptionsMonitor.CurrentValue;
49+
_appServiceOptionsOnChangeListener = appServiceOptionsMonitor.OnChange(UpdateAppServiceOptions);
50+
UpdateAppServiceOptions(appServiceOptionsMonitor.CurrentValue);
4751

4852
_roleInstance = _environment.GetInstanceId();
4953

@@ -54,7 +58,7 @@ public AzureMonitorDiagnosticLogger(string category, string hostInstanceId, IEve
5458

5559
public bool IsEnabled(LogLevel logLevel)
5660
{
57-
if (_environment.IsLegionBasedSku() && !_environment.IsAzureMonitorEnabled(useCache:true))
61+
if (_environment.IsConsumptionOnLegion() && !isAzureMonitorLogsEnabled)
5862
{
5963
return false;
6064
}
@@ -140,6 +144,17 @@ public void Log<TState>(LogLevel logLevel, EventId eventId, TState state, Except
140144
_eventGenerator.LogAzureMonitorDiagnosticLogEvent(logLevel, _hostNameProvider.Value, AzureMonitorOperationName, AzureMonitorCategoryName, _regionName, sw.ToString());
141145
}
142146

147+
public void Dispose()
148+
{
149+
_appServiceOptionsOnChangeListener?.Dispose();
150+
}
151+
152+
private void UpdateAppServiceOptions(AppServiceOptions newOptions)
153+
{
154+
_appServiceOptions = newOptions;
155+
isAzureMonitorLogsEnabled = IsAzureMonitorLoggingEnabled(_appServiceOptions.AzureMonitorTraceCategory);
156+
}
157+
143158
private static void WritePropertyIfNotNull<T>(JsonTextWriter writer, string propertyName, T propertyValue)
144159
{
145160
if (propertyValue != null)

‎src/WebJobs.Script.WebHost/WebScriptHostBuilderExtension.cs

Lines changed: 5 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,6 @@
55
using System.Linq;
66
using System.Net.Http;
77
using System.Runtime.InteropServices;
8-
using Autofac.Core;
9-
using Google.Protobuf.WellKnownTypes;
108
using Microsoft.AspNetCore.Mvc.ApplicationParts;
119
using Microsoft.Azure.WebJobs.Host.Config;
1210
using Microsoft.Azure.WebJobs.Host.Executors;
@@ -34,6 +32,7 @@
3432
using Microsoft.Extensions.Hosting;
3533
using Microsoft.Extensions.Logging;
3634
using Microsoft.Extensions.Options;
35+
using static Microsoft.Azure.WebJobs.Script.Utility;
3736

3837
namespace Microsoft.Azure.WebJobs.Script.WebHost
3938
{
@@ -95,27 +94,19 @@ public static IHostBuilder AddWebScriptHost(this IHostBuilder builder, IServiceP
9594
loggingBuilder.AddWebJobsSystem<SystemLoggerProvider>();
9695
if (environment.IsAzureMonitorEnabled())
9796
{
98-
if (environment.IsLegionBasedSku())
97+
if (environment.IsConsumptionOnLegion())
9998
{
10099
loggingBuilder.Services.AddOptions<LoggerFilterOptions>()
101100
.Configure<IConfiguration>((options, config) =>
102101
{
103-
string setting = config[EnvironmentSettingNames.AzureMonitorCategories];
102+
string value = config[EnvironmentSettingNames.AzureMonitorCategories];
104103
options.AddFilter<AzureMonitorDiagnosticLoggerProvider>((category, level) =>
105104
{
106-
if (setting == null)
107-
{
108-
return true;
109-
}
110-
string[] categories = setting.Split(',');
111-
return categories.Contains(ScriptConstants.AzureMonitorTraceCategory);
105+
return IsAzureMonitorLoggingEnabled(value);
112106
});
113107
});
114108
}
115-
else
116-
{
117-
loggingBuilder.Services.AddSingleton<ILoggerProvider, AzureMonitorDiagnosticLoggerProvider>();
118-
}
109+
loggingBuilder.Services.AddSingleton<ILoggerProvider, AzureMonitorDiagnosticLoggerProvider>();
119110
}
120111

121112
if (!FeatureFlags.IsEnabled(ScriptConstants.FeatureFlagDisableDiagnosticEventLogging))

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,5 +12,7 @@ public class AppServiceOptions
1212
public string RuntimeSiteName { get; set; }
1313

1414
public string SlotName { get; set; }
15+
16+
public string AzureMonitorTraceCategory { get; set; }
1517
}
1618
}

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

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

44
using Microsoft.Extensions.Options;
5+
using static Microsoft.Azure.WebJobs.Script.EnvironmentSettingNames;
56

67
namespace Microsoft.Azure.WebJobs.Script.Configuration
78
{
@@ -20,6 +21,7 @@ public void Configure(AppServiceOptions options)
2021
options.SubscriptionId = _environment.GetSubscriptionId() ?? string.Empty;
2122
options.RuntimeSiteName = _environment.GetRuntimeSiteName() ?? string.Empty;
2223
options.SlotName = _environment.GetSlotName() ?? string.Empty;
24+
options.AzureMonitorTraceCategory = _environment.GetEnvironmentVariable(AzureMonitorCategories) ?? string.Empty;
2325
}
2426
}
2527
}

‎src/WebJobs.Script/Environment/EnvironmentExtensions.cs

Lines changed: 4 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
using Microsoft.Azure.WebJobs.Script.Description;
1212
using Microsoft.Azure.WebJobs.Script.Workers.Rpc;
1313
using static Microsoft.Azure.WebJobs.Script.EnvironmentSettingNames;
14+
using static Microsoft.Azure.WebJobs.Script.Utility;
1415

1516
namespace Microsoft.Azure.WebJobs.Script
1617
{
@@ -20,8 +21,6 @@ internal static class EnvironmentExtensions
2021

2122
private static bool? isMultiLanguageEnabled;
2223

23-
private static bool? isAzureMonitorLogsSubscribed;
24-
2524
internal static string BaseDirectory { get; set; }
2625

2726
public static string GetEnvironmentVariableOrDefault(this IEnvironment environment, string name, string defaultValue)
@@ -69,26 +68,9 @@ public static bool IsEasyAuthEnabled(this IEnvironment environment)
6968
/// <summary>
7069
/// Returns true if any Functions AzureMonitor log categories are enabled.
7170
/// </summary>
72-
public static bool IsAzureMonitorEnabled(this IEnvironment environment, bool useCache=false)
71+
public static bool IsAzureMonitorEnabled(this IEnvironment environment)
7372
{
74-
string value = environment.GetEnvironmentVariable(AzureMonitorCategories);
75-
if (value == null)
76-
{
77-
return true;
78-
}
79-
if (string.Equals(ScriptConstants.DefaultAzureMonitorCategories, value, StringComparison.Ordinal))
80-
{
81-
// Default value for the env variable is None.
82-
// This is set when customer does not subscribe any category.
83-
return false;
84-
}
85-
if (useCache && isAzureMonitorLogsSubscribed != null)
86-
{
87-
return isAzureMonitorLogsSubscribed.Value;
88-
}
89-
string[] categories = value.Split(',');
90-
isAzureMonitorLogsSubscribed = categories.Contains(ScriptConstants.AzureMonitorTraceCategory);
91-
return isAzureMonitorLogsSubscribed.Value;
73+
return IsAzureMonitorLoggingEnabled(environment.GetEnvironmentVariable(AzureMonitorCategories));
9274
}
9375

9476
/// <summary>
@@ -264,16 +246,6 @@ public static bool IsFlexConsumptionSku(this IEnvironment environment)
264246
return !environment.WebsiteSkuIsDynamic();
265247
}
266248

267-
/// <summary>
268-
/// Gets a value indicating whether the application is running in Legion.
269-
/// </summary>
270-
/// <param name="environment">The environment to verify.</param>
271-
/// <returns><see cref="true"/> if running on legion, false otherwise.</returns>
272-
public static bool IsLegionBasedSku(this IEnvironment environment)
273-
{
274-
return environment.IsFlexConsumptionSku() || environment.IsLinuxConsumptionOnLegion();
275-
}
276-
277249
/// <summary>
278250
/// Returns true if the app is running on Virtual Machine Scale Sets (VMSS).
279251
/// </summary>
@@ -360,7 +332,7 @@ public static bool IsLinuxConsumptionOnAtlas(this IEnvironment environment)
360332
string.IsNullOrEmpty(environment.GetEnvironmentVariable(LegionServiceHost));
361333
}
362334

363-
private static bool IsConsumptionOnLegion(this IEnvironment environment)
335+
public static bool IsConsumptionOnLegion(this IEnvironment environment)
364336
{
365337
return !environment.IsAppService() &&
366338
(!string.IsNullOrEmpty(environment.GetEnvironmentVariable(ContainerName)) ||

‎src/WebJobs.Script/Utility.cs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1139,6 +1139,22 @@ public static bool TryReadAsBool(IDictionary<string, object> properties, string
11391139
return result = false;
11401140
}
11411141

1142+
public static bool IsAzureMonitorLoggingEnabled(string azureMonitorcategoriesSubscribed)
1143+
{
1144+
if (azureMonitorcategoriesSubscribed == null)
1145+
{
1146+
return true;
1147+
}
1148+
if (string.Equals(ScriptConstants.DefaultAzureMonitorCategories, azureMonitorcategoriesSubscribed, StringComparison.Ordinal))
1149+
{
1150+
// Default value for the env variable is None.
1151+
// This is set when customer does not subscribe any category.
1152+
return false;
1153+
}
1154+
string[] categories = azureMonitorcategoriesSubscribed.Split(',');
1155+
return categories.Contains(ScriptConstants.AzureMonitorTraceCategory);
1156+
}
1157+
11421158
private class FilteredExpandoObjectConverter : ExpandoObjectConverter
11431159
{
11441160
public override bool CanWrite => true;

0 commit comments

Comments
 (0)