Skip to content

Commit 51625a2

Browse files
pavelsavarapavelsavara
committed
feedback
1 parent b9e9970 commit 51625a2

File tree

7 files changed

+30
-109
lines changed

7 files changed

+30
-109
lines changed

‎src/Components/Components/src/RenderTree/Renderer.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,8 @@ public Renderer(IServiceProvider serviceProvider, ILoggerFactory loggerFactory,
9494

9595
// TODO register RenderingMetrics as singleton in DI
9696
var meterFactory = serviceProvider.GetService<IMeterFactory>();
97-
_renderingMetrics = meterFactory != null ? new RenderingMetrics(meterFactory) : null;
97+
Debug.Assert(meterFactory != null, "IMeterFactory should be registered in DI");
98+
_renderingMetrics = new RenderingMetrics(meterFactory);
9899

99100
ServiceProviderCascadingValueSuppliers = serviceProvider.GetService<ICascadingValueSupplier>() is null
100101
? Array.Empty<ICascadingValueSupplier>()

‎src/Components/Components/src/Rendering/RenderingMetrics.cs

Lines changed: 13 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ internal sealed class RenderingMetrics : IDisposable
1313

1414
private readonly Meter _meter;
1515
private readonly Counter<long> _renderTotalCounter;
16-
private readonly UpDownCounter<long> _renderActiveCounter;
1716
private readonly Histogram<double> _renderDuration;
1817

1918
public RenderingMetrics(IMeterFactory meterFactory)
@@ -25,82 +24,50 @@ public RenderingMetrics(IMeterFactory meterFactory)
2524
_renderTotalCounter = _meter.CreateCounter<long>(
2625
"aspnetcore.components.rendering.count",
2726
unit: "{renders}",
28-
description: "Number of component renders performed.");
29-
30-
_renderActiveCounter = _meter.CreateUpDownCounter<long>(
31-
"aspnetcore.components.rendering.active_renders",
32-
unit: "{renders}",
33-
description: "Number of component renders performed.");
27+
description: "Total number of component renders performed.");
3428

3529
_renderDuration = _meter.CreateHistogram<double>(
3630
"aspnetcore.components.rendering.duration",
37-
unit: "ms",
31+
unit: "s",
3832
description: "Duration of component rendering operations per component.",
3933
advice: new InstrumentAdvice<double> { HistogramBucketBoundaries = MetricsConstants.ShortSecondsBucketBoundaries });
4034
}
4135

4236
public void RenderStart(string componentType)
4337
{
44-
var tags = new TagList();
45-
tags = InitializeRequestTags(componentType, tags);
46-
47-
if (_renderActiveCounter.Enabled)
48-
{
49-
_renderActiveCounter.Add(1, tags);
50-
}
5138
if (_renderTotalCounter.Enabled)
5239
{
40+
var tags = new TagList
41+
{
42+
{ "component.type", componentType }
43+
};
5344
_renderTotalCounter.Add(1, tags);
5445
}
5546
}
5647

5748
public void RenderEnd(string componentType, Exception? exception, long startTimestamp, long currentTimestamp)
5849
{
59-
// Tags must match request start.
60-
var tags = new TagList();
61-
tags = InitializeRequestTags(componentType, tags);
62-
63-
if (_renderActiveCounter.Enabled)
64-
{
65-
_renderActiveCounter.Add(-1, tags);
66-
}
67-
6850
if (_renderDuration.Enabled)
6951
{
52+
var tags = new TagList
53+
{
54+
{ "component.type", componentType }
55+
};
56+
7057
if (exception != null)
7158
{
72-
TryAddTag(ref tags, "error.type", exception.GetType().FullName);
59+
tags.Add("error.type", exception.GetType().FullName);
7360
}
7461

7562
var duration = Stopwatch.GetElapsedTime(startTimestamp, currentTimestamp);
76-
_renderDuration.Record(duration.TotalMilliseconds, tags);
63+
_renderDuration.Record(duration.TotalSeconds, tags);
7764
}
7865
}
7966

80-
private static TagList InitializeRequestTags(string componentType, TagList tags)
81-
{
82-
tags.Add("component.type", componentType);
83-
return tags;
84-
}
85-
8667
public bool IsDurationEnabled() => _renderDuration.Enabled;
8768

8869
public void Dispose()
8970
{
9071
_meter.Dispose();
9172
}
92-
93-
private static bool TryAddTag(ref TagList tags, string name, object? value)
94-
{
95-
for (var i = 0; i < tags.Count; i++)
96-
{
97-
if (tags[i].Key == name)
98-
{
99-
return false;
100-
}
101-
}
102-
103-
tags.Add(new KeyValuePair<string, object?>(name, value));
104-
return true;
105-
}
10673
}

