Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -907,7 +907,7 @@
</ItemGroup>
<ItemGroup>
<Compile Include="$(MSBuildThisFileDirectory)System\Diagnostics\Tracing\ActivityTracker.cs" />
<Compile Include="$(MSBuildThisFileDirectory)System\Diagnostics\Tracing\BaseCounter.cs" />
<Compile Include="$(MSBuildThisFileDirectory)System\Diagnostics\Tracing\DiagnosticCounter.cs" />
<Compile Include="$(MSBuildThisFileDirectory)System\Diagnostics\Tracing\CounterGroup.cs" />
<Compile Include="$(MSBuildThisFileDirectory)System\Diagnostics\Tracing\CounterPayload.cs" />
<Compile Include="$(MSBuildThisFileDirectory)System\Diagnostics\Tracing\EventActivityOptions.cs" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,22 +20,22 @@ namespace System.Diagnostics.Tracing
internal class CounterGroup
{
private readonly EventSource _eventSource;
private readonly List<BaseCounter> _counters;
private readonly List<DiagnosticCounter> _counters;

internal CounterGroup(EventSource eventSource)
{
_eventSource = eventSource;
_counters = new List<BaseCounter>();
_counters = new List<DiagnosticCounter>();
RegisterCommandCallback();
}

internal void Add(BaseCounter eventCounter)
internal void Add(DiagnosticCounter eventCounter)
{
lock (this) // Lock the CounterGroup
_counters.Add(eventCounter);
}

internal void Remove(BaseCounter eventCounter)
internal void Remove(DiagnosticCounter eventCounter)
{
lock (this) // Lock the CounterGroup
_counters.Remove(eventCounter);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,72 +17,26 @@ namespace Microsoft.Diagnostics.Tracing
namespace System.Diagnostics.Tracing
#endif
{
// TODO: This should be removed as we make the new payloads public
[EventData]
internal class EventCounterPayload : IEnumerable<KeyValuePair<string, object>>
{
public string Name { get; set; }

public float Mean { get; set; }

public float StandardDeviation { get; set; }

public int Count { get; set; }

public float Min { get; set; }

public float Max { get; set; }

public float IntervalSec { get; internal set; }

#region Implementation of the IEnumerable interface

public IEnumerator<KeyValuePair<string, object>> GetEnumerator()
{
return ForEnumeration.GetEnumerator();
}

IEnumerator IEnumerable.GetEnumerator()
{
return ForEnumeration.GetEnumerator();
}

private IEnumerable<KeyValuePair<string, object>> ForEnumeration
{
get
{
yield return new KeyValuePair<string, object>("Name", Name);
yield return new KeyValuePair<string, object>("Mean", Mean);
yield return new KeyValuePair<string, object>("StandardDeviation", StandardDeviation);
yield return new KeyValuePair<string, object>("Count", Count);
yield return new KeyValuePair<string, object>("Min", Min);
yield return new KeyValuePair<string, object>("Max", Max);
}
}

#endregion // Implementation of the IEnumerable interface
}

[EventData]
internal class CounterPayload : IEnumerable<KeyValuePair<string, object>>
{
public string Name { get; set; }

public string DisplayName { get; set; }

public float Mean { get; set; }
public double Mean { get; set; }

public float StandardDeviation { get; set; }
public double StandardDeviation { get; set; }

public int Count { get; set; }

public float Min { get; set; }
public double Min { get; set; }

public float Max { get; set; }
public double Max { get; set; }

public float IntervalSec { get; internal set; }

public string MetaData { get; set; }
public string Metadata { get; set; }

#region Implementation of the IEnumerable interface

Expand Down Expand Up @@ -110,7 +64,7 @@ private IEnumerable<KeyValuePair<string, object>> ForEnumeration
yield return new KeyValuePair<string, object>("IntervalSec", IntervalSec);
yield return new KeyValuePair<string, object>("Series", $"Interval={IntervalSec}");
yield return new KeyValuePair<string, object>("CounterType", "Mean");
yield return new KeyValuePair<string, object>("MetaData", MetaData);
yield return new KeyValuePair<string, object>("Metadata", Metadata);
}
}

Expand All @@ -126,11 +80,11 @@ internal class IncrementingCounterPayload : IEnumerable<KeyValuePair<string, obj

public string DisplayRateTimeScale { get; set; }

public float Increment { get; set; }
public double Increment { get; set; }

public float IntervalSec { get; internal set; }

public string MetaData { get; set; }
public string Metadata { get; set; }

#region Implementation of the IEnumerable interface

Expand All @@ -155,7 +109,7 @@ private IEnumerable<KeyValuePair<string, object>> ForEnumeration
yield return new KeyValuePair<string, object>("IntervalSec", IntervalSec);
yield return new KeyValuePair<string, object>("Series", $"Interval={IntervalSec}");
yield return new KeyValuePair<string, object>("CounterType", "Sum");
yield return new KeyValuePair<string, object>("MetaData", MetaData);
yield return new KeyValuePair<string, object>("Metadata", Metadata);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,34 +19,33 @@ namespace System.Diagnostics.Tracing
#endif
{
/// <summary>
/// BaseCounter is an abstract class that serves as the parent class for various Counter* classes,
/// DiagnosticCounter is an abstract class that serves as the parent class for various Counter* classes,
/// namely EventCounter, PollingCounter, IncrementingEventCounter, and IncrementingPollingCounter.
/// </summary>
public abstract class BaseCounter : IDisposable
public abstract class DiagnosticCounter : IDisposable
{
/// <summary>
/// All Counters live as long as the EventSource that they are attached to unless they are
/// explicitly Disposed.
/// </summary>
/// <param name="name">The name.</param>
/// <param name="eventSource">The event source.</param>
public BaseCounter(string name, EventSource eventSource)
public DiagnosticCounter(string name, EventSource eventSource)
{
if (name == null)
{
throw new ArgumentNullException(nameof(_name));
throw new ArgumentNullException(nameof(Name));
}

if (eventSource == null)
{
throw new ArgumentNullException(nameof(eventSource));
throw new ArgumentNullException(nameof(EventSource));
}

_group = CounterGroup.GetCounterGroup(eventSource);
_group.Add(this);
_eventSource = eventSource;
_name = name;
_metaData = new Dictionary<string, string>();
Name = name;
EventSource = eventSource;
}

/// <summary>
Expand All @@ -67,38 +66,45 @@ public void Dispose()
/// <summary>
/// Adds a key-value metadata to the EventCounter that will be included as a part of the payload
/// </summary>
internal void AddMetaData(string key, string value)
public void AddMetadata(string key, string value)
{
lock (MyLock)
{
_metaData.Add(key, value);
_metadata = _metadata ?? new Dictionary<string, string>();
_metadata.Add(key, value);
}
}

internal string DisplayName { get; set; }
public string DisplayName { get; set; }

#region private implementation
public string Name { get; }

public EventSource EventSource { get; }

internal readonly string _name;
#region private implementation

private CounterGroup _group;
private Dictionary<string, string> _metaData;
internal EventSource _eventSource;
private Dictionary<string, string> _metadata;

internal abstract void WritePayload(float intervalSec);

// arbitrarily we use name as the lock object.
internal object MyLock { get { return _name; } }
internal object MyLock { get { return Name; } }
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a need for the second property to access name or could the lock just be on Name? MyLock also feels like a slightly odd name, in collections it would probalby be called SyncLock


internal void ReportOutOfBandMessage(string message)
{
_eventSource.ReportOutOfBandMessage(message, true);
EventSource.ReportOutOfBandMessage(message, true);
}

internal string GetMetaDataString()
internal string GetMetadataString()
{
if (_metadata == null)
{
return "";
}

StringBuilder sb = new StringBuilder("");
foreach(KeyValuePair<string, string> kvPair in _metaData)
foreach(KeyValuePair<string, string> kvPair in _metadata)
{
sb.Append($"{kvPair.Key}:{kvPair.Value},");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ namespace System.Diagnostics.Tracing
/// See https://github.com/dotnet/corefx/blob/master/src/System.Diagnostics.Tracing/tests/BasicEventSourceTest/TestEventCounter.cs
/// which shows tests, which are also useful in seeing actual use.
/// </summary>
public partial class EventCounter : BaseCounter
public partial class EventCounter : DiagnosticCounter
{
/// <summary>
/// Initializes a new instance of the <see cref="EventCounter"/> class.
Expand All @@ -37,8 +37,8 @@ public partial class EventCounter : BaseCounter
/// <param name="eventSource">The event source.</param>
public EventCounter(string name, EventSource eventSource) : base(name, eventSource)
{
_min = float.PositiveInfinity;
_max = float.NegativeInfinity;
_min = double.PositiveInfinity;
_max = double.NegativeInfinity;

InitializeBuffer();
}
Expand All @@ -49,22 +49,27 @@ public EventCounter(string name, EventSource eventSource) : base(name, eventSour
/// </summary>
/// <param name="value">The value.</param>
public void WriteMetric(float value)
{
Enqueue((double)value);
}

public void WriteMetric(double value)
{
Enqueue(value);
}

public override string ToString() => $"EventCounter '{_name}' Count {_count} Mean {(((double)_sum) / _count).ToString("n3")}";
public override string ToString() => $"EventCounter '{Name}' Count {_count} Mean {(_sum / _count).ToString("n3")}";

#region Statistics Calculation

// Statistics
private int _count;
private float _sum;
private float _sumSquared;
private float _min;
private float _max;
private double _sum;
private double _sumSquared;
private double _min;
private double _max;

internal void OnMetricWritten(float value)
internal void OnMetricWritten(double value)
{
Debug.Assert(Monitor.IsEntered(MyLock));
_sum += value;
Expand All @@ -83,14 +88,13 @@ internal override void WritePayload(float intervalSec)
lock (MyLock)
{
Flush();
EventCounterPayload payload = new EventCounterPayload();
payload.Name = _name;
CounterPayload payload = new CounterPayload();
payload.Count = _count;
payload.IntervalSec = intervalSec;
if (0 < _count)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this condition is correct for computing the Mean, but it should be _count > 1 for standard deviation.

{
payload.Mean = _sum / _count;
payload.StandardDeviation = (float)Math.Sqrt(_sumSquared / _count - _sum * _sum / _count / _count);
payload.StandardDeviation = Math.Sqrt(_sumSquared / _count - _sum * _sum / _count / _count);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we add a comment here about potential overflow for large values?

Copy link

@jorive jorive Apr 9, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not familiar with this formula, could you please add a reference to it?
[Edit] You are using the formula for a finite population with equal probabilities for all points :)

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Optionally, it could be:

Math.Sqrt((1.0 / (_count * (_count - 1))) * (_count * _sumSquared - (_sum * _sum)));

}
else
{
Expand All @@ -99,8 +103,12 @@ internal override void WritePayload(float intervalSec)
}
payload.Min = _min;
payload.Max = _max;

payload.Metadata = GetMetadataString();
payload.DisplayName = DisplayName;
payload.Name = Name;
ResetStatistics();
_eventSource.Write("EventCounters", new EventSourceOptions() { Level = EventLevel.LogAlways }, new EventCounterPayloadType(payload));
EventSource.Write("EventCounters", new EventSourceOptions() { Level = EventLevel.LogAlways }, new CounterPayloadType(payload));
}
}
private void ResetStatistics()
Expand All @@ -109,35 +117,35 @@ private void ResetStatistics()
_count = 0;
_sum = 0;
_sumSquared = 0;
_min = float.PositiveInfinity;
_max = float.NegativeInfinity;
_min = double.PositiveInfinity;
_max = double.NegativeInfinity;
}

#endregion // Statistics Calculation

// Values buffering
private const int BufferedSize = 10;
private const float UnusedBufferSlotValue = float.NegativeInfinity;
private const double UnusedBufferSlotValue = double.NegativeInfinity;
private const int UnsetIndex = -1;
private volatile float[] _bufferedValues;
private volatile double[] _bufferedValues;
private volatile int _bufferedValuesIndex;

private void InitializeBuffer()
{
_bufferedValues = new float[BufferedSize];
_bufferedValues = new double[BufferedSize];
for (int i = 0; i < _bufferedValues.Length; i++)
{
_bufferedValues[i] = UnusedBufferSlotValue;
}
}

protected void Enqueue(float value)
private void Enqueue(double value)
{
// It is possible that two threads read the same bufferedValuesIndex, but only one will be able to write the slot, so that is okay.
int i = _bufferedValuesIndex;
while (true)
{
float result = Interlocked.CompareExchange(ref _bufferedValues[i], value, UnusedBufferSlotValue);
double result = Interlocked.CompareExchange(ref _bufferedValues[i], value, UnusedBufferSlotValue);
i++;
if (_bufferedValues.Length <= i)
{
Expand Down Expand Up @@ -178,10 +186,10 @@ protected void Flush()
/// This is the payload that is sent in the with EventSource.Write
/// </summary>
[EventData]
class EventCounterPayloadType
class CounterPayloadType
{
public EventCounterPayloadType(EventCounterPayload payload) { Payload = payload; }
public EventCounterPayload Payload { get; set; }
public CounterPayloadType(CounterPayload payload) { Payload = payload; }
public CounterPayload Payload { get; set; }
}

}
Loading