Remove Santize from MetricsLogger hotpath (#9489)

Azure · Aug 28, 2023 · 6a08852 · 6a08852
1 parent ebaf1af
commit 6a08852
Show file tree

Hide file tree

Showing 5 changed files with 10 additions and 10 deletions.
diff --git a/src/WebJobs.Script.Grpc/Channel/GrpcWorkerChannel.cs b/src/WebJobs.Script.Grpc/Channel/GrpcWorkerChannel.cs
@@ -423,7 +423,7 @@ private void LogWorkerMetadata(WorkerMetadata workerMetadata)
 
             workerMetadata.UpdateWorkerMetadata(_workerConfig);
             var workerMetadataString = workerMetadata.ToString();
-            _metricsLogger.LogEvent(MetricEventNames.WorkerMetadata, functionName: null, workerMetadataString);
+            _metricsLogger.LogEvent(MetricEventNames.WorkerMetadata, functionName: null, Sanitizer.Sanitize(workerMetadataString));
             _workerChannelLogger.LogDebug("Worker metadata: {workerMetadata}", workerMetadataString);
         }
 
@@ -670,7 +670,7 @@ internal FunctionLoadRequest GetFunctionLoadRequest(FunctionMetadata metadata, M
 
                 if (binding.SupportsDeferredBinding() && !binding.SkipDeferredBinding())
                 {
-                    _metricsLogger.LogEvent(MetricEventNames.FunctionBindingDeferred, functionName: metadata.Name);
+                    _metricsLogger.LogEvent(MetricEventNames.FunctionBindingDeferred, functionName: Sanitizer.Sanitize(metadata.Name));
                 }
             }
 
@@ -757,8 +757,7 @@ internal async Task SendInvocationRequest(ScriptInvocationContext context)
                 var invocationRequest = await context.ToRpcInvocationRequest(_workerChannelLogger, _workerCapabilities, _isSharedMemoryDataTransferEnabled, _sharedMemoryManager);
                 AddAdditionalTraceContext(invocationRequest.TraceContext.Attributes, context);
                 _executingInvocations.TryAdd(invocationRequest.InvocationId, context);
-
-                _metricsLogger.LogEvent(string.Format(MetricEventNames.WorkerInvoked, Id), functionName: context.FunctionMetadata.Name);
+                _metricsLogger.LogEvent(string.Format(MetricEventNames.WorkerInvoked, Id), functionName: Sanitizer.Sanitize(context.FunctionMetadata.Name));
 
                 await SendStreamingMessageAsync(new StreamingMessage
                 {

diff --git a/src/WebJobs.Script.WebHost/Diagnostics/FunctionInstanceLogger.cs b/src/WebJobs.Script.WebHost/Diagnostics/FunctionInstanceLogger.cs
@@ -7,6 +7,7 @@
 using System.Threading;
 using System.Threading.Tasks;
 using Microsoft.Azure.WebJobs.Host.Loggers;
+using Microsoft.Azure.WebJobs.Logging;
 using Microsoft.Azure.WebJobs.Script.Description;
 using Microsoft.Azure.WebJobs.Script.Diagnostics;
 using Microsoft.Extensions.Configuration;
@@ -112,7 +113,7 @@ private void EndFunction(FunctionInstanceLogEntry item)
                 data = string.Format(Microsoft.Azure.WebJobs.Script.Properties.Resources.FunctionInvocationMetricsData, startedEvent.FunctionMetadata.Language, functionName, success, Stopwatch.IsHighResolution);
                 _eventDataCache[key] = data;
             }
-            _metrics.LogEvent(eventName, startedEvent.FunctionName, data);
+            _metrics.LogEvent(eventName, startedEvent.FunctionName, Sanitizer.Sanitize(data));
 
             startedEvent.Data = data;
             _metrics.EndEvent(startedEvent);

diff --git a/src/WebJobs.Script.WebHost/Diagnostics/MetricsEventManager.cs b/src/WebJobs.Script.WebHost/Diagnostics/MetricsEventManager.cs
@@ -119,8 +119,6 @@ public void LogEvent(string eventName, string functionName = null, string data =
         {
             ArgumentNullException.ThrowIfNull(eventName);
 
-            eventName = Sanitizer.Sanitize(eventName);
-
             string key = GetAggregateKey(eventName, functionName);
             QueuedEvents.AddOrUpdate(key,
                 (name) =>

diff --git a/src/WebJobs.Script/Host/ScriptHost.cs b/src/WebJobs.Script/Host/ScriptHost.cs
@@ -279,12 +279,12 @@ public async Task InitializeAsync(CancellationToken cancellationToken = default)
                 IEnumerable<FunctionMetadata> functionMetadataList = GetFunctionsMetadata();
 
                 string workerRuntime = _environment.GetEnvironmentVariable(EnvironmentSettingNames.FunctionWorkerRuntime);
-                _logger.FunctionsWorkerRuntimeValue(workerRuntime);
+                _logger.FunctionsWorkerRuntimeValue(Sanitizer.Sanitize(workerRuntime));
 
                 if (workerRuntime is null)
                 {
                     workerRuntime = Utility.GetWorkerRuntime(functionMetadataList);
-                    _logger.ResolvedWorkerRuntimeFromMetadata(workerRuntime);
+                    _logger.ResolvedWorkerRuntimeFromMetadata(Sanitizer.Sanitize(workerRuntime));
                 }
 
                 if (!_environment.IsPlaceholderModeEnabled())

diff --git a/test/WebJobs.Script.Tests.Integration/Diagnostics/MetricsEventManagerTests.cs b/test/WebJobs.Script.Tests.Integration/Diagnostics/MetricsEventManagerTests.cs
@@ -9,6 +9,7 @@
 using System.Text;
 using System.Threading;
 using System.Threading.Tasks;
+using Microsoft.Azure.WebJobs.Logging;
 using Microsoft.Azure.WebJobs.Script.Configuration;
 using Microsoft.Azure.WebJobs.Script.Description;
 using Microsoft.Azure.WebJobs.Script.Diagnostics;
@@ -126,7 +127,8 @@ public MetricsEventManagerTests()
         [InlineData("{ \"AzureWebJobsStorage\": \"DefaultEndpointsProtocol=https;AccountName=testAccount1;AccountKey=mykey1;EndpointSuffix=core.windows.net\", \"AnotherKey\": \"AnotherValue\" }", "{ \"azurewebjobsstorage\": \"[hidden credential]\", \"anotherkey\": \"anothervalue\" }")]
         public void LogEvent_QueuesPendingEvent(string eventName, string expectedEventName)
         {
-            _metricsLogger.LogEvent(eventName);
+            //Note: Caller is responsible for sanitizing the string
+            _metricsLogger.LogEvent(Sanitizer.Sanitize(eventName));
 
             Assert.Equal(1, _metricsEventManager.QueuedEvents.Count);
             SystemMetricEvent evt = _metricsEventManager.QueuedEvents.Values.Single();