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 rate limit to Metrics #3334

Merged
merged 4 commits into from
Apr 8, 2024
Merged
Show file tree
Hide file tree
Changes from 2 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

- Add rate limit to Metrics ([#3334](https://github.com/getsentry/sentry-java/pull/3334))

## 7.7.0

### Features
Expand Down
3 changes: 3 additions & 0 deletions sentry/api/sentry.api
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,7 @@ public final class io/sentry/DataCategory : java/lang/Enum {
public static final field Attachment Lio/sentry/DataCategory;
public static final field Default Lio/sentry/DataCategory;
public static final field Error Lio/sentry/DataCategory;
public static final field MetricBucket Lio/sentry/DataCategory;
public static final field Monitor Lio/sentry/DataCategory;
public static final field Profile Lio/sentry/DataCategory;
public static final field Security Lio/sentry/DataCategory;
Expand Down Expand Up @@ -4925,6 +4926,7 @@ public final class io/sentry/util/ClassLoaderUtils {
}

public final class io/sentry/util/CollectionUtils {
public static fun contains ([Ljava/lang/Object;Ljava/lang/Object;)Z
public static fun filterListEntries (Ljava/util/List;Lio/sentry/util/CollectionUtils$Predicate;)Ljava/util/List;
public static fun filterMapEntries (Ljava/util/Map;Lio/sentry/util/CollectionUtils$Predicate;)Ljava/util/Map;
public static fun map (Ljava/util/List;Lio/sentry/util/CollectionUtils$Mapper;)Ljava/util/List;
Expand Down Expand Up @@ -5092,6 +5094,7 @@ public final class io/sentry/util/SampleRateUtils {
public final class io/sentry/util/StringUtils {
public static fun byteCountToString (J)Ljava/lang/String;
public static fun calculateStringHash (Ljava/lang/String;Lio/sentry/ILogger;)Ljava/lang/String;
public static fun camelCase (Ljava/lang/String;)Ljava/lang/String;
public static fun capitalize (Ljava/lang/String;)Ljava/lang/String;
public static fun countOf (Ljava/lang/String;C)I
public static fun getStringAfterDot (Ljava/lang/String;)Ljava/lang/String;
Expand Down
1 change: 1 addition & 0 deletions sentry/src/main/java/io/sentry/DataCategory.java
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ public enum DataCategory {
Attachment("attachment"),
Monitor("monitor"),
Profile("profile"),
MetricBucket("metric_bucket"),
Transaction("transaction"),
Security("security"),
UserReport("user_report"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,9 @@ private DataCategory categoryFromItemType(SentryItemType itemType) {
if (SentryItemType.Profile.equals(itemType)) {
return DataCategory.Profile;
}
if (SentryItemType.Statsd.equals(itemType)) {
return DataCategory.MetricBucket;
}
if (SentryItemType.Attachment.equals(itemType)) {
return DataCategory.Attachment;
}
Expand Down
35 changes: 28 additions & 7 deletions sentry/src/main/java/io/sentry/transport/RateLimiter.java
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import io.sentry.clientreport.DiscardReason;
import io.sentry.hints.Retryable;
import io.sentry.hints.SubmissionResult;
import io.sentry.util.CollectionUtils;
import io.sentry.util.HintUtils;
import io.sentry.util.StringUtils;
import java.util.ArrayList;
Expand Down Expand Up @@ -168,6 +169,10 @@ private boolean isRetryAfter(final @NotNull String itemType) {
return DataCategory.Attachment;
case "profile":
return DataCategory.Profile;
// The envelope item type used for metrics is statsd, whereas the client report category is
// metric_bucket
case "statsd":
return DataCategory.MetricBucket;
case "transaction":
return DataCategory.Transaction;
case "check_in":
Expand All @@ -189,21 +194,25 @@ public void updateRetryAfterLimits(
final @Nullable String sentryRateLimitHeader,
final @Nullable String retryAfterHeader,
final int errorCode) {
// example: 2700:metric_bucket:organization:quota_exceeded:custom,...
if (sentryRateLimitHeader != null) {
for (String limit : sentryRateLimitHeader.split(",", -1)) {

// Java 11 or so has strip() :(
limit = limit.replace(" ", "");

final String[] retryAfterAndCategories =
limit.split(":", -1); // we only need for 1st and 2nd item though.
final String[] rateLimit = limit.split(":", -1);
// These can be ignored by the SDK.
// final String scope = rateLimit.length > 2 ? rateLimit[2] : null;
// final String reasonCode = rateLimit.length > 3 ? rateLimit[3] : null;
final @Nullable String limitNamespaces = rateLimit.length > 4 ? rateLimit[4] : null;

if (retryAfterAndCategories.length > 0) {
final String retryAfter = retryAfterAndCategories[0];
if (rateLimit.length > 0) {
final String retryAfter = rateLimit[0];
long retryAfterMillis = parseRetryAfterOrDefault(retryAfter);

if (retryAfterAndCategories.length > 1) {
final String allCategories = retryAfterAndCategories[1];
if (rateLimit.length > 1) {
final String allCategories = rateLimit[1];

// we dont care if Date is UTC as we just add the relative seconds
final Date date =
Expand All @@ -215,7 +224,7 @@ public void updateRetryAfterLimits(
for (final String catItem : categories) {
DataCategory dataCategory = DataCategory.Unknown;
try {
final String catItemCapitalized = StringUtils.capitalize(catItem);
final String catItemCapitalized = StringUtils.camelCase(catItem);
if (catItemCapitalized != null) {
dataCategory = DataCategory.valueOf(catItemCapitalized);
} else {
Expand All @@ -228,6 +237,18 @@ public void updateRetryAfterLimits(
if (DataCategory.Unknown.equals(dataCategory)) {
continue;
}
// SDK doesn't support namespaces, yet. Namespaces can be returned by relay in case
// of metric_bucket items. If the namespaces are empty or contain "custom" we apply
// the rate limit to all metrics, otherwise to none.
if (DataCategory.MetricBucket.equals(dataCategory)
&& limitNamespaces != null
&& !limitNamespaces.equals("")) {
final String[] namespaces = limitNamespaces.split(";", -1);
if (namespaces.length > 0 && !CollectionUtils.contains(namespaces, "custom")) {
continue;
}
}

applyRetryAfterOnlyIfLonger(dataCategory, date);
}
} else {
Expand Down
16 changes: 16 additions & 0 deletions sentry/src/main/java/io/sentry/util/CollectionUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,22 @@ public static int size(final @NotNull Iterable<?> data) {
return filteredList;
}

/**
* Returns true if the element is present in the array, false otherwise.
*
* @param array - the array
* @param element - the element
* @return true if the element is present in the array, false otherwise.
*/
public static <T> boolean contains(final @NotNull T[] array, final @NotNull T element) {
for (T t : array) {
if (t.equals(element)) {
stefanosiano marked this conversation as resolved.
Show resolved Hide resolved
return true;
}
}
return false;
}

/**
* A simplified copy of Java 8 Predicate.
*
Expand Down
21 changes: 21 additions & 0 deletions sentry/src/main/java/io/sentry/util/StringUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import java.text.StringCharacterIterator;
import java.util.Iterator;
import java.util.Locale;
import java.util.regex.Pattern;
import org.jetbrains.annotations.ApiStatus;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
Expand All @@ -21,6 +22,7 @@ public final class StringUtils {

private static final String CORRUPTED_NIL_UUID = "0000-0000";
private static final String PROPER_NIL_UUID = "00000000-0000-0000-0000-000000000000";
private static final @NotNull Pattern wordsPattern = Pattern.compile("[\\W_]+");
stefanosiano marked this conversation as resolved.
Show resolved Hide resolved

private StringUtils() {}

Expand Down Expand Up @@ -50,6 +52,25 @@ private StringUtils() {}
return str.substring(0, 1).toUpperCase(Locale.ROOT) + str.substring(1).toLowerCase(Locale.ROOT);
}

/**
* Converts a String to CamelCase format. E.g. metric_bucket => MetricBucket;
*
* @param str the String to convert
* @return the camel case converted String or itself if empty or null
*/
public static @Nullable String camelCase(final @Nullable String str) {
if (str == null || str.isEmpty()) {
return str;
}

String[] words = wordsPattern.split(str, -1);
StringBuilder builder = new StringBuilder();
for (String w : words) {
builder.append(capitalize(w));
}
return builder.toString();
}

/**
* Removes character specified by the delimiter parameter from the beginning and the end of the
* string.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import io.sentry.UncaughtExceptionHandlerIntegration.UncaughtExceptionHint
import io.sentry.UserFeedback
import io.sentry.dsnString
import io.sentry.hints.Retryable
import io.sentry.metrics.EncodedMetrics
import io.sentry.protocol.SentryId
import io.sentry.protocol.SentryTransaction
import io.sentry.protocol.User
Expand Down Expand Up @@ -67,13 +68,14 @@ class ClientReportTest {
SentryEnvelopeItem.fromUserFeedback(opts.serializer, UserFeedback(SentryId(UUID.randomUUID()))),
SentryEnvelopeItem.fromAttachment(opts.serializer, NoOpLogger.getInstance(), Attachment("{ \"number\": 10 }".toByteArray(), "log.json"), 1000),
SentryEnvelopeItem.fromProfilingTrace(ProfilingTraceData(File(""), transaction), 1000, opts.serializer),
SentryEnvelopeItem.fromCheckIn(opts.serializer, CheckIn("monitor-slug-1", CheckInStatus.ERROR))
SentryEnvelopeItem.fromCheckIn(opts.serializer, CheckIn("monitor-slug-1", CheckInStatus.ERROR)),
SentryEnvelopeItem.fromMetrics(EncodedMetrics(emptyMap()))
)

clientReportRecorder.recordLostEnvelope(DiscardReason.NETWORK_ERROR, envelope)

val clientReportAtEnd = clientReportRecorder.resetCountsAndGenerateClientReport()
testHelper.assertTotalCount(12, clientReportAtEnd)
testHelper.assertTotalCount(13, clientReportAtEnd)
testHelper.assertCountFor(DiscardReason.SAMPLE_RATE, DataCategory.Error, 3, clientReportAtEnd)
testHelper.assertCountFor(DiscardReason.BEFORE_SEND, DataCategory.Error, 2, clientReportAtEnd)
testHelper.assertCountFor(DiscardReason.QUEUE_OVERFLOW, DataCategory.Transaction, 1, clientReportAtEnd)
Expand All @@ -83,6 +85,7 @@ class ClientReportTest {
testHelper.assertCountFor(DiscardReason.NETWORK_ERROR, DataCategory.Attachment, 1, clientReportAtEnd)
testHelper.assertCountFor(DiscardReason.NETWORK_ERROR, DataCategory.Profile, 1, clientReportAtEnd)
testHelper.assertCountFor(DiscardReason.NETWORK_ERROR, DataCategory.Monitor, 1, clientReportAtEnd)
testHelper.assertCountFor(DiscardReason.NETWORK_ERROR, DataCategory.MetricBucket, 1, clientReportAtEnd)
}

@Test
Expand Down
77 changes: 73 additions & 4 deletions sentry/src/test/java/io/sentry/transport/RateLimiterTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import io.sentry.TransactionContext
import io.sentry.UserFeedback
import io.sentry.clientreport.DiscardReason
import io.sentry.clientreport.IClientReportRecorder
import io.sentry.metrics.EncodedMetrics
import io.sentry.protocol.SentryId
import io.sentry.protocol.SentryTransaction
import io.sentry.protocol.User
Expand Down Expand Up @@ -102,13 +103,14 @@ class RateLimiterTest {
val eventItem = SentryEnvelopeItem.fromEvent(fixture.serializer, SentryEvent())
val transaction = SentryTransaction(SentryTracer(TransactionContext("name", "op"), hub))
val transactionItem = SentryEnvelopeItem.fromEvent(fixture.serializer, transaction)
val envelope = SentryEnvelope(SentryEnvelopeHeader(), arrayListOf(eventItem, transactionItem))
val statsdItem = SentryEnvelopeItem.fromMetrics(EncodedMetrics(emptyMap()))
val envelope = SentryEnvelope(SentryEnvelopeHeader(), arrayListOf(eventItem, transactionItem, statsdItem))

rateLimiter.updateRetryAfterLimits("1:transaction:key, 1:default;error;security:organization", null, 1)
rateLimiter.updateRetryAfterLimits("1:transaction:key, 1:default;error;metric_bucket;security:organization", null, 1)

val result = rateLimiter.filter(envelope, Hint())
assertNotNull(result)
assertEquals(2, result.items.count())
assertEquals(3, result.items.count())
}

@Test
Expand Down Expand Up @@ -199,8 +201,9 @@ class RateLimiterTest {
val attachmentItem = SentryEnvelopeItem.fromAttachment(fixture.serializer, NoOpLogger.getInstance(), Attachment("{ \"number\": 10 }".toByteArray(), "log.json"), 1000)
val profileItem = SentryEnvelopeItem.fromProfilingTrace(ProfilingTraceData(File(""), transaction), 1000, fixture.serializer)
val checkInItem = SentryEnvelopeItem.fromCheckIn(fixture.serializer, CheckIn("monitor-slug-1", CheckInStatus.ERROR))
val statsdItem = SentryEnvelopeItem.fromMetrics(EncodedMetrics(emptyMap()))

val envelope = SentryEnvelope(SentryEnvelopeHeader(), arrayListOf(eventItem, userFeedbackItem, sessionItem, attachmentItem, profileItem, checkInItem))
val envelope = SentryEnvelope(SentryEnvelopeHeader(), arrayListOf(eventItem, userFeedbackItem, sessionItem, attachmentItem, profileItem, checkInItem, statsdItem))

rateLimiter.updateRetryAfterLimits(null, null, 429)
val result = rateLimiter.filter(envelope, Hint())
Expand All @@ -213,6 +216,7 @@ class RateLimiterTest {
verify(fixture.clientReportRecorder, times(1)).recordLostEnvelopeItem(eq(DiscardReason.RATELIMIT_BACKOFF), same(attachmentItem))
verify(fixture.clientReportRecorder, times(1)).recordLostEnvelopeItem(eq(DiscardReason.RATELIMIT_BACKOFF), same(profileItem))
verify(fixture.clientReportRecorder, times(1)).recordLostEnvelopeItem(eq(DiscardReason.RATELIMIT_BACKOFF), same(checkInItem))
verify(fixture.clientReportRecorder, times(1)).recordLostEnvelopeItem(eq(DiscardReason.RATELIMIT_BACKOFF), same(statsdItem))
verifyNoMoreInteractions(fixture.clientReportRecorder)
}

Expand Down Expand Up @@ -272,6 +276,71 @@ class RateLimiterTest {
verifyNoMoreInteractions(fixture.clientReportRecorder)
}

@Test
fun `drop metrics items as lost`() {
val rateLimiter = fixture.getSUT()
val hub = mock<IHub>()
whenever(hub.options).thenReturn(SentryOptions())

val eventItem = SentryEnvelopeItem.fromEvent(fixture.serializer, SentryEvent())
val f = File.createTempFile("test", "trace")
val transaction = SentryTracer(TransactionContext("name", "op"), hub)
val profileItem = SentryEnvelopeItem.fromProfilingTrace(ProfilingTraceData(f, transaction), 1000, fixture.serializer)
val statsdItem = SentryEnvelopeItem.fromMetrics(EncodedMetrics(emptyMap()))
val envelope = SentryEnvelope(SentryEnvelopeHeader(), arrayListOf(eventItem, profileItem, statsdItem))

rateLimiter.updateRetryAfterLimits("60:metric_bucket:key", null, 1)
val result = rateLimiter.filter(envelope, Hint())

assertNotNull(result)
assertEquals(2, result.items.toList().size)

verify(fixture.clientReportRecorder, times(1)).recordLostEnvelopeItem(eq(DiscardReason.RATELIMIT_BACKOFF), same(statsdItem))
verifyNoMoreInteractions(fixture.clientReportRecorder)
}

@Test
fun `drop metrics items if namespace is custom`() {
val rateLimiter = fixture.getSUT()
val statsdItem = SentryEnvelopeItem.fromMetrics(EncodedMetrics(emptyMap()))
val envelope = SentryEnvelope(SentryEnvelopeHeader(), arrayListOf(statsdItem))

rateLimiter.updateRetryAfterLimits("60:metric_bucket:key:quota_exceeded:custom", null, 1)
val result = rateLimiter.filter(envelope, Hint())
assertNull(result)

verify(fixture.clientReportRecorder, times(1)).recordLostEnvelopeItem(eq(DiscardReason.RATELIMIT_BACKOFF), same(statsdItem))
verifyNoMoreInteractions(fixture.clientReportRecorder)
}

@Test
fun `drop metrics items if namespaces is empty`() {
val rateLimiter = fixture.getSUT()
val statsdItem = SentryEnvelopeItem.fromMetrics(EncodedMetrics(emptyMap()))
val envelope = SentryEnvelope(SentryEnvelopeHeader(), arrayListOf(statsdItem))

rateLimiter.updateRetryAfterLimits("60:metric_bucket:key:quota_exceeded::", null, 1)
val result = rateLimiter.filter(envelope, Hint())
assertNull(result)

verify(fixture.clientReportRecorder, times(1)).recordLostEnvelopeItem(eq(DiscardReason.RATELIMIT_BACKOFF), same(statsdItem))
verifyNoMoreInteractions(fixture.clientReportRecorder)
}

@Test
fun `drop metrics items if namespaces is not present`() {
val rateLimiter = fixture.getSUT()
val statsdItem = SentryEnvelopeItem.fromMetrics(EncodedMetrics(emptyMap()))
val envelope = SentryEnvelope(SentryEnvelopeHeader(), arrayListOf(statsdItem))

rateLimiter.updateRetryAfterLimits("60:metric_bucket:key:quota_exceeded", null, 1)
val result = rateLimiter.filter(envelope, Hint())
assertNull(result)

verify(fixture.clientReportRecorder, times(1)).recordLostEnvelopeItem(eq(DiscardReason.RATELIMIT_BACKOFF), same(statsdItem))
verifyNoMoreInteractions(fixture.clientReportRecorder)
}

@Test
fun `any limit can be checked`() {
val rateLimiter = fixture.getSUT()
Expand Down
17 changes: 17 additions & 0 deletions sentry/src/test/java/io/sentry/util/CollectionUtilsTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import io.sentry.JsonObjectReader
import java.io.StringReader
import kotlin.test.Test
import kotlin.test.assertEquals
import kotlin.test.assertFalse
import kotlin.test.assertTrue

class CollectionUtilsTest {

Expand Down Expand Up @@ -62,4 +64,19 @@ class CollectionUtilsTest {
assertEquals("value1", result?.get("key1"))
assertEquals("value3", result?.get("key3"))
}

@Test
fun `contains returns false for empty arrays`() {
assertFalse(CollectionUtils.contains(emptyArray<String>(), ""))
}

@Test
fun `contains returns true if element is present`() {
assertTrue(CollectionUtils.contains(arrayOf("one", "two", "three"), "two"))
}

@Test
fun `contains returns false if element is not present`() {
assertFalse(CollectionUtils.contains(arrayOf("one", "two", "three"), "four"))
}
}
Loading
Loading