Skip to content

Commit 242cdea

Browse files
stefanosianostefanosiano
committed
added metric_bucket data category for rate limits
updated metric normalization rules
1 parent d02f183 commit 242cdea

File tree

8 files changed

+259
-15
lines changed

8 files changed

+259
-15
lines changed

‎dart/lib/src/metrics/metric.dart

Lines changed: 38 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,9 @@ import 'package:meta/meta.dart';
44

55
import '../../sentry.dart';
66

7-
final RegExp forbiddenKeyCharsRegex = RegExp('[^a-zA-Z0-9_/.-]+');
8-
final RegExp forbiddenValueCharsRegex =
9-
RegExp('[^\\w\\d\\s_:/@\\.\\{\\}\\[\\]\$-]+');
10-
final RegExp forbiddenUnitCharsRegex = RegExp('[^a-zA-Z0-9_/.]+');
7+
final RegExp unitRegex = RegExp('[^\\w]+');
8+
final RegExp nameRegex = RegExp('[^\\w-.]+');
9+
final RegExp tagKeyRegex = RegExp('[^\\w-./]+');
1110

1211
/// Base class for metrics.
1312
/// Each metric is identified by a [key]. Its [type] describes its behaviour.
@@ -69,7 +68,7 @@ abstract class Metric {
6968
/// and it's appended at the end of the encoded metric.
7069
String encodeToStatsd(int bucketKey) {
7170
final buffer = StringBuffer();
72-
buffer.write(_normalizeKey(key));
71+
buffer.write(_sanitizeName(key));
7372
buffer.write("@");
7473

7574
final sanitizeUnitName = _sanitizeUnit(unit.name);
@@ -87,7 +86,7 @@ abstract class Metric {
8786
buffer.write("|#");
8887
final serializedTags = tags.entries
8988
.map((tag) =>
90-
'${_normalizeKey(tag.key)}:${_normalizeTagValue(tag.value)}')
89+
'${_sanitizeTagKey(tag.key)}:${_sanitizeTagValue(tag.value)}')
9190
.join(',');
9291
buffer.write(serializedTags);
9392
}
@@ -117,16 +116,43 @@ abstract class Metric {
117116
String getSpanAggregationKey() => '${type.statsdType}:$key@${unit.name}';
118117

119118
/// Remove forbidden characters from the metric key and tag key.
120-
String _normalizeKey(String input) =>
121-
input.replaceAll(forbiddenKeyCharsRegex, '_');
119+
String _sanitizeName(String input) => input.replaceAll(nameRegex, '_');
122120

123121
/// Remove forbidden characters from the tag value.
124-
String _normalizeTagValue(String input) =>
125-
input.replaceAll(forbiddenValueCharsRegex, '');
122+
String _sanitizeTagKey(String input) => input.replaceAll(tagKeyRegex, '');
126123

127124
/// Remove forbidden characters from the metric unit.
128-
String _sanitizeUnit(String input) =>
129-
input.replaceAll(forbiddenUnitCharsRegex, '_');
125+
String _sanitizeUnit(String input) => input.replaceAll(unitRegex, '');
126+
127+
String _sanitizeTagValue(String input) {
128+
// see https://develop.sentry.dev/sdk/metrics/#tag-values-replacement-map
129+
// Line feed -> \n
130+
// Carriage return -> \r
131+
// Tab -> \t
132+
// Backslash -> \\
133+
// Pipe -> \\u{7c}
134+
// Comma -> \\u{2c}
135+
final buffer = StringBuffer();
136+
for (int i = 0; i < input.length; i++) {
137+
final ch = input[i];
138+
if (ch == '\n') {
139+
buffer.write("\\n");
140+
} else if (ch == '\r') {
141+
buffer.write("\\r");
142+
} else if (ch == '\t') {
143+
buffer.write("\\t");
144+
} else if (ch == '\\') {
145+
buffer.write("\\\\");
146+
} else if (ch == '|') {
147+
buffer.write("\\u{7c}");
148+
} else if (ch == ',') {
149+
buffer.write("\\u{2c}");
150+
} else {
151+
buffer.write(ch);
152+
}
153+
}
154+
return buffer.toString();
155+
}
130156
}
131157

132158
/// Metric [MetricType.counter] that tracks a value that can only be incremented.

‎dart/lib/src/transport/data_category.dart

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ enum DataCategory {
77
transaction,
88
attachment,
99
security,
10+
metricBucket,
1011
unknown
1112
}
1213

@@ -27,6 +28,8 @@ extension DataCategoryExtension on DataCategory {
2728
return DataCategory.attachment;
2829
case 'security':
2930
return DataCategory.security;
31+
case 'metric_bucket':
32+
return DataCategory.metricBucket;
3033
}
3134
return DataCategory.unknown;
3235
}
@@ -47,6 +50,8 @@ extension DataCategoryExtension on DataCategory {
4750
return 'attachment';
4851
case DataCategory.security:
4952
return 'security';
53+
case DataCategory.metricBucket:
54+
return 'metric_bucket';
5055
case DataCategory.unknown:
5156
return 'unknown';
5257
}

