Skip to content

Commit c8eb2d6

Browse files
sandermvanvlietsandermvanvliet
committed
Only modify the log event if properties have actually been masked
This should improve performance by not doing unnecessary work
1 parent f3de165 commit c8eb2d6

File tree

2 files changed

+89
-32
lines changed

2 files changed

+89
-32
lines changed

‎src/Serilog.Enrichers.Sensitive/SensitiveDataEnricher.cs

Lines changed: 60 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -64,69 +64,98 @@ public void Enrich(LogEvent logEvent, ILogEventPropertyFactory propertyFactory)
6464
{
6565
if (_maskingMode == MaskingMode.Globally || SensitiveArea.Instance != null)
6666
{
67-
var messageTemplateText = ReplaceSensitiveDataFromString(logEvent.MessageTemplate.Text);
67+
var (wasTemplateMasked, messageTemplateText) = ReplaceSensitiveDataFromString(logEvent.MessageTemplate.Text);
6868

69-
_messageTemplateBackingField.SetValue(logEvent, Parser.Parse(messageTemplateText));
69+
// Only replace the template if it was actually masked
70+
if (wasTemplateMasked)
71+
{
72+
_messageTemplateBackingField.SetValue(logEvent, Parser.Parse(messageTemplateText));
73+
}
7074

7175
foreach (var property in logEvent.Properties.ToList())
7276
{
73-
var maskedValue = MaskProperty(property);
74-
75-
logEvent
76-
.AddOrUpdateProperty(
77-
new LogEventProperty(
78-
property.Key,
79-
maskedValue));
77+
var (wasMasked, maskedValue) = MaskProperty(property);
78+
79+
// Only update the property if it was actually masked
80+
if (wasMasked)
81+
{
82+
logEvent
83+
.AddOrUpdateProperty(
84+
new LogEventProperty(
85+
property.Key,
86+
maskedValue));
87+
}
8088
}
8189
}
8290
}
8391

84-
private LogEventPropertyValue MaskProperty(KeyValuePair<string, LogEventPropertyValue> property)
92+
private (bool, LogEventPropertyValue?) MaskProperty(KeyValuePair<string, LogEventPropertyValue> property)
8593
{
8694
if (_excludeProperties.Contains(property.Key, StringComparer.InvariantCultureIgnoreCase))
8795
{
88-
return property.Value;
96+
return (false, null);
8997
}
9098

9199
if (_maskProperties.Contains(property.Key, StringComparer.InvariantCultureIgnoreCase))
92100
{
93-
return new ScalarValue(_maskValue);
101+
return (true, new ScalarValue(_maskValue));
94102
}
95103

96-
if (property.Value is ScalarValue scalar && scalar.Value is string stringValue)
104+
switch (property.Value)
97105
{
98-
return new ScalarValue(ReplaceSensitiveDataFromString(stringValue));
99-
}
100-
if (property.Value is StructureValue structureValue)
101-
{
102-
var propList = new List<LogEventProperty>();
103-
104-
foreach (var prop in structureValue.Properties)
105-
{
106-
var maskedValue = MaskProperty(new KeyValuePair<string, LogEventPropertyValue>(prop.Name, prop.Value));
107-
108-
propList.Add(new LogEventProperty(prop.Name, maskedValue));
109-
}
110-
111-
return new StructureValue(propList);
106+
case ScalarValue { Value: string stringValue }:
107+
{
108+
var (wasMasked, maskedValue) = ReplaceSensitiveDataFromString(stringValue);
109+
110+
if (wasMasked)
111+
{
112+
return (true, new ScalarValue(maskedValue));
113+
}
114+
115+
return (false, null);
116+
}
117+
case StructureValue structureValue:
118+
{
119+
var propList = new List<LogEventProperty>();
120+
var anyMasked = false;
121+
foreach (var prop in structureValue.Properties)
122+
{
123+
var (wasMasked, maskedValue) = MaskProperty(new KeyValuePair<string, LogEventPropertyValue>(prop.Name, prop.Value));
124+
125+
if (wasMasked)
126+
{
127+
anyMasked = true;
128+
propList.Add(new LogEventProperty(prop.Name, maskedValue));
129+
}
130+
else
131+
{
132+
propList.Add(prop);
133+
}
134+
}
135+
136+
return (anyMasked, new StructureValue(propList));
137+
}
138+
default:
139+
return (false, null);
112140
}
113-
114-
return property.Value;
115141
}
116142

117-
private string ReplaceSensitiveDataFromString(string input)
143+
private (bool, string) ReplaceSensitiveDataFromString(string input)
118144
{
145+
var isMasked = false;
146+
119147
foreach (var maskingOperator in _maskingOperators)
120148
{
121149
var maskResult = maskingOperator.Mask(input, _maskValue);
122150

123151
if (maskResult.Match)
124152
{
153+
isMasked = true;
125154
input = maskResult.Result;
126155
}
127156
}
128157

129-
return input;
158+
return (isMasked, input);
130159
}
131160

132161
public static IEnumerable<IMaskingOperator> DefaultOperators => new List<IMaskingOperator>

‎test/Serilog.Enrichers.Sensitive.Tests.Unit/WhenMaskingDestructuredObject.cs

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
using System.Collections.Generic;
2+
using System.Linq;
23
using Serilog.Core;
4+
using Serilog.Events;
35
using Serilog.Sinks.InMemory;
46
using Serilog.Sinks.InMemory.Assertions;
57
using Xunit;
@@ -19,7 +21,6 @@ public WhenMaskingDestructuredObject()
1921
.WriteTo.Sink(_sink)
2022
.Enrich.WithSensitiveDataMasking(options =>
2123
{
22-
options.MaskProperties.Add("TestProperty");
2324
options.MaskingOperators = new List<IMaskingOperator> { new EmailAddressMaskingOperator() };
2425
})
2526
.CreateLogger();
@@ -62,6 +63,33 @@ public void GivenLogMessageWithDestructuredObjectPropertyThatHasSensitiveDataInN
6263
.WithProperty("TestProperty")
6364
.WithValue("***MASKED***");
6465
}
66+
67+
[Fact]
68+
public void GivenLogMessageWithDestructuredObjectPropertyWithoutSensitiveDataInNestedProperty_StructureValueIsUnchanged()
69+
{
70+
var testObject = new TestObject
71+
{
72+
TestProperty = "not sensitive",
73+
Nested = new NestedTestObject
74+
{
75+
TestProperty = "also not sensitive"
76+
}
77+
};
78+
79+
_logger.Information("Test message {@TestObject}", testObject);
80+
81+
_sink
82+
.Should()
83+
.HaveMessage("Test message {@TestObject}")
84+
.Appearing()
85+
.Once()
86+
.WithProperty("TestObject")
87+
.HavingADestructuredObject()
88+
.WithProperty("Nested")
89+
.HavingADestructuredObject()
90+
.WithProperty("TestProperty")
91+
.WithValue("also not sensitive");
92+
}
6593
}
6694

6795
public class TestObject

0 commit comments

Comments
 (0)