Skip to content

Commit 2bc0ea5

Browse files
andrii-babanin-symandrii-babanin-sym
committed
Make EventIdPropertyCache non-static; Create unit tests.
1 parent 29b94f3 commit 2bc0ea5

File tree

3 files changed

+154
-73
lines changed

3 files changed

+154
-73
lines changed

‎src/Serilog.Extensions.Logging/Extensions/Logging/EventIdPropertyCache.cs

Lines changed: 59 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -12,75 +12,86 @@
1212
// See the License for the specific language governing permissions and
1313
// limitations under the License.
1414

15-
namespace Serilog.Extensions.Logging
15+
namespace Serilog.Extensions.Logging;
16+
17+
using System.Collections.Concurrent;
18+
using Microsoft.Extensions.Logging;
19+
using Serilog.Events;
20+
21+
class EventIdPropertyCache
1622
{
17-
using System.Collections.Concurrent;
18-
using Microsoft.Extensions.Logging;
19-
using Serilog.Events;
23+
readonly int _maxCachedProperties;
24+
readonly ConcurrentDictionary<EventKey, LogEventProperty> _propertyCache = new();
2025

21-
static class EventIdPropertyCache
22-
{
23-
const int MaxCachedProperties = 1024;
26+
int _count;
2427

25-
static readonly ConcurrentDictionary<EventKey, LogEventProperty> s_propertyCache = new();
26-
static int s_count;
28+
public EventIdPropertyCache(int maxCachedProperties = 1024)
29+
{
30+
_maxCachedProperties = maxCachedProperties;
31+
}
2732

28-
public static LogEventProperty GetOrCreateProperty(in EventId eventId)
29-
{
30-
var eventKey = new EventKey(eventId);
33+
public LogEventProperty GetOrCreateProperty(in EventId eventId)
34+
{
35+
var eventKey = new EventKey(eventId);
3136

32-
LogEventProperty? property;
37+
LogEventProperty? property;
3338

34-
if (s_count >= MaxCachedProperties)
39+
if (_count >= _maxCachedProperties)
40+
{
41+
if (!_propertyCache.TryGetValue(eventKey, out property))
3542
{
36-
if (!s_propertyCache.TryGetValue(eventKey, out property))
37-
{
38-
property = CreateCore(in eventKey);
39-
}
43+
property = CreateProperty(in eventKey);
4044
}
41-
else
45+
}
46+
else
47+
{
48+
if (!_propertyCache.TryGetValue(eventKey, out property))
4249
{
43-
property = s_propertyCache.GetOrAdd(
44-
eventKey,
45-
static key =>
46-
{
47-
Interlocked.Increment(ref s_count);
48-
49-
return CreateCore(in key);
50-
});
50+
// GetOrAdd is moved to a separate method to prevent closure allocation
51+
property = GetOrAddCore(in eventKey);
5152
}
53+
}
5254

53-
return property;
55+
return property;
56+
}
57+
58+
static LogEventProperty CreateProperty(in EventKey eventKey)
59+
{
60+
var properties = new List<LogEventProperty>(2);
61+
62+
if (eventKey.Id != 0)
63+
{
64+
properties.Add(new LogEventProperty("Id", new ScalarValue(eventKey.Id)));
5465
}
5566

56-
static LogEventProperty CreateCore(in EventKey eventKey)
67+
if (eventKey.Name != null)
5768
{
58-
var properties = new List<LogEventProperty>(2);
69+
properties.Add(new LogEventProperty("Name", new ScalarValue(eventKey.Name)));
70+
}
5971

60-
if (eventKey.Id != 0)
61-
{
62-
properties.Add(new LogEventProperty("Id", new ScalarValue(eventKey.Id)));
63-
}
72+
return new LogEventProperty("EventId", new StructureValue(properties));
73+
}
6474

65-
if (eventKey.Name != null)
75+
LogEventProperty GetOrAddCore(in EventKey eventKey) =>
76+
_propertyCache.GetOrAdd(
77+
eventKey,
78+
key =>
6679
{
67-
properties.Add(new LogEventProperty("Name", new ScalarValue(eventKey.Name)));
68-
}
80+
Interlocked.Increment(ref _count);
6981

70-
return new LogEventProperty("EventId", new StructureValue(properties));
71-
}
82+
return CreateProperty(in key);
83+
});
7284

73-
readonly record struct EventKey
85+
readonly record struct EventKey
86+
{
87+
public EventKey(EventId eventId)
7488
{
75-
public EventKey(EventId eventId)
76-
{
77-
Id = eventId.Id;
78-
Name = eventId.Name;
79-
}
89+
Id = eventId.Id;
90+
Name = eventId.Name;
91+
}
8092

81-
public int Id { get; }
93+
public int Id { get; }
8294

83-
public string? Name { get; }
84-
}
95+
public string? Name { get; }
8596
}
8697
}

