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

Added sanitization for Metric Units #3151

Merged
merged 3 commits into from
Feb 15, 2024
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
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
# Changelog

## Unreleased

### Fixes

- Metric unit names are now sanitized correctly. This was preventing some built in metrics from showing in the Sentry dashboard ([#3151](https://github.com/getsentry/sentry-dotnet/pull/3151))

## 4.1.1

### Fixes
Expand Down
10 changes: 10 additions & 0 deletions src/Sentry/MetricHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ internal static partial class MetricHelper
private const int RollupInSeconds = 10;
private const string InvalidKeyCharactersPattern = @"[^a-zA-Z0-9_/.-]+";
private const string InvalidValueCharactersPattern = @"[^\w\d_:/@\.\{\}\[\]$-]+";
// See https://docs.sysdig.com/en/docs/sysdig-monitor/integrations/working-with-integrations/custom-integrations/integrate-statsd-metrics/#characters-allowed-for-statsd-metric-names
private const string InvalidMetricUnitCharactersPattern = @"[^a-zA-Z0-9_/.]+";

#if NET6_0_OR_GREATER
private static readonly DateTimeOffset UnixEpoch = DateTimeOffset.UnixEpoch;
Expand Down Expand Up @@ -48,10 +50,18 @@ internal static DateTimeOffset GetCutoff() => DateTimeOffset.UtcNow
[GeneratedRegex(InvalidValueCharactersPattern, RegexOptions.Compiled)]
private static partial Regex InvalidValueCharacters();
internal static string SanitizeValue(string input) => InvalidValueCharacters().Replace(input, "_");

[GeneratedRegex(InvalidMetricUnitCharactersPattern, RegexOptions.Compiled)]
private static partial Regex InvalidMetricUnitCharacters();
internal static string SanitizeMetricUnit(string input) => InvalidMetricUnitCharacters().Replace(input, "_");
#else
private static readonly Regex InvalidKeyCharacters = new(InvalidKeyCharactersPattern, RegexOptions.Compiled);
internal static string SanitizeKey(string input) => InvalidKeyCharacters.Replace(input, "_");

private static readonly Regex InvalidValueCharacters = new(InvalidValueCharactersPattern, RegexOptions.Compiled);
internal static string SanitizeValue(string input) => InvalidValueCharacters.Replace(input, "_");

private static readonly Regex InvalidMetricUnitCharacters = new(InvalidMetricUnitCharactersPattern, RegexOptions.Compiled);
internal static string SanitizeMetricUnit(string input) => InvalidMetricUnitCharacters.Replace(input, "_");
#endif
}
3 changes: 2 additions & 1 deletion src/Sentry/Protocol/Metrics/Metric.cs
Original file line number Diff line number Diff line change
Expand Up @@ -108,9 +108,10 @@ public async Task SerializeAsync(Stream stream, IDiagnosticLogger? logger, Cance
var metricName = MetricHelper.SanitizeKey(Key);
await Write($"{metricName}@").ConfigureAwait(false);
var unit = Unit ?? MeasurementUnit.None;
var sanitizedUnit = MetricHelper.SanitizeMetricUnit(unit.ToString());
// We don't need ConfigureAwait(false) here as ConfigureAwait on metricName above avoids capturing the ExecutionContext.
#pragma warning disable CA2007
await Write(unit.ToString());
await Write(sanitizedUnit);

foreach (var value in SerializedStatsdValues())
{
Expand Down
13 changes: 13 additions & 0 deletions test/Sentry.Tests/MetricHelperTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -47,4 +47,17 @@ public void SanitizeValue_ShouldReplaceInvalidCharactersWithUnderscore(string in
// Assert
result.Should().Be(expected);
}

[Theory]
[InlineData("Test123_.", "Test123_.")] // Valid characters
[InlineData("test{value}", "test_value_")]
[InlineData("test-value", "test_value")]
public void SanitizeMetricUnit_ShouldReplaceInvalidCharactersWithUnderscore(string input, string expected)
{
// Act
var result = MetricHelper.SanitizeMetricUnit(input);

// Assert
result.Should().Be(expected);
}
}
Loading