Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Commit c01a9a1

Browse files
sywhangsywhang
authored
[3.0 port] Fix first value of counter payload being skewed (#25799)
* Fix issue 25709 * rename * Fix regression test * cleanup * Code review feedback * set maxincrement to 3 * test fix
1 parent 7e89419 commit c01a9a1

File tree

6 files changed

+182
-5
lines changed

6 files changed

+182
-5
lines changed

‎src/System.Private.CoreLib/shared/System/Diagnostics/Tracing/CounterGroup.cs‎

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,7 @@ private void EnableTimer(float pollingIntervalInSeconds)
145145
Debug.WriteLine("Polling interval changed at " + DateTime.UtcNow.ToString("mm.ss.ffffff"));
146146
_pollingIntervalInMilliseconds = (int)(pollingIntervalInSeconds * 1000);
147147
DisposeTimer();
148+
ResetCounters(); // Reset statistics for counters before we start the thread.
148149
_timeStampSinceCollectionStarted = DateTime.UtcNow;
149150
// Don't capture the current ExecutionContext and its AsyncLocals onto the timer causing them to live forever
150151
bool restoreFlow = false;
@@ -165,8 +166,28 @@ private void EnableTimer(float pollingIntervalInSeconds)
165166
ExecutionContext.RestoreFlow();
166167
}
167168
}
168-
// Always fire the timer event (so you see everything up to this time).
169-
OnTimer(null);
169+
}
170+
171+
private void ResetCounters()
172+
{
173+
lock (this) // Lock the CounterGroup
174+
{
175+
foreach (var counter in _counters)
176+
{
177+
if (counter is IncrementingEventCounter ieCounter)
178+
{
179+
ieCounter.UpdateMetric();
180+
}
181+
else if (counter is IncrementingPollingCounter ipCounter)
182+
{
183+
ipCounter.UpdateMetric();
184+
}
185+
else if (counter is EventCounter eCounter)
186+
{
187+
eCounter.ResetStatistics();
188+
}
189+
}
190+
}
170191
}
171192

172193
private void OnTimer(object? state)

‎src/System.Private.CoreLib/shared/System/Diagnostics/Tracing/EventCounter.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ internal override void WritePayload(float intervalSec, int pollingIntervalMillis
114114
}
115115
}
116116

117-
private void ResetStatistics()
117+
internal void ResetStatistics()
118118
{
119119
Debug.Assert(Monitor.IsEntered(this));
120120
_count = 0;

‎src/System.Private.CoreLib/shared/System/Diagnostics/Tracing/IncrementingEventCounter.cs‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,15 @@ internal override void WritePayload(float intervalSec, int pollingIntervalMillis
7373
EventSource.Write("EventCounters", new EventSourceOptions() { Level = EventLevel.LogAlways }, new IncrementingEventCounterPayloadType(payload));
7474
}
7575
}
76+
77+
// Updates the value.
78+
internal void UpdateMetric()
79+
{
80+
lock (this)
81+
{
82+
_prevIncrement = _increment;
83+
}
84+
}
7685
}
7786

7887

