Skip to content
Merged
Show file tree
Hide file tree
Changes from 7 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
11 changes: 10 additions & 1 deletion src/Sentry/Internal/BatchProcessor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ internal void Enqueue(TItem item)
activeBuffer = ReferenceEquals(activeBuffer, _buffer1) ? _buffer2 : _buffer1;
if (!TryEnqueue(activeBuffer, item))
{
_clientReportRecorder.RecordDiscardedEvent(DiscardReason.Backpressure, DataCategory.Default, 1);
_clientReportRecorder.RecordDiscardedEvent(DiscardReason.Backpressure, DiscardCategory, 1);
_diagnosticLogger?.LogInfo("{0}-Buffer full ... dropping {0}", item.GetType().Name);
}
}
Expand Down Expand Up @@ -131,6 +131,11 @@ private void CaptureItems(BatchBuffer<TItem> buffer)

protected abstract void CaptureEnvelope(IHub hub, TItem[] items);

/// <summary>
/// The data category used when recording dropped items as client reports.
/// </summary>
protected abstract DataCategory DiscardCategory { get; }

private void OnTimeoutExceeded(BatchBuffer<TItem> buffer)
{
if (!buffer.IsEmpty)
Expand Down Expand Up @@ -158,6 +163,8 @@ protected override void CaptureEnvelope(IHub hub, SentryLog[] items)
{
_ = hub.CaptureEnvelope(Envelope.FromLog(new StructuredLog(items)));
}

protected override DataCategory DiscardCategory => DataCategory.LogItem;
}

internal sealed class SentryMetricBatchProcessor : BatchProcessor<SentryMetric>
Expand All @@ -171,4 +178,6 @@ protected override void CaptureEnvelope(IHub hub, SentryMetric[] items)
{
_ = hub.CaptureEnvelope(Envelope.FromMetric(new TraceMetric(items)));
}

protected override DataCategory DiscardCategory => DataCategory.TraceMetric;
}
6 changes: 5 additions & 1 deletion src/Sentry/Internal/DataCategory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,19 @@ namespace Sentry.Internal;

