Skip to content

Commit

Permalink
Fixes issue when monitoring a process launched via the same command l…
Browse files Browse the repository at this point in the history 
…ine (#76965)

* Initial fix for null reference exception

* Took away lazy and put back private constructor

* Added parent property and made handler thread safe
  • Loading branch information
@dramos020
dramos020 authored Oct 20, 2022
1 parent b1aa168 commit a32feb0
Showing 1 changed file with 51 additions and 34 deletions.
85 changes: 51 additions & 34 deletions .../System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/MetricsEventSource.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using System.Globalization;
using System.Runtime.Versioning;
using System.Text;
using System.Threading;

namespace System.Diagnostics.Metrics
{
Expand Down Expand Up @@ -60,13 +61,22 @@ public static class Keywords
public const EventKeywords InstrumentPublishing = (EventKeywords)0x4;
}

private CommandHandler _handler;
private CommandHandler? _handler;

private MetricsEventSource()
private CommandHandler Handler
{
_handler = new CommandHandler();
get
{
if (_handler == null)
{
Interlocked.CompareExchange(ref _handler, new CommandHandler(this), null);
}
return _handler;
}
}

private MetricsEventSource() { }

/// <summary>
/// Used to send ad-hoc diagnostics to humans.
/// </summary>
Expand Down Expand Up @@ -189,7 +199,7 @@ protected override void OnEventCommand(EventCommandEventArgs command)
{
lock (this)
{
_handler.OnEventCommand(command);
Handler.OnEventCommand(command);
}
}

Expand All @@ -202,6 +212,13 @@ private sealed class CommandHandler
private AggregationManager? _aggregationManager;
private string _sessionId = "";

public CommandHandler(MetricsEventSource parent)
{
Parent = parent;
}

public MetricsEventSource Parent { get; private set;}

public void OnEventCommand(EventCommandEventArgs command)
{
try
Expand All @@ -215,7 +232,7 @@ public void OnEventCommand(EventCommandEventArgs command)
// This limitation shouldn't really matter because browser also doesn't support out-of-proc EventSource communication
// which is the intended scenario for this EventSource. If it matters in the future AggregationManager can be
// modified to have some other fallback path that works for browser.
Log.Error("", "System.Diagnostics.Metrics EventSource not supported on browser");
Parent.Error("", "System.Diagnostics.Metrics EventSource not supported on browser");
return;
}
#endif
Expand All @@ -233,13 +250,13 @@ public void OnEventCommand(EventCommandEventArgs command)
// one other listener is active. In the future we might be able to figure out how
// to infer the changes from the info we do have or add a better API but for now
// I am taking the simple route and not supporting it.
Log.MultipleSessionsNotSupportedError(_sessionId);
Parent.MultipleSessionsNotSupportedError(_sessionId);
return;
}

_aggregationManager.Dispose();
_aggregationManager = null;
Log.Message($"Previous session with id {_sessionId} is stopped");
Parent.Message($"Previous session with id {_sessionId} is stopped");
}
_sessionId = "";
}
Expand All @@ -249,12 +266,12 @@ public void OnEventCommand(EventCommandEventArgs command)
if (command.Arguments!.TryGetValue("SessionId", out string? id))
{
_sessionId = id!;
Log.Message($"SessionId argument received: {_sessionId}");
Parent.Message($"SessionId argument received: {_sessionId}");
}
else
{
_sessionId = System.Guid.NewGuid().ToString();
Log.Message($"New session started. SessionId auto-generated: {_sessionId}");
Parent.Message($"New session started. SessionId auto-generated: {_sessionId}");
}


Expand All @@ -263,55 +280,55 @@ public void OnEventCommand(EventCommandEventArgs command)
double refreshIntervalSecs;
if (command.Arguments!.TryGetValue("RefreshInterval", out string? refreshInterval))
{
Log.Message($"RefreshInterval argument received: {refreshInterval}");
Parent.Message($"RefreshInterval argument received: {refreshInterval}");
if (!double.TryParse(refreshInterval, out refreshIntervalSecs))
{
Log.Message($"Failed to parse RefreshInterval. Using default {defaultIntervalSecs}s.");
Parent.Message($"Failed to parse RefreshInterval. Using default {defaultIntervalSecs}s.");
refreshIntervalSecs = defaultIntervalSecs;
}
else if (refreshIntervalSecs < AggregationManager.MinCollectionTimeSecs)
{
Log.Message($"RefreshInterval too small. Using minimum interval {AggregationManager.MinCollectionTimeSecs} seconds.");
Parent.Message($"RefreshInterval too small. Using minimum interval {AggregationManager.MinCollectionTimeSecs} seconds.");
refreshIntervalSecs = AggregationManager.MinCollectionTimeSecs;
}
}
else
{
Log.Message($"No RefreshInterval argument received. Using default {defaultIntervalSecs}s.");
Parent.Message($"No RefreshInterval argument received. Using default {defaultIntervalSecs}s.");
refreshIntervalSecs = defaultIntervalSecs;
}