‎src/System.Private.CoreLib/shared/System/Diagnostics/Tracing/IncrementingPollingCounter.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,12 +52,13 @@ public IncrementingPollingCounter(string name, EventSource eventSource, Func<dou
5252
/// <summary>
5353
/// Calls "_totalValueProvider" to enqueue the counter value to the queue.
5454
/// </summary>
55-
private void UpdateMetric()
55+
internal void UpdateMetric()
5656
{
5757
try
5858
{
5959
lock (this)
6060
{
61+
_prevIncrement = _increment;
6162
_increment = _totalValueProvider();
6263
}
6364
}
@@ -82,7 +83,6 @@ internal override void WritePayload(float intervalSec, int pollingIntervalMillis
8283
payload.Metadata = GetMetadataString();
8384
payload.Increment = _increment - _prevIncrement;
8485
payload.DisplayUnits = DisplayUnits ?? "";
85-
_prevIncrement = _increment;
8686
EventSource.Write("EventCounters", new EventSourceOptions() { Level = EventLevel.LogAlways }, new IncrementingPollingCounterPayloadType(payload));
8787
}
8888
}

‎tests/src/tracing/eventcounter/regression-25709.cs‎

Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,117 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
// See the LICENSE file in the project root for more information.
4+
5+
#if USE_MDT_EVENTSOURCE
6+
using Microsoft.Diagnostics.Tracing;
7+
#else
8+
using System.Diagnostics.Tracing;
9+
#endif
10+
using System;
11+
using System.Collections.Generic;
12+
using System.Threading;
13+
using System.Threading.Tasks;
14+
using System.Diagnostics;
15+
16+
namespace EventCounterRegressionTests
17+
{
18+
19+
public class SimpleEventListener : EventListener
20+
{
21+
private readonly EventLevel _level = EventLevel.Verbose;
22+
23+
public int MaxIncrement { get; private set; } = 0;
24+
25+
public SimpleEventListener()
26+
{
27+
}
28+
29+
30+
protected override void OnEventSourceCreated(EventSource source)
31+
{
32+
if (source.Name.Equals("System.Runtime"))
33+
{
34+
Dictionary<string, string> refreshInterval = new Dictionary<string, string>();
35+
refreshInterval.Add("EventCounterIntervalSec", "1");
36+
EnableEvents(source, _level, (EventKeywords)(-1), refreshInterval);
37+
}
38+
}
39+
40+
protected override void OnEventWritten(EventWrittenEventArgs eventData)
41+
{
42+
int increment = 0;
43+
bool isExceptionCounter = false;
44+
45+
for (int i = 0; i < eventData.Payload.Count; i++)
46+
{
47+
IDictionary<string, object> eventPayload = eventData.Payload[i] as IDictionary<string, object>;
48+
if (eventPayload != null)
49+
{
50+
foreach (KeyValuePair<string, object> payload in eventPayload)
51+
{
52+
if (payload.Key.Equals("Name") && payload.Value.ToString().Equals("exception-count"))
53+
isExceptionCounter = true;
54+
if (payload.Key.Equals("Increment"))
55+
{
56+
increment = Int32.Parse(payload.Value.ToString());
57+
}
58+
}
59+
if (isExceptionCounter)
60+
{
61+
if (MaxIncrement < increment)
62+
{
63+
MaxIncrement = increment;
64+
}
65+
}
66+
}
67+
}
68+
}
69+
}
70+
71+
public partial class TestEventCounter
72+
{
73+
74+
public static void ThrowExceptionTask()
75+
{
76+
// This will throw an exception every 1000 ms
77+
while (true)
78+
{
79+
Thread.Sleep(1000);
80+
try
81+
{
82+
Debug.WriteLine("Exception thrown at " + DateTime.UtcNow.ToString("mm.ss.ffffff"));
83+
throw new Exception("an exception");
84+
}
85+
catch
86+
{}
87+
}
88+
}
89+
90+
public static int Main(string[] args)
91+
{
92+
Task exceptionTask = Task.Run(ThrowExceptionTask);
93+
Thread.Sleep(5000);
94+
95+
// Create an EventListener.
96+
using (SimpleEventListener myListener = new SimpleEventListener())
97+
{
98+
Thread.Sleep(5000);
99+
100+
// The number below is supposed to be 2 at maximum, but in debug builds, the calls to
101+
// EventSource.Write() takes a lot longer than we thought(~1s), and the reflection in
102+
// workingset counter also adds a huge amount of time (> 1s), which makes the test fail in
103+
// debug CIs. Setting the check to 4 to compensate for these.
104+
if (myListener.MaxIncrement > 4)
105+
{
106+
Console.WriteLine($"Test Failed - Saw more than 3 exceptions / sec {myListener.MaxIncrement}");
107+
return 1;
108+
}
109+
else
110+
{
111+
Console.WriteLine("Test passed");
112+
return 100;
113+
}
114+
}
115+
}
116+
}
117+
}

‎tests/src/tracing/eventcounter/regression-25709.csproj‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
<?xml version="1.0" encoding="utf-8"?>
2+
<Project ToolsVersion="12.0" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
3+
<Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.props))\dir.props" />
4+
<PropertyGroup>
5+
<Configuration Condition=" '$(Configuration)' == '' ">Debug</Configuration>
6+
<Platform Condition=" '$(Platform)' == '' ">AnyCPU</Platform>
7+
<SchemaVersion>2.0</SchemaVersion>
8+
<ProjectGuid>{8E3244CB-407F-4142-BAAB-E7A55901A5FA}</ProjectGuid>
9+
<OutputType>Exe</OutputType>
10+
<ProjectTypeGuids>{786C830F-07A1-408B-BD7F-6EE04809D6DB};{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}</ProjectTypeGuids>
11+
<SolutionDir Condition="$(SolutionDir) == '' Or $(SolutionDir) == '*Undefined*'">..\..\</SolutionDir>
12+
<CLRTestKind>BuildAndRun</CLRTestKind>
13+
<DefineConstants>$(DefineConstants);STATIC</DefineConstants>
14+
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
15+
<CLRTestPriority>0</CLRTestPriority>
16+
<GCStressIncompatible>true</GCStressIncompatible>
17+
<!-- This test has a secondary thread with an infinite loop -->
18+
<UnloadabilityIncompatible>true</UnloadabilityIncompatible>
19+
</PropertyGroup>
20+
<!-- Default configurations to help VS understand the configurations -->
21+
<PropertyGroup Condition="'$(Configuration)|$(Platform)' == 'Debug|x64'">
22+
</PropertyGroup>
23+
<PropertyGroup Condition="'$(Configuration)|$(Platform)' == 'Release|x64'">
24+
</PropertyGroup>
25+
<ItemGroup>
26+
<Compile Include="regression-25709.cs" />
27+
<ProjectReference Include="../common/common.csproj" />
28+
</ItemGroup>
29+
<Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.targets))\dir.targets" />
30+
</Project>

0 commit comments

Comments
 (0)