‎dart/lib/src/transport/rate_limit.dart

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,10 @@ import 'data_category.dart';
22

33
/// `RateLimit` containing limited `DataCategory` and duration in milliseconds.
44
class RateLimit {
5-
RateLimit(this.category, this.duration);
5+
RateLimit(this.category, this.duration, {List<String>? namespaces})
6+
: namespaces = (namespaces?..removeWhere((e) => e.isEmpty)) ?? [];
67

78
final DataCategory category;
89
final Duration duration;
10+
final List<String> namespaces;
911
}

‎dart/lib/src/transport/rate_limit_parser.dart

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ class RateLimitParser {
1414
if (rateLimitHeader == null) {
1515
return [];
1616
}
17+
// example: 2700:metric_bucket:organization:quota_exceeded:custom,...
1718
final rateLimits = <RateLimit>[];
1819
final rateLimitValues = rateLimitHeader.toLowerCase().split(',');
1920
for (final rateLimitValue in rateLimitValues) {
@@ -30,7 +31,17 @@ class RateLimitParser {
3031
final categoryValues = allCategories.split(';');
3132
for (final categoryValue in categoryValues) {
3233
final category = DataCategoryExtension.fromStringValue(categoryValue);
33-
if (category != DataCategory.unknown) {
34+
// Metric buckets rate limit can have namespaces
35+
if (category == DataCategory.metricBucket) {
36+
final namespaces = durationAndCategories.length > 4
37+
? durationAndCategories[4]
38+
: null;
39+
rateLimits.add(RateLimit(
40+
category,
41+
duration,
42+
namespaces: namespaces?.trim().split(','),
43+
));
44+
} else if (category != DataCategory.unknown) {
3445
rateLimits.add(RateLimit(category, duration));
3546
}
3647
}

‎dart/lib/src/transport/rate_limiter.dart

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,11 @@ class RateLimiter {
6464
}
6565

6666
for (final rateLimit in rateLimits) {
67+
if (rateLimit.category == DataCategory.metricBucket &&
68+
rateLimit.namespaces.isNotEmpty &&
69+
!rateLimit.namespaces.contains('custom')) {
70+
continue;
71+
}
6772
_applyRetryAfterOnlyIfLonger(
6873
rateLimit.category,
6974
DateTime.fromMillisecondsSinceEpoch(
@@ -111,6 +116,10 @@ class RateLimiter {
111116
return DataCategory.attachment;
112117
case 'transaction':
113118
return DataCategory.transaction;
119+
// The envelope item type used for metrics is statsd,
120+
// whereas the client report category is metric_bucket
121+
case 'statsd':
122+
return DataCategory.metricBucket;
114123
default:
115124
return DataCategory.unknown;
116125
}

‎dart/test/metrics/metric_test.dart

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,10 +58,50 @@ void main() {
5858
test('encode CounterMetric', () async {
5959
final int bucketKey = 10;
6060
final expectedStatsd =
61-
'key_metric_@hour:2.1|c|#tag1:tag value 1,key_2:@13/-d_s|T10';
61+
'key_metric_@hour:2.1|c|#tag1:tag\\u{2c} value 1,key2:&@"13/-d_s|T10';
6262
final actualStatsd = fixture.counterMetric.encodeToStatsd(bucketKey);
6363
expect(actualStatsd, expectedStatsd);
6464
});
65+
66+
test('sanitize name', () async {
67+
final metric = Metric.fromType(
68+
type: MetricType.counter,
69+
value: 2.1,
70+
key: 'key£ - @# metric!',
71+
unit: DurationSentryMeasurementUnit.day,
72+
tags: {},
73+
);
74+
75+
final expectedStatsd = 'key_-_metric_@day:2.1|c|T10';
76+
expect(metric.encodeToStatsd(10), expectedStatsd);
77+
});
78+
79+
test('sanitize unit', () async {
80+
final metric = Metric.fromType(
81+
type: MetricType.counter,
82+
value: 2.1,
83+
key: 'key',
84+
unit: CustomSentryMeasurementUnit('weird-measurement name!'),
85+
tags: {},
86+
);
87+
88+
final expectedStatsd = 'key@weirdmeasurementname:2.1|c|T10';
89+
expect(metric.encodeToStatsd(10), expectedStatsd);
90+
});
91+
92+
test('sanitize tags', () async {
93+
final metric = Metric.fromType(
94+
type: MetricType.counter,
95+
value: 2.1,
96+
key: 'key',
97+
unit: DurationSentryMeasurementUnit.day,
98+
tags: {'tag1': 'tag, value 1', 'key 2': '&@"13/-d_s'},
99+
);
100+
101+
final expectedStatsd =
102+
'key@day:2.1|c|#tag1:tag\\u{2c} value 1,key2:&@"13/-d_s|T10';
103+
expect(metric.encodeToStatsd(10), expectedStatsd);
104+
});
65105
});
66106

67107
group('getCompositeKey', () {

‎dart/test/protocol/rate_limit_parser_test.dart

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,45 @@ void main() {
7575
expect(sut[0].duration.inMilliseconds,
7676
RateLimitParser.httpRetryAfterDefaultDelay.inMilliseconds);
7777
});
78+
79+
test('do not parse namespaces if not metric_bucket', () {
80+
final sut =
81+
RateLimitParser('1:transaction:organization:quota_exceeded:custom')
82+
.parseRateLimitHeader();
83+
84+
expect(sut.length, 1);
85+
expect(sut[0].category, DataCategory.transaction);
86+
expect(sut[0].namespaces, isEmpty);
87+
});
88+
89+
test('parse namespaces on metric_bucket', () {
90+
final sut =
91+
RateLimitParser('1:metric_bucket:organization:quota_exceeded:custom')
92+
.parseRateLimitHeader();
93+
94+
expect(sut.length, 1);
95+
expect(sut[0].category, DataCategory.metricBucket);
96+
expect(sut[0].namespaces, isNotEmpty);
97+
expect(sut[0].namespaces.first, 'custom');
98+
});
99+
100+
test('parse empty namespaces on metric_bucket', () {
101+
final sut =
102+
RateLimitParser('1:metric_bucket:organization:quota_exceeded:')
103+
.parseRateLimitHeader();
104+
105+
expect(sut.length, 1);
106+
expect(sut[0].category, DataCategory.metricBucket);
107+
expect(sut[0].namespaces, isEmpty);
108+
});
109+
110+
test('parse missing namespaces on metric_bucket', () {
111+
final sut = RateLimitParser('1:metric_bucket').parseRateLimitHeader();
112+
113+
expect(sut.length, 1);
114+
expect(sut[0].category, DataCategory.metricBucket);
115+
expect(sut[0].namespaces, isEmpty);
116+
});
78117
});
79118

80119
group('parseRetryAfterHeader', () {

‎dart/test/protocol/rate_limiter_test.dart

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -228,6 +228,118 @@ void main() {
228228
expect(fixture.mockRecorder.category, DataCategory.transaction);
229229
expect(fixture.mockRecorder.reason, DiscardReason.rateLimitBackoff);
230230
});
231+
232+
test('dropping of metrics recorded', () {
233+
final rateLimiter = fixture.getSut();
234+
235+
final metricsItem = SentryEnvelopeItem.fromMetrics({});
236+
final eventEnvelope = SentryEnvelope(
237+
SentryEnvelopeHeader.newEventId(),
238+
[metricsItem],
239+
);
240+
241+
rateLimiter.updateRetryAfterLimits(
242+
'1:metric_bucket:key, 5:metric_bucket:organization', null, 1);
243+
244+
final result = rateLimiter.filter(eventEnvelope);
245+
expect(result, isNull);
246+
247+
expect(fixture.mockRecorder.category, DataCategory.metricBucket);
248+
expect(fixture.mockRecorder.reason, DiscardReason.rateLimitBackoff);
249+
});
250+
251+
group('apply rateLimit', () {
252+
test('error', () {
253+
final rateLimiter = fixture.getSut();
254+
fixture.dateTimeToReturn = 0;
255+
256+
final eventItem = SentryEnvelopeItem.fromEvent(SentryEvent());
257+
final envelope = SentryEnvelope(
258+
SentryEnvelopeHeader.newEventId(),
259+
[eventItem],
260+
);
261+
262+
rateLimiter.updateRetryAfterLimits(
263+
'1:error:key, 5:error:organization', null, 1);
264+
265+
expect(rateLimiter.filter(envelope), isNull);
266+
});
267+
268+
test('transaction', () {
269+
final rateLimiter = fixture.getSut();
270+
fixture.dateTimeToReturn = 0;
271+
272+
final transaction = fixture.getTransaction();
273+
final eventItem = SentryEnvelopeItem.fromTransaction(transaction);
274+
final envelope = SentryEnvelope(
275+
SentryEnvelopeHeader.newEventId(),
276+
[eventItem],
277+
);
278+
279+
rateLimiter.updateRetryAfterLimits(
280+
'1:transaction:key, 5:transaction:organization', null, 1);
281+
282+
final result = rateLimiter.filter(envelope);
283+
expect(result, isNull);
284+
});
285+
286+
test('metrics', () {
287+
final rateLimiter = fixture.getSut();
288+
fixture.dateTimeToReturn = 0;
289+
290+
final metricsItem = SentryEnvelopeItem.fromMetrics({});
291+
final envelope = SentryEnvelope(
292+
SentryEnvelopeHeader.newEventId(),
293+
[metricsItem],
294+
);
295+
296+
rateLimiter.updateRetryAfterLimits(
297+
'1:metric_bucket:key, 5:metric_bucket:organization', null, 1);
298+
299+
final result = rateLimiter.filter(envelope);
300+
expect(result, isNull);
301+
});
302+
303+
test('metrics with empty namespaces', () {
304+
final rateLimiter = fixture.getSut();
305+
fixture.dateTimeToReturn = 0;
306+
307+
final eventItem = SentryEnvelopeItem.fromEvent(SentryEvent());
308+
final metricsItem = SentryEnvelopeItem.fromMetrics({});
309+
final envelope = SentryEnvelope(
310+
SentryEnvelopeHeader.newEventId(),
311+
[eventItem, metricsItem],
312+
);
313+
314+
rateLimiter.updateRetryAfterLimits(
315+
'10:metric_bucket:key:quota_exceeded:', null, 1);
316+
317+
final result = rateLimiter.filter(envelope);
318+
expect(result, isNotNull);
319+
expect(result!.items.length, 1);
320+
expect(result.items.first.header.type, 'event');
321+
});
322+
323+
test('metrics with custom namespace', () {
324+
final rateLimiter = fixture.getSut();
325+
fixture.dateTimeToReturn = 0;
326+
327+
final eventItem = SentryEnvelopeItem.fromEvent(SentryEvent());
328+
final metricsItem = SentryEnvelopeItem.fromMetrics({});
329+
final envelope = SentryEnvelope(
330+
SentryEnvelopeHeader.newEventId(),
331+
[eventItem, metricsItem],
332+
);
333+
334+
rateLimiter.updateRetryAfterLimits(
335+
'10:metric_bucket:key:quota_exceeded:custom', null, 1);
336+
337+
final result = rateLimiter.filter(envelope);
338+
expect(result, isNotNull);
339+
expect(result!.items.length, 1);
340+
expect(result.items.first.header.type, 'event');
341+
});
342+
});
231343
}
232344

233345
class Fixture {

0 commit comments

Comments
 (0)