-
-
Notifications
You must be signed in to change notification settings - Fork 248
Metrics improvements #3191
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
Merged
Merged
Metrics improvements #3191
Changes from all commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
5540494
Timings no longer create spans
jamescrosswell 4d7b81e
Added transaction, release and environment tags to metrics
jamescrosswell 7e2e937
Revert "Timings no longer create spans"
jamescrosswell ebc41c7
Spans and Transactions maintain local aggregates
jamescrosswell 56a5cdd
Metrics Summary is now serialized with Spans/Transactions
jamescrosswell 4579fed
Update CHANGELOG.md
jamescrosswell 32f21f6
Format code
getsentry-bot 88f2b7b
Refactored LocalAggregator into separate aggregator and protocol classes
jamescrosswell ab32493
DictionaryExtensions and get Tags from Options instead of tx
jamescrosswell File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| namespace Sentry.Internal.Extensions; | ||
|
|
||
| internal static class DictionaryExtensions | ||
| { | ||
| public static void AddIfNotNullOrEmpty<TKey>(this IDictionary<TKey, string> dictionary, TKey key, string? value) | ||
| where TKey : notnull | ||
| { | ||
| if (!string.IsNullOrEmpty(value)) | ||
| { | ||
| dictionary.Add(key, value); | ||
| } | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| using Sentry.Protocol.Metrics; | ||
|
|
||
| namespace Sentry; | ||
|
|
||
| internal class MetricsSummaryAggregator | ||
| { | ||
| private Lazy<ConcurrentDictionary<string, SpanMetric>> LazyMeasurements { get; } = new(); | ||
| internal ConcurrentDictionary<string, SpanMetric> Measurements => LazyMeasurements.Value; | ||
|
|
||
| public void Add( | ||
| MetricType ty, | ||
| string key, | ||
| double value = 1.0, | ||
| MeasurementUnit? unit = null, | ||
| IDictionary<string, string>? tags = null | ||
| ) | ||
| { | ||
| unit ??= MeasurementUnit.None; | ||
|
|
||
| var bucketKey = MetricHelper.GetMetricBucketKey(ty, key, unit.Value, tags); | ||
|
|
||
| Measurements.AddOrUpdate( | ||
| bucketKey, | ||
| _ => new SpanMetric(ty, key, value, unit.Value, tags), | ||
| (_, metric) => | ||
| { | ||
| // This prevents multiple threads from trying to mutate the metric at the same time. The only other | ||
| // operation performed against metrics is adding one to the bucket (guaranteed to be atomic due to | ||
| // the use of a ConcurrentDictionary for the timeBucket). | ||
| lock (metric) | ||
| { | ||
| metric.Add(value); | ||
| } | ||
| return metric; | ||
| }); | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,47 @@ | ||
| using Sentry.Extensibility; | ||
|
|
||
| namespace Sentry.Protocol.Metrics; | ||
|
|
||
| internal class MetricsSummary : ISentryJsonSerializable | ||
| { | ||
| private readonly IDictionary<string, List<SpanMetric>> _measurements; | ||
|
|
||
| public MetricsSummary(MetricsSummaryAggregator aggregator) | ||
| { | ||
| // For the Metrics Summary we group all the metrics by an export key. | ||
| // See https://github.com/getsentry/rfcs/blob/main/text/0123-metrics-correlation.md#basics | ||
| var measurements = new Dictionary<string, List<SpanMetric>>(); | ||
| foreach (var (_, value) in aggregator.Measurements) | ||
| { | ||
| var exportKey = value.ExportKey; | ||
| #if NET6_0_OR_GREATER | ||
| measurements.TryAdd(exportKey, new List<SpanMetric>()); | ||
| #else | ||
| if (!measurements.ContainsKey(exportKey)) | ||
| { | ||
| measurements.Add(exportKey, new List<SpanMetric>()); | ||
| } | ||
| #endif | ||
| measurements[exportKey].Add(value); | ||
| } | ||
| _measurements = measurements.ToImmutableSortedDictionary(); | ||
| } | ||
|
|
||
| public void WriteTo(Utf8JsonWriter writer, IDiagnosticLogger? logger) | ||
| { | ||
| writer.WriteStartObject(); | ||
|
|
||
| foreach (var (exportKey, value) in _measurements) | ||
| { | ||
| writer.WritePropertyName(exportKey); | ||
| writer.WriteStartArray(); | ||
| foreach (var metric in value.OrderBy(x => MetricHelper.GetMetricBucketKey(x.MetricType, x.Key, x.Unit, x.Tags))) | ||
| { | ||
| metric.WriteTo(writer, logger); | ||
| } | ||
| writer.WriteEndArray(); | ||
| } | ||
|
|
||
| writer.WriteEndObject(); | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,54 @@ | ||
| using Sentry.Extensibility; | ||
| using Sentry.Internal.Extensions; | ||
|
|
||
| namespace Sentry.Protocol.Metrics; | ||
|
|
||
| internal record SpanMetric | ||
| { | ||
| public SpanMetric(MetricType MetricType, | ||
| string key, | ||
| double value, | ||
| MeasurementUnit unit, | ||
| IDictionary<string, string>? tags = null) | ||
| { | ||
| this.MetricType = MetricType; | ||
| Key = key; | ||
| Unit = unit; | ||
| Tags = tags; | ||
| Min = value; | ||
| Max = value; | ||
| Sum = value; | ||
| } | ||
|
|
||
| public MetricType MetricType { get; init; } | ||
| public string Key { get; init; } | ||
| public MeasurementUnit Unit { get; init; } | ||
| public IDictionary<string, string>? Tags { get; init; } | ||
|
|
||
| public double Min { get; private set; } | ||
| public double Max { get; private set; } | ||
| public double Sum { get; private set; } | ||
| public double Count { get; private set; } = 1; | ||
|
|
||
| public string ExportKey => $"{MetricType.ToStatsdType()}:{Key}@{Unit}"; | ||
|
|
||
| public void Add(double value) | ||
| { | ||
| Min = Math.Min(Min, value); | ||
| Max = Math.Max(Max, value); | ||
| Sum += value; | ||
| Count++; | ||
| } | ||
|
|
||
| /// <inheritdoc cref="ISentryJsonSerializable.WriteTo"/> | ||
| public void WriteTo(Utf8JsonWriter writer, IDiagnosticLogger? logger) | ||
| { | ||
| writer.WriteStartObject(); | ||
| writer.WriteNumber("min", Min); | ||
| writer.WriteNumber("max", Max); | ||
| writer.WriteNumber("count", Count); | ||
| writer.WriteNumber("sum", Sum); | ||
| writer.WriteStringDictionaryIfNotEmpty("tags", (IEnumerable<KeyValuePair<string, string?>>?)Tags); | ||
| writer.WriteEndObject(); | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
btw: I'd love to get rid of the OrderBy here. This only exists to ensure the order of things in JSON stays exactly the same, so our Verify tests pass. If I could work out how to have Verify do a Deep object comparison rather than a string compare, we could get rid of the OrderBy here and the ToImmutableSortedDictionary call above.