Skip to content

Commit 2218335

Browse files
pavelsavarapavelsavara
authored
[browser] managed Diagnostics.Metrics and Diagnostics.Tracing single-threaded (#113524)
1 parent 04aa3ef commit 2218335

File tree

26 files changed

+542
-332
lines changed

26 files changed

+542
-332
lines changed

‎src/libraries/Common/tests/TestUtilities/System/PlatformDetection.cs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,7 @@ private static bool ComputeIsLinqSizeOptimized()
163163
public static bool IsFirefox => IsEnvironmentVariableTrue("IsFirefox");
164164
public static bool IsChromium => IsEnvironmentVariableTrue("IsChromium");
165165
public static bool IsNotNodeJS => !IsNodeJS;
166+
public static bool IsNotNodeJSOrFirefox => !IsNodeJS && !IsFirefox;
166167
public static bool IsNodeJSOnWindows => GetNodeJSPlatform() == "win32";
167168
public static bool LocalEchoServerIsNotAvailable => !LocalEchoServerIsAvailable;
168169
public static bool LocalEchoServerIsAvailable => IsBrowser;

‎src/libraries/Microsoft.Extensions.Diagnostics/tests/DefaultMetricsFactoryTests.cs‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,6 @@ public void NegativeTest()
104104
}
105105

106106
[Fact]
107-
[ActiveIssue("https://github.com/dotnet/runtime/issues/93754", TestPlatforms.Browser)]
108107
public void MeterDisposeTest()
109108
{
110109
ServiceCollection services = new ServiceCollection();

‎src/libraries/Microsoft.Extensions.Diagnostics/tests/MetricsSubscriptionManagerTests.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ private class FakeListener : IMetricsListener
4242
public void MeasurementsCompleted(Instrument instrument, object? userState) => throw new NotImplementedException();
4343
}
4444