‎src/Components/Components/test/Rendering/RenderingMetricsTest.cs

Lines changed: 2 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,6 @@ public void RenderStart_IncreasesCounters()
3939
var renderingMetrics = new RenderingMetrics(_meterFactory);
4040
using var totalCounter = new MetricCollector<long>(_meterFactory,
4141
RenderingMetrics.MeterName, "aspnetcore.components.rendering.count");
42-
using var activeCounter = new MetricCollector<long>(_meterFactory,
43-
RenderingMetrics.MeterName, "aspnetcore.components.rendering.active_renders");
4442

4543
var componentType = "TestComponent";
4644

@@ -49,24 +47,17 @@ public void RenderStart_IncreasesCounters()
4947

5048
// Assert
5149
var totalMeasurements = totalCounter.GetMeasurementSnapshot();
52-
var activeMeasurements = activeCounter.GetMeasurementSnapshot();
5350

5451
Assert.Single(totalMeasurements);
5552
Assert.Equal(1, totalMeasurements[0].Value);
5653
Assert.Equal(componentType, totalMeasurements[0].Tags["component.type"]);
57-
58-
Assert.Single(activeMeasurements);
59-
Assert.Equal(1, activeMeasurements[0].Value);
60-
Assert.Equal(componentType, activeMeasurements[0].Tags["component.type"]);
6154
}
6255

6356
[Fact]
6457
public void RenderEnd_DecreasesActiveCounterAndRecordsDuration()
6558
{
6659
// Arrange
6760
var renderingMetrics = new RenderingMetrics(_meterFactory);
68-
using var activeCounter = new MetricCollector<long>(_meterFactory,
69-
RenderingMetrics.MeterName, "aspnetcore.components.rendering.active_renders");
7061
using var durationCollector = new MetricCollector<double>(_meterFactory,
7162
RenderingMetrics.MeterName, "aspnetcore.components.rendering.duration");
7263

@@ -79,13 +70,8 @@ public void RenderEnd_DecreasesActiveCounterAndRecordsDuration()
7970
renderingMetrics.RenderEnd(componentType, null, startTime, endTime);
8071

8172
// Assert
82-
var activeMeasurements = activeCounter.GetMeasurementSnapshot();
8373
var durationMeasurements = durationCollector.GetMeasurementSnapshot();
8474

85-
Assert.Single(activeMeasurements);
86-
Assert.Equal(-1, activeMeasurements[0].Value);
87-
Assert.Equal(componentType, activeMeasurements[0].Tags["component.type"]);
88-
8975
Assert.Single(durationMeasurements);
9076
Assert.True(durationMeasurements[0].Value > 0);
9177
Assert.Equal(componentType, durationMeasurements[0].Tags["component.type"]);
@@ -132,14 +118,12 @@ public void IsDurationEnabled_ReturnsMeterEnabledState()
132118
}
133119