‎src/Serilog.Extensions.Logging/Extensions/Logging/SerilogLogger.cs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,9 @@ internal static string GetKeyWithoutFirstSymbol(ConcurrentDictionary<string, str
2828

2929
readonly SerilogLoggerProvider _provider;
3030
readonly ILogger _logger;
31+
readonly EventIdPropertyCache _eventIdPropertyCache = new();
3132

32-
static readonly CachingMessageTemplateParser MessageTemplateParser = new ();
33+
static readonly CachingMessageTemplateParser MessageTemplateParser = new();
3334

3435
public SerilogLogger(
3536
SerilogLoggerProvider provider,
@@ -150,7 +151,7 @@ LogEvent PrepareWrite<TState>(LogEventLevel level, EventId eventId, TState state
150151
}
151152

152153
if (eventId.Id != 0 || eventId.Name != null)
153-
properties.Add(EventIdPropertyCache.GetOrCreateProperty(in eventId));
154+
properties.Add(_eventIdPropertyCache.GetOrCreateProperty(in eventId));
154155

155156
var (traceId, spanId) = Activity.Current is { } activity ?
156157
(activity.TraceId, activity.SpanId) :

‎test/Serilog.Extensions.Logging.Tests/EventIdPropertyCacheTests.cs

Lines changed: 92 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -16,30 +16,99 @@
1616
using Serilog.Events;
1717
using Xunit;
1818

19-
namespace Serilog.Extensions.Logging.Tests
19+
namespace Serilog.Extensions.Logging.Tests;
20+
21+
public class EventIdPropertyCacheTests
2022
{
21-
public class EventIdPropertyCacheTests
23+
[Fact]
24+
public void CreatesPropertyWithCorrectIdAndName()
25+
{
26+
// Arrange
27+
const int id = 101;
28+
const string name = "TestEvent";
29+
var eventId = new EventId(id, name);
30+
31+
var cache = new EventIdPropertyCache();
32+
33+
// Act
34+
var eventProperty = cache.GetOrCreateProperty(eventId);
35+
36+
// Assert
37+
var value = Assert.IsType<StructureValue>(eventProperty.Value);
38+
39+
Assert.Equal(2, value.Properties.Count);
40+
41+
var idValue = value.Properties.Single(property => property.Name == "Id").Value;
42+
var nameValue = value.Properties.Single(property => property.Name == "Name").Value;
43+
44+
var scalarId = Assert.IsType<ScalarValue>(idValue);
45+
var scalarName = Assert.IsType<ScalarValue>(nameValue);
46+
47+
Assert.Equal(id, scalarId.Value);
48+
Assert.Equal(name, scalarName.Value);
49+
}
50+
51+
[Fact]
52+
public void EventsWithDSameKeysHaveSameReferences()
53+
{
54+
// Arrange
55+
var cache = new EventIdPropertyCache();
56+
57+
// Act
58+
var property1 = cache.GetOrCreateProperty(new EventId(1, "Name1"));
59+
var property2 = cache.GetOrCreateProperty(new EventId(1, "Name1"));
60+
61+
// Assert
62+
Assert.Same(property1, property2);
63+
}
64+
65+
[Theory]
66+
[InlineData(1, "SomeName", 1, "AnotherName")]
67+
[InlineData(1, "SomeName", 2, "SomeName")]
68+
[InlineData(1, "SomeName", 2, "AnotherName")]
69+
public void EventsWithDifferentKeysHaveDifferentReferences(int firstId, string firstName, int secondId, string secondName)
70+
{
71+
// Arrange
72+
var cache = new EventIdPropertyCache();
73+
74+
// Act
75+
var property1 = cache.GetOrCreateProperty(new EventId(firstId, firstName));
76+
var property2 = cache.GetOrCreateProperty(new EventId(secondId, secondName));
77+
78+
// Assert
79+
Assert.NotSame(property1, property2);
80+
}
81+
82+
83+
[Fact]
84+
public void WhenLimitIsNotOverSameEventsHaveSameReferences()
2285
{
23-
[Theory]
24-
[InlineData(1)]
25-
[InlineData(10)]
26-
[InlineData(48)]
27-
[InlineData(100)]
28-
public void LowAndHighNumberedEventIdsAreMapped(int id)
29-
{
30-
// Arrange
31-
var eventId = new EventId(id, "test");
32-
33-
// Act
34-
var mapped = EventIdPropertyCache.GetOrCreateProperty(eventId);
35-
36-
// Assert
37-
var value = Assert.IsType<StructureValue>(mapped.Value);
38-
Assert.Equal(2, value.Properties.Count);
39-
40-
var idValue = value.Properties.Single(p => p.Name == "Id").Value;
41-
var scalar = Assert.IsType<ScalarValue>(idValue);
42-
Assert.Equal(id, scalar.Value);
43-
}
86+
// Arrange
87+
var eventId = new EventId(101, "test");
88+
var cache = new EventIdPropertyCache();
89+
90+
// Act
91+
var property1 = cache.GetOrCreateProperty(eventId);
92+
var property2 = cache.GetOrCreateProperty(eventId);
93+
94+
// Assert
95+
Assert.Same(property1, property2);
96+
}
97+
98+
[Fact]
99+
public void WhenLimitIsOverSameEventsHaveDifferentReferences()
100+
{
101+
// Arrange
102+
var cache = new EventIdPropertyCache(maxCachedProperties: 1);
103+
cache.GetOrCreateProperty(new EventId(1, "InitialEvent"));
104+
105+
var eventId = new EventId(101, "DifferentEvent");
106+
107+
// Act
108+
var property1 = cache.GetOrCreateProperty(eventId);
109+
var property2 = cache.GetOrCreateProperty(eventId);
110+
111+
// Assert
112+
Assert.NotSame(property1, property2);
44113
}
45114
}

0 commit comments

Comments
 (0)