45-
[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotBrowser))]
45+
[Fact]
4646
public void TestSubscriptionManagerDisposal()
4747
{
4848
var meter = new Meter("TestMeter");

‎src/libraries/Microsoft.Extensions.Diagnostics/tests/Microsoft.Extensions.Diagnostics.Tests.csproj‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,16 @@
11
<Project Sdk="Microsoft.NET.Sdk">
22

33
<PropertyGroup>
4-
<TargetFrameworks>$(NetCoreAppCurrent);$(NetFrameworkCurrent)</TargetFrameworks>
4+
<TargetFrameworks>$(NetCoreAppCurrent);$(NetCoreAppCurrent)-browser;$(NetFrameworkCurrent)</TargetFrameworks>
55
<EnableDefaultItems>true</EnableDefaultItems>
66
<IncludeRemoteExecutor>true</IncludeRemoteExecutor>
77
</PropertyGroup>
8+
<PropertyGroup Condition="'$(TargetOS)' == 'browser'">
9+
<!-- Enable diagnostic features. They will add appropriate RuntimeHostConfigurationOption values to runtime config and ILLink.
10+
https://github.com/dotnet/docs/blob/main/docs/core/deploying/trimming/trimming-options.md#trim-framework-library-features
11+
-->
12+
<MetricsSupport>true</MetricsSupport>
13+
</PropertyGroup>
814

915
<ItemGroup>
1016
<Compile Include="..\src\Metrics\Configuration\MetricsConfigureOptions.cs" />

‎src/libraries/System.Diagnostics.DiagnosticSource/src/System.Diagnostics.DiagnosticSource.csproj‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ System.Diagnostics.DiagnosticSource</PackageDescription>
1717
<!-- DesignTimeBuild requires all the TargetFramework Derived Properties to not be present in the first property group. -->
1818
<PropertyGroup>
1919
<DefineConstants Condition="$([MSBuild]::GetTargetFrameworkIdentifier('$(TargetFramework)')) == '.NETFramework'">$(DefineConstants);ENABLE_HTTP_HANDLER</DefineConstants>
20-
<DefineConstants Condition="$([MSBuild]::GetTargetFrameworkIdentifier('$(TargetFramework)')) == '.NETCoreApp'">$(DefineConstants);W3C_DEFAULT_ID_FORMAT;MEMORYMARSHAL_SUPPORT;OS_ISBROWSER_SUPPORT</DefineConstants>
20+
<DefineConstants Condition="$([MSBuild]::GetTargetFrameworkIdentifier('$(TargetFramework)')) == '.NETCoreApp'">$(DefineConstants);W3C_DEFAULT_ID_FORMAT;MEMORYMARSHAL_SUPPORT;OS_ISWASI_SUPPORT;OS_ISBROWSER_SUPPORT</DefineConstants>
2121
<IncludePlatformAttributes>true</IncludePlatformAttributes>
2222
<!-- TODO: Add package README file: https://github.com/dotnet/runtime/issues/99358 -->
2323
<EnableDefaultPackageReadmeFile>false</EnableDefaultPackageReadmeFile>

‎src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/AggregationManager.cs‎

Lines changed: 102 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111

1212
namespace System.Diagnostics.Metrics
1313
{
14-
[UnsupportedOSPlatform("browser")]
1514
[SecuritySafeCritical]
1615
internal sealed class AggregationManager
1716
{
@@ -29,6 +28,9 @@ internal sealed class AggregationManager
2928
private readonly ConcurrentDictionary<Instrument, InstrumentState> _instrumentStates = new();
3029
private readonly CancellationTokenSource _cts = new();
3130
private Thread? _collectThread;
31+
#if OS_ISBROWSER_SUPPORT
32+
private Timer? _pollingTimer;
33+
#endif
3234
private readonly MeterListener _listener;
3335
private int _currentTimeSeries;
3436
private int _currentHistograms;
@@ -43,6 +45,9 @@ internal sealed class AggregationManager
4345
private readonly Action _timeSeriesLimitReached;
4446
private readonly Action _histogramLimitReached;
4547
private readonly Action<Exception> _observableInstrumentCallbackError;
48+
private DateTime _startTime;
49+
private DateTime _intervalStartTime;
50+
private DateTime _nextIntervalStartTime;
4651

4752
public AggregationManager(
4853
int maxTimeSeries,
@@ -160,15 +165,28 @@ public void Start()
160165
Debug.Assert(_collectThread == null && !_cts.IsCancellationRequested);
161166
Debug.Assert(CollectionPeriod.TotalSeconds >= MinCollectionTimeSecs);
162167

163-
// This explicitly uses a Thread and not a Task so that metrics still work
164-
// even when an app is experiencing thread-pool starvation. Although we
165-
// can't make in-proc metrics robust to everything, this is a common enough
166-
// problem in .NET apps that it feels worthwhile to take the precaution.
167-
_collectThread = new Thread(() => CollectWorker(_cts.Token));
168-
_collectThread.IsBackground = true;
169-
_collectThread.Name = "MetricsEventSource CollectWorker";
170-
_collectThread.Start();
168+
_intervalStartTime = _nextIntervalStartTime = _startTime = DateTime.UtcNow;
169+
#if OS_ISBROWSER_SUPPORT
170+
if (OperatingSystem.IsBrowser())
171+
{
172+
TimeSpan delayTime = CalculateDelayTime(CollectionPeriod.TotalSeconds);
173+
_pollingTimer = new Timer(CollectOnTimer, null, (int)delayTime.TotalMilliseconds, 0);
174+
}
175+
else
176+
#endif
177+
{
178+
// This explicitly uses a Thread and not a Task so that metrics still work
179+
// even when an app is experiencing thread-pool starvation. Although we
180+
// can't make in-proc metrics robust to everything, this is a common enough
181+
// problem in .NET apps that it feels worthwhile to take the precaution.
182+
_collectThread = new Thread(CollectWorker);
183+
_collectThread.IsBackground = true;
184+
_collectThread.Name = "MetricsEventSource CollectWorker";
185+
#pragma warning disable CA1416 // 'Thread.Start' is unsupported on: 'browser', there the actual implementation is in AggregationManager.Wasm.cs
186+
_collectThread.Start();
187+
#pragma warning restore CA1416
171188

189+
}
172190
_listener.Start();
173191
_initialInstrumentEnumerationComplete();
174192
}
@@ -187,57 +205,64 @@ public void Update()
187205
_initialInstrumentEnumerationComplete();
188206
}
189207

190-
private void CollectWorker(CancellationToken cancelToken)
208+
private TimeSpan CalculateDelayTime(double collectionIntervalSecs)
209+
{
210+
_intervalStartTime = _nextIntervalStartTime;
211+
212+
// intervals end at startTime + X*collectionIntervalSecs. Under normal
213+
// circumstance X increases by 1 each interval, but if the time it
214+
// takes to do collection is very large then we might need to skip
215+
// ahead multiple intervals to catch back up.
216+
//
217+
DateTime now = DateTime.UtcNow;
218+
double secsSinceStart = (now - _startTime).TotalSeconds;
219+
double alignUpSecsSinceStart = Math.Ceiling(secsSinceStart / collectionIntervalSecs) *
220+
collectionIntervalSecs;
221+
_nextIntervalStartTime = _startTime.AddSeconds(alignUpSecsSinceStart);
222+
223+
// The delay timer precision isn't exact. We might have a situation
224+
// where in the previous loop iterations intervalStartTime=20.00,
225+
// nextIntervalStartTime=21.00, the timer was supposed to delay for 1s but
226+
// it exited early so we looped around and DateTime.Now=20.99.
227+
// Aligning up from DateTime.Now would give us 21.00 again so we also need to skip
228+
// forward one time interval
229+
DateTime minNextInterval = _intervalStartTime.AddSeconds(collectionIntervalSecs);
230+
if (_nextIntervalStartTime <= minNextInterval)
231+
{
232+
_nextIntervalStartTime = minNextInterval;
233+
}
234+
return _nextIntervalStartTime - now;
235+
}
236+
237+
private void CollectWorker()
191238
{
192239
try
193240
{
194241
double collectionIntervalSecs = -1;
242+
CancellationToken cancelToken;
195243
lock (this)
196244
{
197245
collectionIntervalSecs = CollectionPeriod.TotalSeconds;
246+
cancelToken = _cts.Token;
198247
}
199248
Debug.Assert(collectionIntervalSecs >= MinCollectionTimeSecs);
200249

201250
DateTime startTime = DateTime.UtcNow;
202251
DateTime intervalStartTime = startTime;
203-
while (!cancelToken.IsCancellationRequested)
252+
while (!_cts.Token.IsCancellationRequested)
204253
{
205-
// intervals end at startTime + X*collectionIntervalSecs. Under normal
206-
// circumstance X increases by 1 each interval, but if the time it
207-
// takes to do collection is very large then we might need to skip
208-
// ahead multiple intervals to catch back up.
209-
//
210-
DateTime now = DateTime.UtcNow;
211-
double secsSinceStart = (now - startTime).TotalSeconds;
212-
double alignUpSecsSinceStart = Math.Ceiling(secsSinceStart / collectionIntervalSecs) *
213-
collectionIntervalSecs;
214-
DateTime nextIntervalStartTime = startTime.AddSeconds(alignUpSecsSinceStart);
215-
216-
// The delay timer precision isn't exact. We might have a situation
217-
// where in the previous loop iterations intervalStartTime=20.00,
218-
// nextIntervalStartTime=21.00, the timer was supposed to delay for 1s but
219-
// it exited early so we looped around and DateTime.Now=20.99.
220-
// Aligning up from DateTime.Now would give us 21.00 again so we also need to skip
221-
// forward one time interval
222-
DateTime minNextInterval = intervalStartTime.AddSeconds(collectionIntervalSecs);
223-
if (nextIntervalStartTime <= minNextInterval)
224-
{
225-
nextIntervalStartTime = minNextInterval;
226-
}
227-
228254
// pause until the interval is complete
229-
TimeSpan delayTime = nextIntervalStartTime - now;
255+
TimeSpan delayTime = CalculateDelayTime(collectionIntervalSecs);
230256
if (cancelToken.WaitHandle.WaitOne(delayTime))
231257
{
232258
// don't do collection if timer may not have run to completion
233259
break;
234260
}
235261

236262
// collect statistics for the completed interval
237-
_beginCollection(intervalStartTime, nextIntervalStartTime);
263+
_beginCollection(_intervalStartTime, _nextIntervalStartTime);
238264
Collect();
239-
_endCollection(intervalStartTime, nextIntervalStartTime);
240-
intervalStartTime = nextIntervalStartTime;
265+
_endCollection(_intervalStartTime, _nextIntervalStartTime);
241266
}
242267
}
243268
catch (Exception e)
@@ -246,12 +271,49 @@ private void CollectWorker(CancellationToken cancelToken)
246271
}
247272
}
248273

274+
#if OS_ISBROWSER_SUPPORT
275+
private void CollectOnTimer(object? _)
276+
{
277+
try
278+
{
279+
// this is single-threaded so we don't need to lock
280+
CancellationToken cancelToken = _cts.Token;
281+
double collectionIntervalSecs = CollectionPeriod.TotalSeconds;
282+
283+
if (cancelToken.IsCancellationRequested)
284+
{
285+
return;
286+
}
287+
288+
// collect statistics for the completed interval
289+
_beginCollection(_intervalStartTime, _nextIntervalStartTime);
290+
Collect();
291+
_endCollection(_intervalStartTime, _nextIntervalStartTime);
292+
293+
TimeSpan delayTime = CalculateDelayTime(collectionIntervalSecs);
294+
// schedule the next collection
295+
_pollingTimer!.Change((int)delayTime.TotalMilliseconds, 0);
296+
}
297+
catch (Exception e)
298+
{
299+
_collectionError(e);
300+
}
301+
}
302+
#endif
303+
249304
public void Dispose()
250305
{
251306
_cts.Cancel();
252-
if (_collectThread != null)
307+
#if OS_ISBROWSER_SUPPORT
308+
if (OperatingSystem.IsBrowser())
309+
{
310+
_pollingTimer?.Dispose();
311+
_pollingTimer = null;
312+
}
313+
else
314+
#endif
253315
{
254-
_collectThread.Join();
316+
_collectThread?.Join();
255317
_collectThread = null;
256318
}
257319
_listener.Dispose();

‎src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/MetricsEventSource.cs‎

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,7 @@ public void BeginInstrumentReporting(
173173
string? meterTelemetrySchemaUrl)
174174
{
175175
WriteEvent(7, sessionId, meterName, meterVersion ?? "", instrumentName, instrumentType, unit ?? "", description ?? "",
176-
instrumentTags, meterTags, meterScopeHash, instrumentId, meterTelemetrySchemaUrl);
176+
instrumentTags, meterTags, meterScopeHash, instrumentId, meterTelemetrySchemaUrl ?? "");
177177
}
178178

179179
// Sent when we stop monitoring the value of a instrument, either because new session filter arguments changed subscriptions
@@ -198,7 +198,7 @@ public void EndInstrumentReporting(
198198
string? meterTelemetrySchemaUrl)
199199
{
200200
WriteEvent(8, sessionId, meterName, meterVersion ?? "", instrumentName, instrumentType, unit ?? "", description ?? "",
201-
instrumentTags, meterTags, meterScopeHash, instrumentId, meterTelemetrySchemaUrl);
201+
instrumentTags, meterTags, meterScopeHash, instrumentId, meterTelemetrySchemaUrl ?? "");
202202
}
203203