internal readonly struct DataCategory : IEnumeration<DataCategory>
{
// See https://develop.sentry.dev/sdk/rate-limiting/#definitions for list
// See https://develop.sentry.dev/sdk/expected-features/rate-limiting/#definitions for list
public static DataCategory Attachment = new("attachment");
public static DataCategory Default = new("default");
public static DataCategory Error = new("error");
public static DataCategory Feedback = new("feedback");
public static DataCategory Internal = new("internal");
public static DataCategory LogItem = new("log_item");
public static DataCategory MetricBucket = new("metric_bucket");
public static DataCategory Monitor = new("monitor");
public static DataCategory Security = new("security");
public static DataCategory Session = new("session");
public static DataCategory Span = new("span");
public static DataCategory TraceMetric = new("trace_metric");
public static DataCategory Transaction = new("transaction");
public static DataCategory Profile = new("profile");

Expand Down
25 changes: 3 additions & 22 deletions src/Sentry/Internal/Http/RateLimitCategory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,28 +10,9 @@ internal class RateLimitCategory : IEquatable<RateLimitCategory>

public RateLimitCategory(string name) => Name = name;

public bool Matches(EnvelopeItem item)
{
if (IsMatchAll)
{
return true;
}

var type = item.TryGetType();
if (string.IsNullOrWhiteSpace(type))
{
return false;
}

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 Matches(EnvelopeItem item) =>
// Rate limits are keyed by data category (e.g. "error", "monitor", "metric_bucket"), not envelope item type.
IsMatchAll || string.Equals(Name, item.DataCategory.ToString(), StringComparison.OrdinalIgnoreCase);

public bool Equals(RateLimitCategory? other)
{
Expand Down
13 changes: 12 additions & 1 deletion src/Sentry/Protocol/Envelopes/EnvelopeItem.cs
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,20 @@ public sealed class EnvelopeItem : ISerializable, IDisposable
TypeValueSession => DataCategory.Session,
TypeValueAttachment => DataCategory.Attachment,
TypeValueProfile => DataCategory.Profile,
TypeValueLog => DataCategory.LogItem,
TypeValueTraceMetric => DataCategory.TraceMetric,

// The "check_in" item type corresponds to the "monitor" data category
TypeValueCheckIn => DataCategory.Monitor,

// The "statsd" item type corresponds to the "metric_bucket" data category
TypeValueMetric => DataCategory.MetricBucket,

// Client reports are SDK telemetry that Relay never rate-limits - mapped to internal
TypeValueClientReport => DataCategory.Internal,

// Not all envelope item types equate to data categories
// Specifically, user_report and client_report just use "default"
// Specifically, user_report just uses "default"
Comment thread
cursor[bot] marked this conversation as resolved.
_ => DataCategory.Default
};

Comment thread
sentry[bot] marked this conversation as resolved.
Expand Down
2 changes: 1 addition & 1 deletion test/Sentry.Tests/Internals/BatchProcessorTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ public async Task Enqueue_Concurrency_CaptureEnvelopes()
if (_fixture.ClientReportRecorder.GenerateClientReport() is { } clientReport)
{
var discardedEvent = Assert.Single(clientReport.DiscardedEvents);
Assert.Equal(new DiscardReasonWithCategory(DiscardReason.Backpressure, DataCategory.Default), discardedEvent.Key);
Assert.Equal(new DiscardReasonWithCategory(DiscardReason.Backpressure, DataCategory.LogItem), discardedEvent.Key);

droppedLogs = discardedEvent.Value;
_fixture.ExpectedDiagnosticLogs = discardedEvent.Value;
Expand Down
8 changes: 4 additions & 4 deletions test/Sentry.Tests/Internals/Http/HttpTransportTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,7 @@ public async Task SendEnvelopeAsync_ItemRateLimit_DropsItem(string metricNamespa
// Arrange
using var httpHandler = new RecordingHttpMessageHandler(
new FakeHttpMessageHandler(
() => SentryResponses.GetRateLimitResponse($"1234:event, 897:transaction, {metricNamespace}")
() => SentryResponses.GetRateLimitResponse($"1234:error, 897:transaction, {metricNamespace}")
));

var httpTransport = new HttpTransport(
Expand Down Expand Up @@ -369,7 +369,7 @@ public async Task SendEnvelopeAsync_RateLimited_CountsDiscardedEventsCorrectly()
// Arrange
using var httpHandler = new RecordingHttpMessageHandler(
new FakeHttpMessageHandler(
() => SentryResponses.GetRateLimitResponse("1234:event, 897:transaction")
() => SentryResponses.GetRateLimitResponse("1234:error, 897:transaction")
));

var options = new SentryOptions
Expand Down Expand Up @@ -485,7 +485,7 @@ public async Task SendEnvelopeAsync_Fails_RestoresDiscardedEventCounts()

// We also expect two new items recorded, due to the forced HTTP failure.
{DiscardReason.SendError.WithCategory(DataCategory.Error), 1}, // from the event
{DiscardReason.SendError.WithCategory(DataCategory.Default), 1} // from the client report
{DiscardReason.SendError.WithCategory(DataCategory.Internal), 1} // from the client report
});
}

Expand Down Expand Up @@ -858,7 +858,7 @@ public async Task SendEnvelopeAsync_RateLimited_CallsBackpressureMonitor()
// Arrange
using var httpHandler = new RecordingHttpMessageHandler(
new FakeHttpMessageHandler(
() => SentryResponses.GetRateLimitResponse("1234:event, 897:transaction")
() => SentryResponses.GetRateLimitResponse("1234:error, 897:transaction")
));

using var backpressureMonitor = new BackpressureMonitor(null, _fakeClock, false);
Expand Down
14 changes: 12 additions & 2 deletions test/Sentry.Tests/Internals/Http/RateLimitCategoryTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,16 @@ namespace Sentry.Tests.Internals.Http;
public class RateLimitCategoryTests
{
[Theory]
[InlineData("event", EnvelopeItem.TypeValueEvent)]
// Rate limits are keyed by data category, which is not always the envelope item type
[InlineData("error", EnvelopeItem.TypeValueEvent)]
[InlineData("monitor", EnvelopeItem.TypeValueCheckIn)]
[InlineData("log_item", EnvelopeItem.TypeValueLog)]
[InlineData("trace_metric", EnvelopeItem.TypeValueTraceMetric)]
[InlineData("metric_bucket", EnvelopeItem.TypeValueMetric)]
[InlineData("session", EnvelopeItem.TypeValueSession)]
[InlineData("transaction", EnvelopeItem.TypeValueTransaction)]
[InlineData("attachment", EnvelopeItem.TypeValueAttachment)]
[InlineData("profile", EnvelopeItem.TypeValueProfile)]
[InlineData("", EnvelopeItem.TypeValueEvent)]
[InlineData("", EnvelopeItem.TypeValueMetric)]
[InlineData("", EnvelopeItem.TypeValueSession)]
Expand All @@ -31,7 +36,12 @@ public void Matches_IncludedItemType_ShouldMatch(string categoryName, string ite
}

[Theory]
[InlineData("event", EnvelopeItem.TypeValueTransaction)]
// The rate limit category is the data category, not the envelope item type: an "event" item is
// limited by the "error" category, so the literal item types below must not match.
[InlineData("event", EnvelopeItem.TypeValueEvent)]
[InlineData("check_in", EnvelopeItem.TypeValueCheckIn)]
[InlineData("statsd", EnvelopeItem.TypeValueMetric)]
[InlineData("error", EnvelopeItem.TypeValueTransaction)]
[InlineData("error", EnvelopeItem.TypeValueAttachment)]
[InlineData("session", EnvelopeItem.TypeValueEvent)]
[InlineData("metric_bucket", EnvelopeItem.TypeValueSession)]
Expand Down
Loading