Skip to content

Commit 534a66e

Browse files
manikantanallagatlaManikanta Nallagatla
manikantanallagatla
and
Manikanta Nallagatla
authored
Filter Azure monitor logs when customer does not subscribe for the categories (#10982)
* try logger changes * address all comments * spaces change * correct filter * clean code * address comments * Addres more comments * Update tests * Fix tests build * address comments * Address commments * address comment * Fix UTs --------- Co-authored-by: Manikanta Nallagatla <[email protected]>
1 parent 265424c commit 534a66e

File tree

9 files changed

+71
-38
lines changed

9 files changed

+71
-38
lines changed

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

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,8 @@ public class AzureMonitorDiagnosticLogger : ILogger
2929
private readonly IEventGenerator _eventGenerator;
3030
private readonly IEnvironment _environment;
3131
private readonly IExternalScopeProvider _scopeProvider;
32-
private AppServiceOptions _appServiceOptions;
32+
private bool _isAzureMonitorLoggingEnabled;
33+
private IOptionsMonitor<AppServiceOptions> _appServiceOptionsMonitor;
3334

3435
public AzureMonitorDiagnosticLogger(string category, string hostInstanceId, IEventGenerator eventGenerator, IEnvironment environment, IExternalScopeProvider scopeProvider,
3536
HostNameProvider hostNameProvider, IOptionsMonitor<AppServiceOptions> appServiceOptionsMonitor)
@@ -40,10 +41,9 @@ public AzureMonitorDiagnosticLogger(string category, string hostInstanceId, IEve
4041
_environment = environment ?? throw new ArgumentNullException(nameof(environment));
4142
_scopeProvider = scopeProvider ?? throw new ArgumentNullException(nameof(scopeProvider));
4243
_hostNameProvider = hostNameProvider ?? throw new ArgumentNullException(nameof(hostNameProvider));
43-
_ = appServiceOptionsMonitor ?? throw new ArgumentNullException(nameof(appServiceOptionsMonitor));
44-
45-
appServiceOptionsMonitor.OnChange(newOptions => _appServiceOptions = newOptions);
46-
_appServiceOptions = appServiceOptionsMonitor.CurrentValue;
44+
_appServiceOptionsMonitor = appServiceOptionsMonitor ?? throw new ArgumentNullException(nameof(appServiceOptionsMonitor));
45+
UpdateAppServiceOptions(_appServiceOptionsMonitor.CurrentValue);
46+
_appServiceOptionsMonitor.OnChange(UpdateAppServiceOptions);
4747

4848
_roleInstance = _environment.GetInstanceId();
4949

@@ -55,7 +55,7 @@ public AzureMonitorDiagnosticLogger(string category, string hostInstanceId, IEve
5555
public bool IsEnabled(LogLevel logLevel)
5656
{
5757
// We want to instantiate this Logger in placeholder mode to warm it up, but do not want to log anything.
58-
return !string.IsNullOrEmpty(_hostNameProvider.Value) && !_environment.IsPlaceholderModeEnabled();
58+
return _isAzureMonitorLoggingEnabled && !string.IsNullOrEmpty(_hostNameProvider.Value) && !_environment.IsPlaceholderModeEnabled();
5959
}
6060

6161
public void Log<TState>(LogLevel logLevel, EventId eventId, TState state, Exception exception, Func<TState, Exception, string> formatter)
@@ -107,7 +107,7 @@ public void Log<TState>(LogLevel logLevel, EventId eventId, TState state, Except
107107
using (JsonTextWriter writer = new JsonTextWriter(sw) { Formatting = Formatting.None })
108108
{
109109
writer.WriteStartObject();
110-
WritePropertyIfNotNull(writer, "appName", _appServiceOptions.AppName);
110+
WritePropertyIfNotNull(writer, "appName", _appServiceOptionsMonitor.CurrentValue.AppName);
111111
WritePropertyIfNotNull(writer, "roleInstance", _roleInstance);
112112
WritePropertyIfNotNull(writer, "message", formattedMessage);
113113
WritePropertyIfNotNull(writer, "category", _category);
@@ -136,6 +136,11 @@ public void Log<TState>(LogLevel logLevel, EventId eventId, TState state, Except
136136
_eventGenerator.LogAzureMonitorDiagnosticLogEvent(logLevel, _hostNameProvider.Value, AzureMonitorOperationName, AzureMonitorCategoryName, _regionName, sw.ToString());
137137
}
138138

139+
private void UpdateAppServiceOptions(AppServiceOptions appServiceOptions)
140+
{
141+
_isAzureMonitorLoggingEnabled = !(_environment.IsConsumptionOnLegion() && !_appServiceOptionsMonitor.CurrentValue.IsAzureMonitorLoggingEnabled);
142+
}
143+
139144
private static void WritePropertyIfNotNull<T>(JsonTextWriter writer, string propertyName, T propertyValue)
140145
{
141146
if (propertyValue != null)

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

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,11 +25,13 @@
2525
using Microsoft.Azure.WebJobs.Script.WebHost.Management;
2626
using Microsoft.Azure.WebJobs.Script.WebHost.Middleware;
2727
using Microsoft.Azure.WebJobs.Script.WebHost.Storage;
28+
using Microsoft.Extensions.Configuration;
2829
using Microsoft.Extensions.DependencyInjection;
2930
using Microsoft.Extensions.DependencyInjection.Extensions;
3031
using Microsoft.Extensions.Hosting;
3132
using Microsoft.Extensions.Logging;
3233
using Microsoft.Extensions.Options;
34+
using static Microsoft.Azure.WebJobs.Script.Utility;
3335

3436
namespace Microsoft.Azure.WebJobs.Script.WebHost
3537
{
@@ -91,6 +93,17 @@ public static IHostBuilder AddWebScriptHost(this IHostBuilder builder, IServiceP
9193
loggingBuilder.AddWebJobsSystem<SystemLoggerProvider>();
9294
if (environment.IsAzureMonitorEnabled())
9395
{
96+
if (environment.IsConsumptionOnLegion())
97+
{
98+
loggingBuilder.Services.AddOptions<LoggerFilterOptions>()
99+
.Configure<IOptionsMonitor<AppServiceOptions>>((filters, options) =>
100+
{
101+
filters.AddFilter<AzureMonitorDiagnosticLoggerProvider>((category, level) =>
102+
{
103+
return options.CurrentValue.IsAzureMonitorLoggingEnabled;
104+
});
105+
});
106+
}
94107
loggingBuilder.Services.AddSingleton<ILoggerProvider, AzureMonitorDiagnosticLoggerProvider>();
95108
}
96109

‎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 bool IsAzureMonitorLoggingEnabled { get; set; }
1517
}
1618
}

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
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;
6+
using static Microsoft.Azure.WebJobs.Script.Utility;
57

68
namespace Microsoft.Azure.WebJobs.Script.Configuration
79
{
@@ -20,6 +22,7 @@ public void Configure(AppServiceOptions options)
2022
options.SubscriptionId = _environment.GetSubscriptionId() ?? string.Empty;
2123
options.RuntimeSiteName = _environment.GetRuntimeSiteName() ?? string.Empty;
2224
options.SlotName = _environment.GetSlotName() ?? string.Empty;
25+
options.IsAzureMonitorLoggingEnabled = _environment.IsAzureMonitorEnabled();
2326
}
2427
}
2528
}

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

Lines changed: 3 additions & 8 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
{
@@ -69,13 +70,7 @@ public static bool IsEasyAuthEnabled(this IEnvironment environment)
6970
/// </summary>
7071
public static bool IsAzureMonitorEnabled(this IEnvironment environment)
7172
{
72-
string value = environment.GetEnvironmentVariable(AzureMonitorCategories);
73-
if (value == null)
74-
{
75-
return true;
76-
}
77-
string[] categories = value.Split(',');
78-
return categories.Contains(ScriptConstants.AzureMonitorTraceCategory);
73+
return IsAzureMonitorLoggingEnabled(environment.GetEnvironmentVariable(AzureMonitorCategories));
7974
}
8075

8176
/// <summary>
@@ -337,7 +332,7 @@ public static bool IsLinuxConsumptionOnAtlas(this IEnvironment environment)
337332
string.IsNullOrEmpty(environment.GetEnvironmentVariable(LegionServiceHost));
338333
}
339334

340-
private static bool IsConsumptionOnLegion(this IEnvironment environment)
335+
public static bool IsConsumptionOnLegion(this IEnvironment environment)
341336
{
342337
return !environment.IsAppService() &&
343338
(!string.IsNullOrEmpty(environment.GetEnvironmentVariable(ContainerName)) ||

‎src/WebJobs.Script/ScriptConstants.cs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -227,6 +227,7 @@ public static class ScriptConstants
227227
public const string ExtendedPlatformChannelNameUpper = "EXTENDED";
228228

229229
public const string AzureMonitorTraceCategory = "FunctionAppLogs";
230+
public const string DefaultAzureMonitorCategories = "None";
230231

231232
public const string KubernetesManagedAppName = "K8SE_APP_NAME";
232233
public const string KubernetesManagedAppNamespace = "K8SE_APP_NAMESPACE";

‎src/WebJobs.Script/Utility.cs‎

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

1148+
public static bool IsAzureMonitorLoggingEnabled(string azureMonitorcategoriesSubscribed)
1149+
{
1150+
if (azureMonitorcategoriesSubscribed == null)
1151+
{
1152+
return true;
1153+
}
1154+
if (string.Equals(ScriptConstants.DefaultAzureMonitorCategories, azureMonitorcategoriesSubscribed, StringComparison.Ordinal))
1155+
{
1156+
// Default value for the env variable is None.
1157+
// This is set when customer does not subscribe any category.
1158+
return false;
1159+
}
1160+
string[] categories = azureMonitorcategoriesSubscribed.Split(',');
1161+
return categories.Contains(ScriptConstants.AzureMonitorTraceCategory);
1162+
}
1163+
11481164
private class FilteredExpandoObjectConverter : ExpandoObjectConverter
11491165
{
11501166
public override bool CanWrite => true;

‎test/WebJobs.Script.Tests/Eventing/DiagnosticLoggerTests.cs‎

Lines changed: 19 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -206,35 +206,31 @@ public void Log_Sanitizes()
206206
Assert.True(JToken.DeepEquals(actual, expected), $"Actual: {actual.ToString()}{Environment.NewLine}Expected: {expected.ToString()}");
207207
}
208208

209-
[Fact]
210-
public void Log_DisabledIfPlaceholder()
209+
[Theory]
210+
[InlineData(EnvironmentSettingNames.AzureWebsitePlaceholderMode, "1", false, false, false)] // Placeholder
211+
[InlineData(EnvironmentSettingNames.AzureWebsitePlaceholderMode, "1", true, false, false)] // Placeholder
212+
[InlineData(EnvironmentSettingNames.AzureWebsiteHostName, null, false, false, false)] // NoSiteName
213+
[InlineData(EnvironmentSettingNames.AzureWebsiteHostName, null, true, false, false)] // NoSiteName
214+
[InlineData(EnvironmentSettingNames.AzureWebsiteHostName, "host", false, false, true)]
215+
[InlineData(EnvironmentSettingNames.AzureWebsiteHostName, "host", true, false, false)]
216+
[InlineData(EnvironmentSettingNames.AzureWebsiteHostName, "host", true, true, true)]
217+
public void Log_IsEnabled(string envVariableName, string envVariableVale, bool isConsumptionOnLegion, bool isAzureMonitorEnabled, bool isDisabled)
211218
{
212-
string message = "TestMessage";
213219
string functionInvocationId = Guid.NewGuid().ToString();
214220
string activityId = Guid.NewGuid().ToString();
215221

216-
_environment.SetEnvironmentVariable(EnvironmentSettingNames.AzureWebsitePlaceholderMode, "1");
217-
218-
_logger.LogInformation(message);
219-
220-
Assert.False(_logger.IsEnabled(LogLevel.Information));
221-
_mockEventGenerator.Verify(m => m.LogAzureMonitorDiagnosticLogEvent(It.IsAny<LogLevel>(), It.IsAny<string>(), It.IsAny<string>(), It.IsAny<string>(), It.IsAny<string>(), It.IsAny<string>()), Times.Never);
222-
}
223-
224-
[Fact]
225-
public void Log_DisabledIfNoSiteName()
226-
{
227-
string message = "TestMessage";
228-
string functionInvocationId = Guid.NewGuid().ToString();
229-
string activityId = Guid.NewGuid().ToString();
230-
_environment.SetEnvironmentVariable(EnvironmentSettingNames.AzureWebsiteHostName, null);
231-
232-
// Recreate the logger was we cache the site name in the constructor
233-
ILogger logger = new AzureMonitorDiagnosticLogger(_category, _hostInstanceId, _mockEventGenerator.Object, _environment, new LoggerExternalScopeProvider(), _hostNameProvider, _appServiceOptionsWrapper);
222+
_environment.SetEnvironmentVariable(envVariableName, envVariableVale);
223+
if (isConsumptionOnLegion)
224+
{
225+
_environment.SetEnvironmentVariable(EnvironmentSettingNames.AzureWebsiteInstanceId, string.Empty);
226+
_environment.SetEnvironmentVariable(EnvironmentSettingNames.ContainerName, "containername");
227+
_environment.SetEnvironmentVariable(EnvironmentSettingNames.LegionServiceHost, "legionhost");
228+
}
234229

235-
logger.LogInformation(message);
230+
_appServiceOptionsWrapper.CurrentValue.IsAzureMonitorLoggingEnabled = isAzureMonitorEnabled;
231+
_appServiceOptionsWrapper.InvokeChanged();
236232

237-
Assert.False(logger.IsEnabled(LogLevel.Information));
233+
Assert.Equal(isDisabled, _logger.IsEnabled(LogLevel.Information));
238234
_mockEventGenerator.Verify(m => m.LogAzureMonitorDiagnosticLogEvent(It.IsAny<LogLevel>(), It.IsAny<string>(), It.IsAny<string>(), It.IsAny<string>(), It.IsAny<string>(), It.IsAny<string>()), Times.Never);
239235
}
240236

‎test/WebJobs.Script.Tests/Extensions/EnvironmentExtensionsTests.cs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,8 @@ public void IsVMSS_RetrunsExpectedResult(string roleInstanceId, bool expected)
6262
[InlineData(null, true)]
6363
[InlineData("", false)]
6464
[InlineData("Foo,FunctionAppLogs,Bar", true)]
65+
[InlineData("FunctionAppLogs", true)]
66+
[InlineData("None", false)]
6567
[InlineData("Foo,Bar", false)]
6668
public void IsAzureMonitorEnabled_ReturnsExpectedResult(string value, bool expected)
6769
{

0 commit comments

Comments
 (0)