134120
[Fact]
135-
public void FullRenderingLifecycle_RecordsAllMetricsCorrectly()
121+
public async Task FullRenderingLifecycle_RecordsAllMetricsCorrectly()
136122
{
137123
// Arrange
138124
var renderingMetrics = new RenderingMetrics(_meterFactory);
139125
using var totalCounter = new MetricCollector<long>(_meterFactory,
140126
RenderingMetrics.MeterName, "aspnetcore.components.rendering.count");
141-
using var activeCounter = new MetricCollector<long>(_meterFactory,
142-
RenderingMetrics.MeterName, "aspnetcore.components.rendering.active_renders");
143127
using var durationCollector = new MetricCollector<double>(_meterFactory,
144128
RenderingMetrics.MeterName, "aspnetcore.components.rendering.duration");
145129

@@ -152,27 +136,19 @@ public void FullRenderingLifecycle_RecordsAllMetricsCorrectly()
152136
renderingMetrics.RenderStart(componentType);
153137

154138
// 2. Component render ends
155-
Thread.Sleep(10); // Add a small delay to ensure a measurable duration
139+
await Task.Delay(10); // Add a small delay to ensure a measurable duration
156140
var endTime = Stopwatch.GetTimestamp();
157141
renderingMetrics.RenderEnd(componentType, null, startTime, endTime);
158142

159143
// Assert
160144
var totalMeasurements = totalCounter.GetMeasurementSnapshot();
161-
var activeMeasurements = activeCounter.GetMeasurementSnapshot();
162145
var durationMeasurements = durationCollector.GetMeasurementSnapshot();
163146

164147
// Total render count should have 1 measurement with value 1
165148
Assert.Single(totalMeasurements);
166149
Assert.Equal(1, totalMeasurements[0].Value);
167150
Assert.Equal(componentType, totalMeasurements[0].Tags["component.type"]);
168151

169-
// Active render count should have 2 measurements (1 for start, -1 for end)
170-
Assert.Equal(2, activeMeasurements.Count);
171-
Assert.Equal(1, activeMeasurements[0].Value);
172-
Assert.Equal(-1, activeMeasurements[1].Value);
173-
Assert.Equal(componentType, activeMeasurements[0].Tags["component.type"]);
174-
Assert.Equal(componentType, activeMeasurements[1].Tags["component.type"]);
175-
176152
// Duration should have 1 measurement with a positive value
177153
Assert.Single(durationMeasurements);
178154
Assert.True(durationMeasurements[0].Value > 0);
@@ -186,8 +162,6 @@ public void MultipleRenders_TracksMetricsIndependently()
186162
var renderingMetrics = new RenderingMetrics(_meterFactory);
187163
using var totalCounter = new MetricCollector<long>(_meterFactory,
188164
RenderingMetrics.MeterName, "aspnetcore.components.rendering.count");
189-
using var activeCounter = new MetricCollector<long>(_meterFactory,
190-
RenderingMetrics.MeterName, "aspnetcore.components.rendering.active_renders");
191165
using var durationCollector = new MetricCollector<double>(_meterFactory,
192166
RenderingMetrics.MeterName, "aspnetcore.components.rendering.duration");
193167

@@ -215,21 +189,13 @@ public void MultipleRenders_TracksMetricsIndependently()
215189

216190
// Assert
217191
var totalMeasurements = totalCounter.GetMeasurementSnapshot();
218-
var activeMeasurements = activeCounter.GetMeasurementSnapshot();
219192
var durationMeasurements = durationCollector.GetMeasurementSnapshot();
220193

221194
// Should have 2 total render counts (one for each component)
222195
Assert.Equal(2, totalMeasurements.Count);
223196
Assert.Contains(totalMeasurements, m => m.Value == 1 && m.Tags["component.type"] as string == componentType1);
224197
Assert.Contains(totalMeasurements, m => m.Value == 1 && m.Tags["component.type"] as string == componentType2);
225198

226-
// Should have 4 active render counts (start and end for each component)
227-
Assert.Equal(4, activeMeasurements.Count);
228-
Assert.Contains(activeMeasurements, m => m.Value == 1 && m.Tags["component.type"] as string == componentType1);
229-
Assert.Contains(activeMeasurements, m => m.Value == 1 && m.Tags["component.type"] as string == componentType2);
230-
Assert.Contains(activeMeasurements, m => m.Value == -1 && m.Tags["component.type"] as string == componentType1);
231-
Assert.Contains(activeMeasurements, m => m.Value == -1 && m.Tags["component.type"] as string == componentType2);
232-
233199
// Should have 2 duration measurements (one for each component)
234200
Assert.Equal(2, durationMeasurements.Count);
235201
Assert.Contains(durationMeasurements, m => m.Value > 0 && m.Tags["component.type"] as string == componentType1);

‎src/Components/Server/src/Circuits/CircuitMetrics.cs

Lines changed: 8 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ public CircuitMetrics(IMeterFactory meterFactory)
2626
_circuitTotalCounter = _meter.CreateCounter<long>(
2727
"aspnetcore.components.circuits.count",
2828
unit: "{circuits}",
29-
description: "Number of active circuits.");
29+
description: "Total number of circuits.");
3030

