Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for System.Diagnostics.Metrics in dotnet-monitor's collector #3587

Merged
merged 24 commits into from
Jan 30, 2023
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
d55df48
[Dotnet Monitor] Ignore - Adding System.Diagnostics.Metrics Support (…
kkeirstead Nov 30, 2022
e6b5f21
[Dotnet Monitor] Revert To Logging a single payload (not a list) (#3538)
kkeirstead Dec 7, 2022
b52b4c4
Merge pull request #3545 from dotnet/main
wiktork Dec 8, 2022
ddf091e
[Feature branch changes, no review] Fixup counter apis (#3559)
wiktork Dec 16, 2022
959ebd6
[Dotnet-Monitor] [Feature Branch] Switched quantile to Percentile (#3…
kkeirstead Dec 21, 2022
f3fd8a6
Merge pull request #3575 from dotnet/main
wiktork Dec 29, 2022
c822924
Merge pull request #3579 from dotnet/main
wiktork Jan 4, 2023
5052752
Minor branch cleanup
wiktork Jan 4, 2023
1b70d14
Merge pull request #3580 from wiktork/dev/wiktork/cleanupFeature
wiktork Jan 4, 2023
308cffc
PR for feature branch
wiktork Jan 13, 2023
6f01912
Merge pull request #3599 from wiktork/dev/wiktork/cleanupFeature
wiktork Jan 17, 2023
149f3ad
PR feedback
wiktork Jan 19, 2023
22ff644
Pr feedback feedback
wiktork Jan 24, 2023
645c2d0
Merge pull request #3608 from wiktork/dev/wiktork/prFeedback
wiktork Jan 24, 2023
fe36113
Convert Histogram to single payload
wiktork Jan 25, 2023
79172f9
Merge pull request #3614 from wiktork/dev/wiktork/prFeedback
wiktork Jan 25, 2023
b47a957
Tweaks to account for new All flag
kkeirstead Jan 25, 2023
887c6d4
Fixed build/test failures from Wiktor's changes
kkeirstead Jan 25, 2023
ff4586f
Merge pull request #3615 from kkeirstead/kkeirstead/SDM_MetricsTypeCo…
kkeirstead Jan 25, 2023
4970332
Merge pull request #3617 from dotnet/main
wiktork Jan 26, 2023
ba1fac5
Fix issue with empty quantiles
wiktork Jan 27, 2023
2c1beab
Merge pull request #3618 from wiktork/dev/wiktork/prFeedback
wiktork Jan 27, 2023
8f8bce2
Initial PR feedback (#3620)
wiktork Jan 27, 2023
b2971ce
Fixes outdated naming in test
kkeirstead Jan 30, 2023
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 @@ -16,6 +16,7 @@ namespace Microsoft.Diagnostics.Monitoring.EventPipe
public sealed class MetricSourceConfiguration : MonitoringSourceConfiguration
{
private readonly IList<EventPipeProvider> _eventPipeProviders;
public string SessionId { get; private set; }

public MetricSourceConfiguration(float metricIntervalSeconds, IEnumerable<string> customProviderNames)
{
Expand Down Expand Up @@ -45,6 +46,37 @@ public MetricSourceConfiguration(float metricIntervalSeconds, IEnumerable<string
})).ToList();
}

public MetricSourceConfiguration(float metricIntervalSeconds, IEnumerable<string> customProviderNames, int maxHistograms, int maxTimeSeries) : this(metricIntervalSeconds, customProviderNames)
{
const long TimeSeriesValues = 0x2;
wiktork marked this conversation as resolved.
Show resolved Hide resolved
StringBuilder metrics = new StringBuilder();
foreach (string provider in customProviderNames)
wiktork marked this conversation as resolved.
Show resolved Hide resolved
{
if (metrics.Length != 0)
{
metrics.Append(",");
}

metrics.Append(provider);
}

SessionId = Guid.NewGuid().ToString();

EventPipeProvider metricsEventSourceProvider =
new EventPipeProvider("System.Diagnostics.Metrics", EventLevel.Informational, TimeSeriesValues,
new Dictionary<string, string>()
{
{ "SessionId", SessionId },
{ "Metrics", metrics.ToString() },
{ "RefreshInterval", MetricIntervalSeconds.ToString() },
{ "MaxTimeSeries", maxTimeSeries.ToString() },
{ "MaxHistograms", maxHistograms.ToString() }
}
);

_eventPipeProviders = _eventPipeProviders.Append(metricsEventSourceProvider).ToArray();
}

private string MetricIntervalSeconds { get; }

public override IList<EventPipeProvider> GetProviders() => _eventPipeProviders;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,12 @@ public bool IsIncluded(string providerName, string counterName, int intervalMill
{
return false;
}

return IsIncluded(providerName, counterName);
}

public bool IsIncluded(string providerName, string counterName)
{
if (_enabledCounters.Count == 0)
{
return true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,11 @@

namespace Microsoft.Diagnostics.Monitoring.EventPipe
{
/// <summary>
/// TODO This is currently a duplication of the src\Tools\dotnet-counters\CounterPayload.cs stack. The two will be unified in a separate change.
/// </summary>
internal class CounterPayload : ICounterPayload
{
#if NETSTANDARD
private static readonly IReadOnlyDictionary<string, string> Empty = new ReadOnlyDictionary<string, string>(new Dictionary<string, string>(0));
#else
private static readonly IReadOnlyDictionary<string, string> Empty = System.Collections.Immutable.ImmutableDictionary<string, string>.Empty;
#endif

public CounterPayload(DateTime timestamp,
string provider,
string name,
Expand All @@ -24,7 +21,7 @@ public CounterPayload(DateTime timestamp,
double value,
CounterType counterType,
float interval,
Dictionary<string, string> metadata)
string metadata)
{
Timestamp = timestamp;
Name = name;
Expand All @@ -34,14 +31,27 @@ public CounterPayload(DateTime timestamp,
CounterType = counterType;
Provider = provider;
Interval = interval;
Metadata = metadata ?? Empty;
Metadata = metadata;
EventType = EventType.Gauge;
}

// Copied from dotnet-counters
public CounterPayload(string providerName, string name, string displayName, string displayUnits, string metadata, double value, DateTime timestamp, string type, EventType eventType)
Copy link
Member

Choose a reason for hiding this comment

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

displayName is not used; either remove it, or (preferably) have it set the DisplayName property, change the property to get-only, and call all upstreams to pass in the coerced display name value.

Copy link
Member Author

Choose a reason for hiding this comment

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

There is a separate change for cleaning up all these constructors.

{
Provider = providerName;
Name = name;
Metadata = metadata;
Value = value;
Timestamp = timestamp;
CounterType = (CounterType)Enum.Parse(typeof(CounterType), type);
EventType = eventType;
}

public string Namespace { get; }

public string Name { get; }

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

public string Unit { get; }

Expand All @@ -55,6 +65,77 @@ public CounterPayload(DateTime timestamp,

public string Provider { get; }

public IReadOnlyDictionary<string, string> Metadata { get; }
public string Metadata { get; }

public EventType EventType { get; set; }
}

internal class GaugePayload : CounterPayload
{
public GaugePayload(string providerName, string name, string displayName, string displayUnits, string metadata, double value, DateTime timestamp) :
base(providerName, name, displayName, displayUnits, metadata, value, timestamp, "Metric", EventType.Gauge)
{
// In case these properties are not provided, set them to appropriate values.
string counterName = string.IsNullOrEmpty(displayName) ? name : displayName;
DisplayName = !string.IsNullOrEmpty(displayUnits) ? $"{counterName} ({displayUnits})" : counterName;
}
}

internal class CounterEndedPayload : CounterPayload
{
public CounterEndedPayload(string providerName, string name, string displayName, DateTime timestamp)
: base(providerName, name, displayName, string.Empty, null, 0.0, timestamp, "Metric", EventType.CounterEnded)
{

}
}

internal class RatePayload : CounterPayload
{
public RatePayload(string providerName, string name, string displayName, string displayUnits, string metadata, double value, double intervalSecs, DateTime timestamp) :
base(providerName, name, displayName, displayUnits, metadata, value, timestamp, "Rate", EventType.Rate)
{
// In case these properties are not provided, set them to appropriate values.
string counterName = string.IsNullOrEmpty(displayName) ? name : displayName;
string unitsName = string.IsNullOrEmpty(displayUnits) ? "Count" : displayUnits;
string intervalName = intervalSecs.ToString() + " sec";
DisplayName = $"{counterName} ({unitsName} / {intervalName})";
}
}

internal class PercentilePayload : CounterPayload
{
public PercentilePayload(string providerName, string name, string displayName, string displayUnits, string metadata, double val, DateTime timestamp) :
base(providerName, name, displayName, displayUnits, metadata, val, timestamp, "Metric", EventType.Histogram)
{
// In case these properties are not provided, set them to appropriate values.
string counterName = string.IsNullOrEmpty(displayName) ? name : displayName;
DisplayName = !string.IsNullOrEmpty(displayUnits) ? $"{counterName} ({displayUnits})" : counterName;
}
}

internal class ErrorPayload : CounterPayload
{
public ErrorPayload(string errorMessage) : this(errorMessage, DateTime.UtcNow)
{
}

public ErrorPayload(string errorMessage, DateTime timestamp) :
base(string.Empty, string.Empty, string.Empty, string.Empty, null, 0.0, timestamp, "Metric", EventType.Error)
{
ErrorMessage = errorMessage;
}

public string ErrorMessage { get; private set; }
}

// If keep this, should probably put it somewhere else
wiktork marked this conversation as resolved.
Show resolved Hide resolved
internal enum EventType : int
{
Rate,
Gauge,
Histogram,
Error,
CounterEnded
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ internal class EventCounterPipeline : EventSourcePipeline<EventPipeCounterPipeli
{
wiktork marked this conversation as resolved.
Show resolved Hide resolved
private readonly IEnumerable<ICountersLogger> _loggers;
private readonly CounterFilter _filter;
private string _sessionId;

public EventCounterPipeline(DiagnosticsClient client,
EventPipeCounterPipelineSettings settings,
Expand All @@ -38,20 +39,29 @@ public EventCounterPipeline(DiagnosticsClient client,

protected override MonitoringSourceConfiguration CreateConfiguration()
{
return new MetricSourceConfiguration(Settings.CounterIntervalSeconds, _filter.GetProviders());
var config = new MetricSourceConfiguration(Settings.CounterIntervalSeconds, _filter.GetProviders(), Settings.MaxHistograms, Settings.MaxTimeSeries);

_sessionId = config.SessionId;

return config;
}

protected override async Task OnEventSourceAvailable(EventPipeEventSource eventSource, Func<Task> stopSessionAsync, CancellationToken token)
{
ExecuteCounterLoggerAction((metricLogger) => metricLogger.PipelineStarted());
await ExecuteCounterLoggerActionAsync((metricLogger) => metricLogger.PipelineStarted(token));

eventSource.Dynamic.All += traceEvent =>
{
try
{
if (traceEvent.TryGetCounterPayload(_filter, out ICounterPayload counterPayload))
if (traceEvent.TryGetCounterPayload(_filter, _sessionId, out List<ICounterPayload> counterPayload))
{
ExecuteCounterLoggerAction((metricLogger) => metricLogger.Log(counterPayload));
ExecuteCounterLoggerAction((metricLogger) => {
foreach (var payload in counterPayload)
{
metricLogger.Log(payload);
}
});
}
}
catch (Exception)
Expand All @@ -67,7 +77,21 @@ protected override async Task OnEventSourceAvailable(EventPipeEventSource eventS

await sourceCompletedTaskSource.Task;

ExecuteCounterLoggerAction((metricLogger) => metricLogger.PipelineStopped());
await ExecuteCounterLoggerActionAsync((metricLogger) => metricLogger.PipelineStopped(token));
}

private async Task ExecuteCounterLoggerActionAsync(Func<ICountersLogger, Task> action)
{
foreach (ICountersLogger logger in _loggers)
{
try
{
await action(logger);
}
catch (ObjectDisposedException)
{
}
}
}

private void ExecuteCounterLoggerAction(Action<ICountersLogger> action)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@ internal class EventPipeCounterPipelineSettings : EventSourcePipelineSettings
//Do not use TimeSpan here since we may need to synchronize this pipeline interval
//with a different session and want to make sure the values are identical.
public float CounterIntervalSeconds { get; set; }

public int MaxHistograms { get; set; }

public int MaxTimeSeries { get; set; }
}

internal class EventPipeCounterGroup
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,17 @@ internal interface ICounterPayload

DateTime Timestamp { get; }

/// <summary>
/// The interval between counters. Note this is the actual measure of time elapsed, not the requested interval.
/// </summary>
float Interval { get; }

IReadOnlyDictionary<string, string> Metadata { get; }
/// <summary>
/// Optional metadata for counters. Note that normal counters use ':' as a separator character, while System.Diagnostics.Metrics use ';'.
/// We do not immediately convert string to Dictionary, since dotnet-counters does not need this conversion.
/// </summary>
string Metadata { get; }

EventType EventType { get; set; }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,20 +2,16 @@
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System;
using System.Collections.Generic;
using System.Text;
using System.Threading;
using System.Threading.Tasks;

namespace Microsoft.Diagnostics.Monitoring.EventPipe
{
internal interface ICountersLogger
{
//TODO Consider making these async.

void Log(ICounterPayload counter);
void PipelineStarted();
void PipelineStopped();

Task PipelineStarted(CancellationToken token);
Task PipelineStopped(CancellationToken token);
}
}
Loading