int defaultMaxTimeSeries = 1000;
int maxTimeSeries;
if (command.Arguments!.TryGetValue("MaxTimeSeries", out string? maxTimeSeriesString))
{
Log.Message($"MaxTimeSeries argument received: {maxTimeSeriesString}");
Parent.Message($"MaxTimeSeries argument received: {maxTimeSeriesString}");
if (!int.TryParse(maxTimeSeriesString, out maxTimeSeries))
{
Log.Message($"Failed to parse MaxTimeSeries. Using default {defaultMaxTimeSeries}");
Parent.Message($"Failed to parse MaxTimeSeries. Using default {defaultMaxTimeSeries}");
maxTimeSeries = defaultMaxTimeSeries;
}
}
else
{
Log.Message($"No MaxTimeSeries argument received. Using default {defaultMaxTimeSeries}");
Parent.Message($"No MaxTimeSeries argument received. Using default {defaultMaxTimeSeries}");
maxTimeSeries = defaultMaxTimeSeries;
}

int defaultMaxHistograms = 20;
int maxHistograms;
if (command.Arguments!.TryGetValue("MaxHistograms", out string? maxHistogramsString))
{
Log.Message($"MaxHistograms argument received: {maxHistogramsString}");
Parent.Message($"MaxHistograms argument received: {maxHistogramsString}");
if (!int.TryParse(maxHistogramsString, out maxHistograms))
{
Log.Message($"Failed to parse MaxHistograms. Using default {defaultMaxHistograms}");
Parent.Message($"Failed to parse MaxHistograms. Using default {defaultMaxHistograms}");
maxHistograms = defaultMaxHistograms;
}
}
else
{
Log.Message($"No MaxHistogram argument received. Using default {defaultMaxHistograms}");
Parent.Message($"No MaxHistogram argument received. Using default {defaultMaxHistograms}");
maxHistograms = defaultMaxHistograms;
}

Expand All @@ -320,27 +337,27 @@ public void OnEventCommand(EventCommandEventArgs command)
maxTimeSeries,
maxHistograms,
(i, s) => TransmitMetricValue(i, s, sessionId),
(startIntervalTime, endIntervalTime) => Log.CollectionStart(sessionId, startIntervalTime, endIntervalTime),
(startIntervalTime, endIntervalTime) => Log.CollectionStop(sessionId, startIntervalTime, endIntervalTime),
i => Log.BeginInstrumentReporting(sessionId, i.Meter.Name, i.Meter.Version, i.Name, i.GetType().Name, i.Unit, i.Description),
i => Log.EndInstrumentReporting(sessionId, i.Meter.Name, i.Meter.Version, i.Name, i.GetType().Name, i.Unit, i.Description),
i => Log.InstrumentPublished(sessionId, i.Meter.Name, i.Meter.Version, i.Name, i.GetType().Name, i.Unit, i.Description),
() => Log.InitialInstrumentEnumerationComplete(sessionId),
e => Log.Error(sessionId, e.ToString()),
() => Log.TimeSeriesLimitReached(sessionId),
() => Log.HistogramLimitReached(sessionId),
e => Log.ObservableInstrumentCallbackError(sessionId, e.ToString()));
(startIntervalTime, endIntervalTime) => Parent.CollectionStart(sessionId, startIntervalTime, endIntervalTime),
(startIntervalTime, endIntervalTime) => Parent.CollectionStop(sessionId, startIntervalTime, endIntervalTime),
i => Parent.BeginInstrumentReporting(sessionId, i.Meter.Name, i.Meter.Version, i.Name, i.GetType().Name, i.Unit, i.Description),
i => Parent.EndInstrumentReporting(sessionId, i.Meter.Name, i.Meter.Version, i.Name, i.GetType().Name, i.Unit, i.Description),
i => Parent.InstrumentPublished(sessionId, i.Meter.Name, i.Meter.Version, i.Name, i.GetType().Name, i.Unit, i.Description),
() => Parent.InitialInstrumentEnumerationComplete(sessionId),
e => Parent.Error(sessionId, e.ToString()),
() => Parent.TimeSeriesLimitReached(sessionId),
() => Parent.HistogramLimitReached(sessionId),
e => Parent.ObservableInstrumentCallbackError(sessionId, e.ToString()));

_aggregationManager.SetCollectionPeriod(TimeSpan.FromSeconds(refreshIntervalSecs));

if (command.Arguments!.TryGetValue("Metrics", out string? metricsSpecs))
{
Log.Message($"Metrics argument received: {metricsSpecs}");
Parent.Message($"Metrics argument received: {metricsSpecs}");
ParseSpecs(metricsSpecs);
}
else
{
Log.Message("No Metrics argument received");
Parent.Message("No Metrics argument received");
}

_aggregationManager.Start();
Expand All @@ -354,7 +371,7 @@ public void OnEventCommand(EventCommandEventArgs command)

private bool LogError(Exception e)
{
Log.Error(_sessionId, e.ToString());
Parent.Error(_sessionId, e.ToString());
// this code runs as an exception filter
// returning false ensures the catch handler isn't run
return false;
Expand All @@ -374,11 +391,11 @@ private void ParseSpecs(string? metricsSpecs)
{
if (!MetricSpec.TryParse(specString, out MetricSpec spec))
{
Log.Message($"Failed to parse metric spec: {specString}");
Parent.Message($"Failed to parse metric spec: {specString}");
}
else
{
Log.Message($"Parsed metric: {spec}");
Parent.Message($"Parsed metric: {spec}");
if (spec.InstrumentName != null)
{
_aggregationManager!.Include(spec.MeterName, spec.InstrumentName);
Expand Down

0 comments on commit a32feb0

Please sign in to comment.