204204
[Event(9, Keywords = Keywords.TimeSeriesValues | Keywords.Messages | Keywords.InstrumentPublishing)]
@@ -233,7 +233,7 @@ public void InstrumentPublished(
233233
string? meterTelemetrySchemaUrl)
234234
{
235235
WriteEvent(11, sessionId, meterName, meterVersion ?? "", instrumentName, instrumentType, unit ?? "", description ?? "",
236-
instrumentTags, meterTags, meterScopeHash, instrumentId, meterTelemetrySchemaUrl);
236+
instrumentTags, meterTags, meterScopeHash, instrumentId, meterTelemetrySchemaUrl ?? "");
237237
}
238238

239239
[Event(12, Keywords = Keywords.TimeSeriesValues)]
@@ -338,16 +338,16 @@ public void OnEventCommand(EventCommandEventArgs command)
338338
{
339339
try
340340
{
341-
#if OS_ISBROWSER_SUPPORT
342-
if (OperatingSystem.IsBrowser() || OperatingSystem.IsWasi())
341+
#if OS_ISWASI_SUPPORT
342+
if (OperatingSystem.IsWasi())
343343
{
344344
// AggregationManager uses a dedicated thread to avoid losing data for apps experiencing threadpool starvation
345-
// and browser doesn't support Thread.Start()
345+
// and wasi doesn't support Thread.Start()
346346
//
347-
// This limitation shouldn't really matter because browser also doesn't support out-of-proc EventSource communication
347+
// This limitation shouldn't really matter because wasi also doesn't support out-of-proc EventSource communication
348348
// which is the intended scenario for this EventSource. If it matters in the future AggregationManager can be
349-
// modified to have some other fallback path that works for browser.
350-
Parent.Error("", "System.Diagnostics.Metrics EventSource not supported on browser and wasi");
349+
// modified to have some other fallback path that works for wasi.
350+
Parent.Error("", "System.Diagnostics.Metrics EventSource not supported on wasi");
351351
return;
352352
}
353353
#endif
@@ -661,7 +661,6 @@ private bool LogError(Exception e)
661661

662662
private static readonly char[] s_instrumentSeparators = new char[] { '\r', '\n', ',', ';' };
663663

664-
[UnsupportedOSPlatform("browser")]
665664
private void ParseSpecs(string? metricsSpecs)
666665
{
667666
if (metricsSpecs == null)

‎src/libraries/System.Diagnostics.DiagnosticSource/tests/ActivityTests.cs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1416,7 +1416,7 @@ public void DiagnosticSourceStartStop()
14161416
/// <summary>
14171417
/// Tests that Activity.Current flows correctly within async methods
14181418
/// </summary>
1419-
[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsThreadingSupported))]
1419+
[Fact]
14201420
public async Task ActivityCurrentFlowsWithAsyncSimple()
14211421
{
14221422
Activity activity = new Activity("activity").Start();
@@ -1433,7 +1433,7 @@ await Task.Run(() =>
14331433
/// <summary>
14341434
/// Tests that Activity.Current flows correctly within async methods
14351435
/// </summary>
1436-
[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsThreadingSupported))]
1436+
[Fact]
14371437
public async Task ActivityCurrentFlowsWithAsyncComplex()
14381438
{
14391439
Activity originalActivity = Activity.Current;
@@ -1472,7 +1472,7 @@ public async Task ActivityCurrentFlowsWithAsyncComplex()
14721472
/// <summary>
14731473
/// Tests that Activity.Current could be set
14741474
/// </summary>
1475-
[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsThreadingSupported))]
1475+
[Fact]
14761476
public async Task ActivityCurrentSet()
14771477
{
14781478
Activity activity = new Activity("activity");

0 commit comments

Comments
 (0)