diff --git a/CHANGELOG.md b/CHANGELOG.md index b22c8eb38a..08f49f3f18 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +### Features + +- Metrics now honor any Rate Limits set in HTTP headers returned by Sentry ([#3276](https://github.com/getsentry/sentry-dotnet/pull/3276)) + ### Fixes - Fixed normalization for metric tag values for carriage return, line feed and tab characters ([#3281](https://github.com/getsentry/sentry-dotnet/pull/3281)) diff --git a/src/Sentry/Http/HttpTransportBase.cs b/src/Sentry/Http/HttpTransportBase.cs index 3590898802..47232d9868 100644 --- a/src/Sentry/Http/HttpTransportBase.cs +++ b/src/Sentry/Http/HttpTransportBase.cs @@ -271,6 +271,12 @@ private void ExtractRateLimits(HttpHeaders responseHeaders) { foreach (var rateLimitCategory in rateLimit.Categories) { + if (string.Equals(rateLimitCategory.Name, "metric_bucket", StringComparison.OrdinalIgnoreCase) + && !rateLimit.IsDefaultNamespace) + { + // Currently we only back off for default/empty namespaces + continue; + } CategoryLimitResets[rateLimitCategory] = now + rateLimit.RetryAfter; } } diff --git a/src/Sentry/Internal/Http/RateLimit.cs b/src/Sentry/Internal/Http/RateLimit.cs index d36b487fb5..adfceab429 100644 --- a/src/Sentry/Internal/Http/RateLimit.cs +++ b/src/Sentry/Internal/Http/RateLimit.cs @@ -4,14 +4,19 @@ internal class RateLimit { public IReadOnlyList Categories { get; } + public IReadOnlyList? Namespaces { get; } + + internal bool IsDefaultNamespace => + Namespaces is null || + (Namespaces.Count == 1 && string.Equals(Namespaces[0], "custom", StringComparison.OrdinalIgnoreCase)); + public TimeSpan RetryAfter { get; } - public RateLimit( - IReadOnlyList categories, - TimeSpan retryAfter) + public RateLimit(TimeSpan retryAfter, IReadOnlyList categories, IReadOnlyList? namespaces = null) { - Categories = categories; RetryAfter = retryAfter; + Categories = categories; + Namespaces = namespaces; } public static RateLimit Parse(string rateLimitEncoded) @@ -21,10 +26,20 @@ public static RateLimit Parse(string rateLimitEncoded) var retryAfter = TimeSpan.FromSeconds(int.Parse(components[0], CultureInfo.InvariantCulture)); var categories = components[1].Split(';').Select(c => new RateLimitCategory(c)).ToArray(); + string[]? namespaces = null; + foreach (var category in categories) + { + if (!string.Equals(category.Name, "metric_bucket", StringComparison.OrdinalIgnoreCase)) + { + continue; + } + + // Response header looking like this: X-Sentry-Rate-Limits: 2700:metric_bucket:organization:quota_exceeded:custom + namespaces = components.Length > 4 ? components[4].Split(';') : null; + break; + } - return new RateLimit( - categories, - retryAfter); + return new RateLimit(retryAfter, categories, namespaces); } public static IEnumerable ParseMany(string rateLimitsEncoded) => diff --git a/src/Sentry/Internal/Http/RateLimitCategory.cs b/src/Sentry/Internal/Http/RateLimitCategory.cs index 3eacf1b394..cecea51561 100644 --- a/src/Sentry/Internal/Http/RateLimitCategory.cs +++ b/src/Sentry/Internal/Http/RateLimitCategory.cs @@ -23,7 +23,14 @@ public bool Matches(EnvelopeItem item) return false; } - return string.Equals(Name, type, StringComparison.OrdinalIgnoreCase); + return type switch + { + EnvelopeItem.TypeValueMetric => + // Metrics are a bit unique - the envelope item type is `statsd` but the category is `metric_bucket` + string.Equals(Name, "metric_bucket", StringComparison.OrdinalIgnoreCase), + // For most reporting categories, the envelope item type matches the client report category + _ => string.Equals(Name, type, StringComparison.OrdinalIgnoreCase) + }; } public bool Equals(RateLimitCategory? other) diff --git a/src/Sentry/Protocol/Envelopes/EnvelopeItem.cs b/src/Sentry/Protocol/Envelopes/EnvelopeItem.cs index c030debe84..1ce52ecea5 100644 --- a/src/Sentry/Protocol/Envelopes/EnvelopeItem.cs +++ b/src/Sentry/Protocol/Envelopes/EnvelopeItem.cs @@ -12,16 +12,16 @@ public sealed class EnvelopeItem : ISerializable, IDisposable { private const string TypeKey = "type"; - private const string TypeValueEvent = "event"; - private const string TypeValueUserReport = "user_report"; - private const string TypeValueTransaction = "transaction"; - private const string TypeValueSession = "session"; - private const string TypeValueCheckIn = "check_in"; - private const string TypeValueAttachment = "attachment"; - private const string TypeValueClientReport = "client_report"; - private const string TypeValueProfile = "profile"; - private const string TypeValueMetric = "statsd"; - private const string TypeValueCodeLocations = "metric_meta"; + internal const string TypeValueEvent = "event"; + internal const string TypeValueUserReport = "user_report"; + internal const string TypeValueTransaction = "transaction"; + internal const string TypeValueSession = "session"; + internal const string TypeValueCheckIn = "check_in"; + internal const string TypeValueAttachment = "attachment"; + internal const string TypeValueClientReport = "client_report"; + internal const string TypeValueProfile = "profile"; + internal const string TypeValueMetric = "statsd"; + internal const string TypeValueCodeLocations = "metric_meta"; private const string LengthKey = "length"; private const string FileNameKey = "filename"; diff --git a/test/Sentry.Tests/Internals/Http/HttpTransportTests.cs b/test/Sentry.Tests/Internals/Http/HttpTransportTests.cs index c19e16a585..0fdb6548a4 100644 --- a/test/Sentry.Tests/Internals/Http/HttpTransportTests.cs +++ b/test/Sentry.Tests/Internals/Http/HttpTransportTests.cs @@ -276,13 +276,17 @@ public async Task SendEnvelopeAsync_ResponseNotOkNoMessage_LogsError() ).Should().BeTrue(); } - [Fact] - public async Task SendEnvelopeAsync_ItemRateLimit_DropsItem() + [Theory] + [InlineData("2700:metric_bucket:organization:quota_exceeded:custom", true)] // Default namespace... we back off + [InlineData("2700:metric_bucket:organization:quota_exceeded", true)] // No namespace... we back off + [InlineData("2700:metric_bucket:organization:", true)] // Empty namespace... we back off + [InlineData("2700:metric_bucket:organization:quota_exceeded:foo", false)] // Specific namespace... we don't back off for these yet + public async Task SendEnvelopeAsync_ItemRateLimit_DropsItem(string metricNamespace, bool shouldDropMetricEnvelope) { // Arrange using var httpHandler = new RecordingHttpMessageHandler( new FakeHttpMessageHandler( - () => SentryResponses.GetRateLimitResponse("1234:event, 897:transaction") + () => SentryResponses.GetRateLimitResponse($"1234:event, 897:transaction, {metricNamespace}") )); var httpTransport = new HttpTransport( @@ -305,29 +309,43 @@ public async Task SendEnvelopeAsync_ItemRateLimit_DropsItem() { // Should be dropped new EnvelopeItem( - new Dictionary {["type"] = "event"}, + new Dictionary {["type"] = EnvelopeItem.TypeValueEvent}, new EmptySerializable()), new EnvelopeItem( - new Dictionary {["type"] = "event"}, + new Dictionary {["type"] = EnvelopeItem.TypeValueEvent}, new EmptySerializable()), new EnvelopeItem( - new Dictionary {["type"] = "transaction"}, + new Dictionary {["type"] = EnvelopeItem.TypeValueTransaction}, new EmptySerializable()), // Should stay new EnvelopeItem( new Dictionary {["type"] = "other"}, - new EmptySerializable()) + new EmptySerializable()), + + // Dropped if metricNamespace is "custom" or empty + new EnvelopeItem( + new Dictionary {["type"] = EnvelopeItem.TypeValueMetric}, + new EmptySerializable()), }); + var expectedItems = new List + { + new EnvelopeItem( + new Dictionary {["type"] = "other"}, + new EmptySerializable()) + }; + if (!shouldDropMetricEnvelope) + { + expectedItems.Add( + new EnvelopeItem( + new Dictionary { ["type"] = EnvelopeItem.TypeValueMetric }, + new EmptySerializable())); + } var expectedEnvelope = new Envelope( new Dictionary(), - new[] - { - new EnvelopeItem( - new Dictionary {["type"] = "other"}, - new EmptySerializable()) - }); + expectedItems + ); var expectedEnvelopeSerialized = await expectedEnvelope.SerializeToStringAsync(_testOutputLogger, _fakeClock); diff --git a/test/Sentry.Tests/Internals/Http/RateLimitCategoryTests.cs b/test/Sentry.Tests/Internals/Http/RateLimitCategoryTests.cs index 0974483a94..f2d4001b39 100644 --- a/test/Sentry.Tests/Internals/Http/RateLimitCategoryTests.cs +++ b/test/Sentry.Tests/Internals/Http/RateLimitCategoryTests.cs @@ -5,13 +5,15 @@ namespace Sentry.Tests.Internals.Http; public class RateLimitCategoryTests { [Theory] - [InlineData("event", "event")] - [InlineData("session", "session")] - [InlineData("transaction", "transaction")] - [InlineData("attachment", "attachment")] - [InlineData("", "event")] - [InlineData("", "session")] - [InlineData("", "transaction")] + [InlineData("event", EnvelopeItem.TypeValueEvent)] + [InlineData("metric_bucket", EnvelopeItem.TypeValueMetric)] + [InlineData("session", EnvelopeItem.TypeValueSession)] + [InlineData("transaction", EnvelopeItem.TypeValueTransaction)] + [InlineData("attachment", EnvelopeItem.TypeValueAttachment)] + [InlineData("", EnvelopeItem.TypeValueEvent)] + [InlineData("", EnvelopeItem.TypeValueMetric)] + [InlineData("", EnvelopeItem.TypeValueSession)] + [InlineData("", EnvelopeItem.TypeValueTransaction)] public void Matches_IncludedItemType_ShouldMatch(string categoryName, string itemType) { // Arrange @@ -29,9 +31,10 @@ public void Matches_IncludedItemType_ShouldMatch(string categoryName, string ite } [Theory] - [InlineData("event", "transaction")] - [InlineData("error", "attachment")] - [InlineData("session", "event")] + [InlineData("event", EnvelopeItem.TypeValueTransaction)] + [InlineData("error", EnvelopeItem.TypeValueAttachment)] + [InlineData("session", EnvelopeItem.TypeValueEvent)] + [InlineData("metric_bucket", EnvelopeItem.TypeValueSession)] public void Matches_NotIncludedItemType_ShouldNotMatch(string categoryName, string itemType) { // Arrange diff --git a/test/Sentry.Tests/Internals/Http/RateLimitTests.cs b/test/Sentry.Tests/Internals/Http/RateLimitTests.cs index 01a62b9690..30991e5470 100644 --- a/test/Sentry.Tests/Internals/Http/RateLimitTests.cs +++ b/test/Sentry.Tests/Internals/Http/RateLimitTests.cs @@ -14,10 +14,7 @@ public void Parse_MinimalFormat_Works() var rateLimit = RateLimit.Parse(value); // Assert - rateLimit.Should().BeEquivalentTo(new RateLimit( - new[] { new RateLimitCategory("transaction") }, - TimeSpan.FromSeconds(60) - )); + rateLimit.Should().BeEquivalentTo(new RateLimit(TimeSpan.FromSeconds(60), new[] { new RateLimitCategory("transaction") })); } [Fact] @@ -30,10 +27,7 @@ public void Parse_MinimalFormat_EmptyCatgetory_Works() var rateLimit = RateLimit.Parse(value); // Assert - rateLimit.Should().BeEquivalentTo(new RateLimit( - new[] { new RateLimitCategory("") }, - TimeSpan.FromSeconds(60) - )); + rateLimit.Should().BeEquivalentTo(new RateLimit(TimeSpan.FromSeconds(60), new[] { new RateLimitCategory("") })); } [Fact] @@ -46,10 +40,7 @@ public void Parse_MinimalFormat_EmptyCategory_IgnoresScope() var rateLimit = RateLimit.Parse(value); // Assert - rateLimit.Should().BeEquivalentTo(new RateLimit( - new[] { new RateLimitCategory("") }, - TimeSpan.FromSeconds(60) - )); + rateLimit.Should().BeEquivalentTo(new RateLimit(TimeSpan.FromSeconds(60), new[] { new RateLimitCategory("") })); } [Fact] @@ -62,10 +53,7 @@ public void Parse_FullFormat_Works() var rateLimit = RateLimit.Parse(value); // Assert - rateLimit.Should().BeEquivalentTo(new RateLimit( - new[] { new RateLimitCategory("transaction") }, - TimeSpan.FromSeconds(60) - )); + rateLimit.Should().BeEquivalentTo(new RateLimit(TimeSpan.FromSeconds(60), new[] { new RateLimitCategory("transaction") })); } [Fact] @@ -77,15 +65,62 @@ public void Parse_MultipleCategories_Works() // Act var rateLimit = RateLimit.Parse(value); + // Assert + rateLimit.Should().BeEquivalentTo(new RateLimit(TimeSpan.FromSeconds(2700), new[] + { + new RateLimitCategory("default"), + new RateLimitCategory("error"), + new RateLimitCategory("security") + })); + } + + [Fact] + public void Parse_SingleNamespace_Works() + { + // Arrange + const string value = "2700:metric_bucket:organization:quota_exceeded:custom"; + + // Act + var rateLimit = RateLimit.Parse(value); + + // Assert + rateLimit.Should().BeEquivalentTo(new RateLimit( + TimeSpan.FromSeconds(2700), + [new RateLimitCategory("metric_bucket")], + ["custom"] + )); + } + + [Fact] + public void Parse_MultipleNamespaces_Works() + { + // Arrange + const string value = "2700:metric_bucket:organization:quota_exceeded:apples;oranges;pears"; + + // Act + var rateLimit = RateLimit.Parse(value); + + // Assert + rateLimit.Should().BeEquivalentTo(new RateLimit( + TimeSpan.FromSeconds(2700), + [new RateLimitCategory("metric_bucket")], + ["apples", "oranges", "pears"] + )); + } + + [Fact] + public void Parse_NotMetricBucket_NamespacesIgnored() + { + // Arrange + const string value = "2700:default:organization:quota_exceeded:custom"; + + // Act + var rateLimit = RateLimit.Parse(value); + // Assert rateLimit.Should().BeEquivalentTo(new RateLimit( - new[] - { - new RateLimitCategory("default"), - new RateLimitCategory("error"), - new RateLimitCategory("security") - }, - TimeSpan.FromSeconds(2700) + TimeSpan.FromSeconds(2700), + [new RateLimitCategory("default")] )); } }