3131
_circuitActiveCounter = _meter.CreateUpDownCounter<long>(
3232
"aspnetcore.components.circuits.active_circuits",
@@ -47,57 +47,48 @@ public CircuitMetrics(IMeterFactory meterFactory)
4747

4848
public void OnCircuitOpened()
4949
{
50-
var tags = new TagList();
51-
5250
if (_circuitActiveCounter.Enabled)
5351
{
54-
_circuitActiveCounter.Add(1, tags);
52+
_circuitActiveCounter.Add(1);
5553
}
5654
if (_circuitTotalCounter.Enabled)
5755
{
58-
_circuitTotalCounter.Add(1, tags);
56+
_circuitTotalCounter.Add(1);
5957
}
6058
}
6159

6260
public void OnConnectionUp()
6361
{
64-
var tags = new TagList();
65-
6662
if (_circuitConnectedCounter.Enabled)
6763
{
68-
_circuitConnectedCounter.Add(1, tags);
64+
_circuitConnectedCounter.Add(1);
6965
}
7066
}
7167

7268
public void OnConnectionDown()
7369
{
74-
var tags = new TagList();
75-
7670
if (_circuitConnectedCounter.Enabled)
7771
{
78-
_circuitConnectedCounter.Add(-1, tags);
72+
_circuitConnectedCounter.Add(-1);
7973
}
8074
}
8175

8276
public void OnCircuitDown(long startTimestamp, long currentTimestamp)
8377
{
84-
// Tags must match request start.
85-
var tags = new TagList();
86-
8778
if (_circuitActiveCounter.Enabled)
8879
{
89-
_circuitActiveCounter.Add(-1, tags);
80+
_circuitActiveCounter.Add(-1);
9081
}
9182

9283
if (_circuitConnectedCounter.Enabled)
9384
{
94-
_circuitConnectedCounter.Add(-1, tags);
85+
_circuitConnectedCounter.Add(-1);
9586
}
9687

9788
if (_circuitDuration.Enabled)
9889
{
9990
var duration = Stopwatch.GetElapsedTime(startTimestamp, currentTimestamp);
100-
_circuitDuration.Record(duration.TotalSeconds, tags);
91+
_circuitDuration.Record(duration.TotalSeconds);
10192
}
10293
}
10394

‎src/Components/Server/src/DependencyInjection/ComponentServiceCollectionExtensions.cs

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -62,11 +62,7 @@ public static IServerSideBlazorBuilder AddServerSideBlazor(this IServiceCollecti
6262
// user's configuration. So even if the user has multiple independent server-side
6363
// Components entrypoints, this lot is the same and repeated registrations are a no-op.
6464

65-
services.TryAddSingleton(s =>
66-
{
67-
var meterFactory = s.GetService<IMeterFactory>();
68-
return meterFactory != null ? new CircuitMetrics(meterFactory) : null;
69-
});
65+
services.TryAddSingleton<CircuitMetrics>();
7066
services.TryAddSingleton<ICircuitFactory, CircuitFactory>();
7167
services.TryAddSingleton<ICircuitHandleRegistry, CircuitHandleRegistry>();
7268
services.TryAddSingleton<RootTypeCache>();

‎src/Components/Server/test/Circuits/CircuitMetricsTest.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ public void OnConnectionDown_DecreasesConnectedCounter()
9595
}
9696

9797
[Fact]
98-
public void OnCircuitDown_UpdatesCountersAndRecordsDuration()
98+
public async Task OnCircuitDown_UpdatesCountersAndRecordsDuration()
9999
{
100100
// Arrange
101101
var circuitMetrics = new CircuitMetrics(_meterFactory);
@@ -108,7 +108,7 @@ public void OnCircuitDown_UpdatesCountersAndRecordsDuration()
108108

109109
// Act
110110
var startTime = Stopwatch.GetTimestamp();
111-
Thread.Sleep(10); // Add a small delay to ensure a measurable duration
111+
await Task.Delay(10); // Add a small delay to ensure a measurable duration
112112
var endTime = Stopwatch.GetTimestamp();
113113
circuitMetrics.OnCircuitDown(startTime, endTime);
114114

‎src/Shared/Metrics/MetricsConstants.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,6 @@ internal static class MetricsConstants
1111
// Not based on a standard. Larger bucket sizes for longer lasting operations, e.g. HTTP connection duration. See https://github.com/open-telemetry/semantic-conventions/issues/336
1212
public static readonly IReadOnlyList<double> LongSecondsBucketBoundaries = [0.01, 0.02, 0.05, 0.1, 0.2, 0.5, 1, 2, 5, 10, 30, 60, 120, 300];
1313

14-
// For Blazor/signalR sessions, which can last a long time.
15-
public static readonly IReadOnlyList<double> VeryLongSecondsBucketBoundaries = [0.5, 1, 2, 5, 10, 30, 60, 120, 300, 600, 1500, 60*60, 2 * 60 * 60, 4 * 60 * 60];
14+
// For blazor sessions, which can last a long time.
15+
public static readonly IReadOnlyList<double> VeryLongSecondsBucketBoundaries = [1, 10, 30, 1 * 60, 2 * 60, 3 * 60, 4 * 60, 5 * 60, 6 * 60, 7 * 60, 8 * 60, 9 * 60, 10 * 60, 1 * 60 * 60, 2 * 60 * 60, 3 * 60 * 60, 6 * 60 * 60, 12 * 60 * 60, 24 * 60 * 60];
1616
}

0 commit comments

Comments
 (0)