Skip to content

Commit 88429de

Browse files
BrandonArpBrandonArp
authored
add influx style tag parsing, add tests for invalid tag data (#125)
1 parent d250a5a commit 88429de

File tree

2 files changed

+92
-36
lines changed

2 files changed

+92
-36
lines changed

‎src/main/java/com/arpnetworking/metrics/mad/parsers/StatsdToRecordParser.java‎

Lines changed: 48 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -77,39 +77,48 @@ public List<Record> parse(final ByteBuffer datagram) throws ParsingException {
7777
// CHECKSTYLE.OFF: IllegalInstantiation - This is the recommended way
7878
final String datagramAsString = new String(datagram.array(), Charsets.UTF_8);
7979
final ImmutableList.Builder<Record> recordListBuilder = ImmutableList.builder();
80-
for (final String line : LINE_SPLITTER.split(datagramAsString)) {
81-
// CHECKSTYLE.ON: IllegalInstantiation
82-
final Matcher matcher = STATSD_PATTERN.matcher(line);
83-
if (!matcher.matches()) {
84-
throw new ParsingException("Invalid statsd line", line.getBytes(Charsets.UTF_8));
85-
}
80+
try {
81+
for (final String line : LINE_SPLITTER.split(datagramAsString)) {
82+
// CHECKSTYLE.ON: IllegalInstantiation
83+
final Matcher matcher = STATSD_PATTERN.matcher(line);
84+
if (!matcher.matches()) {
85+
throw new ParsingException("Invalid statsd line", line.getBytes(Charsets.UTF_8));
86+
}
8687

87-
// Parse the name
88-
final String name = parseName(datagram, matcher.group("NAME"));
88+
// Parse the name
89+
final String name = parseName(datagram, matcher.group("NAME"));
8990

90-
// Parse the _metricType
91-
final StatsdType type = parseStatsdType(datagram, matcher.group("TYPE"));
91+
// Parse the _metricType
92+
final StatsdType type = parseStatsdType(datagram, matcher.group("TYPE"));
9293

93-
// Parse the value
94-
final Number value = parseValue(datagram, matcher.group("VALUE"), type);
94+
// Parse the value
95+
final Number value = parseValue(datagram, matcher.group("VALUE"), type);
9596

96-
// Parse the sample rate
97-
final Optional<Double> sampleRate = parseSampleRate(datagram, matcher.group("SAMPLERATE"), type);
97+
// Parse the sample rate
98+
final Optional<Double> sampleRate = parseSampleRate(datagram, matcher.group("SAMPLERATE"), type);
9899

99-
// Parse the tags
100-
final ImmutableMap<String, String> annotations = parseTags(matcher.group("TAGS"));
100+
// Parse the tags
101+
final ImmutableMap<String, String> annotations = ImmutableMap.<String, String>builder()
102+
.putAll(parseTags(matcher.group("TAGS")))
103+
.putAll(parseInfluxStyleTags(matcher.group("INFLUXTAGS")))
104+
.build();
101105

102-
// Enforce sampling
103-
if (sampleRate.isPresent() && sampleRate.get().compareTo(1.0) != 0) {
104-
if (sampleRate.get().compareTo(0.0) == 0) {
105-
return Collections.emptyList();
106-
}
107-
if (Double.compare(_randomSupplier.get().nextDouble(), sampleRate.get()) > 0) {
108-
return Collections.emptyList();
106+
// Enforce sampling
107+
if (sampleRate.isPresent() && sampleRate.get().compareTo(1.0) != 0) {
108+
if (sampleRate.get().compareTo(0.0) == 0) {
109+
return Collections.emptyList();
110+
}
111+
if (Double.compare(_randomSupplier.get().nextDouble(), sampleRate.get()) > 0) {
112+
return Collections.emptyList();
113+
}
109114
}
110-
}
111115

112-
recordListBuilder.add(createRecord(name, value, type, annotations));
116+
recordListBuilder.add(createRecord(name, value, type, annotations));
117+
}
118+
// CHECKSTYLE.OFF: IllegalCatch - We want to turn any exceptions we catch into a ParsingException
119+
} catch (final RuntimeException e) {
120+
// CHECKSTYLE.ON: IllegalCatch
121+
throw new ParsingException("Error pasring record", datagram.array(), e);
113122
}
114123

115124
return recordListBuilder.build();
@@ -153,17 +162,20 @@ private Number parseValue(
153162

154163
@SuppressFBWarnings("NP_PARAMETER_MUST_BE_NONNULL_BUT_MARKED_AS_NULLABLE")
155164
// See: https://github.com/findbugsproject/findbugs/issues/79
156-
private ImmutableMap<String, String> parseTags(@Nullable final String taqsAsString) {
157-
final ImmutableMap.Builder<String, String> annotations = ImmutableMap.builder();
158-
if (null != taqsAsString) {
159-
for (final String keyValue : taqsAsString.split(",")) {
160-
final int pivot = keyValue.indexOf(':');
161-
annotations.put(
162-
keyValue.substring(0, pivot),
163-
keyValue.substring(pivot + 1));
164-
}
165+
private ImmutableMap<String, String> parseTags(@Nullable final String tagsAsString) {
166+
if (null != tagsAsString) {
167+
return ImmutableMap.copyOf(Splitter.on(',').withKeyValueSeparator(':').split(tagsAsString));
168+
}
169+
return ImmutableMap.of();
170+
}
171+
172+
@SuppressFBWarnings("NP_PARAMETER_MUST_BE_NONNULL_BUT_MARKED_AS_NULLABLE")
173+
// See: https://github.com/findbugsproject/findbugs/issues/79
174+
private ImmutableMap<String, String> parseInfluxStyleTags(@Nullable final String tagsAsString) {
175+
if (null != tagsAsString) {
176+
return ImmutableMap.copyOf(Splitter.on(',').withKeyValueSeparator('=').split(tagsAsString));
165177
}
166-
return annotations.build();
178+
return ImmutableMap.of();
167179
}
168180

169181
private Optional<Double> parseSampleRate(
@@ -237,7 +249,7 @@ public StatsdToRecordParser() {
237249
private static final Splitter LINE_SPLITTER = Splitter.on('\n').omitEmptyStrings();
238250
private static final ThreadLocal<NumberFormat> NUMBER_FORMAT = ThreadLocal.withInitial(NumberFormat::getInstance);
239251
private static final Pattern STATSD_PATTERN = Pattern.compile(
240-
"^(?<NAME>[^:@|]+):(?<VALUE>[^|]+)\\|(?<TYPE>[^|]+)(\\|@(?<SAMPLERATE>[^|]+))?(\\|#(?<TAGS>.+))?$");
252+
"^(?<NAME>[^:@|,]+)(,(?<INFLUXTAGS>[^:@|]+))?:(?<VALUE>[^|]+)\\|(?<TYPE>[^|]+)(\\|@(?<SAMPLERATE>[^|]+))?(\\|#(?<TAGS>.+))?$");
241253

242254
private enum StatsdType {
243255
COUNTER("c", MetricType.COUNTER, null),

‎src/test/java/com/arpnetworking/metrics/mad/parsers/StatsdToRecordParserTest.java‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -214,12 +214,56 @@ public void testExampleTagsSampledAccept() throws ParsingException {
214214
_parser.parse(ByteBuffer.wrap("users.online:1|c|@0.5|#country:china".getBytes(Charsets.UTF_8)))));
215215
}
216216

217+
@Test(expected = ParsingException.class)
218+
public void testExampleTagsInvalid() throws ParsingException {
219+
Mockito.doReturn(0.50).when(_random).nextDouble();
220+
_parser.parse(ByteBuffer.wrap("users.online:1|c|@0.5|#country:china,anotherTag".getBytes(Charsets.UTF_8)));
221+
}
222+
217223
@Test
218224
public void testExampleTagsSampledReject() throws ParsingException {
219225
Mockito.doReturn(0.51).when(_random).nextDouble();
220226
Assert.assertTrue(_parser.parse(ByteBuffer.wrap("users.online:1|c|@0.5|#country:china".getBytes(Charsets.UTF_8))).isEmpty());
221227
}
222228

229+
@Test
230+
public void testInfluxStyleTagFormat() throws ParsingException {
231+
assertRecordEquality(
232+
new DefaultRecord.Builder()
233+
.setTime(_now)
234+
.setId(UUID.randomUUID().toString())
235+
.setAnnotations(ImmutableMap.of())
236+
.setDimensions(ImmutableMap.of(
237+
"service", "statsd"))
238+
.setMetrics(ImmutableMap.of(
239+
"users.online",
240+
new DefaultMetric.Builder()
241+
.setType(MetricType.COUNTER)
242+
.setValues(ImmutableList.of(
243+
new Quantity.Builder()
244+
.setValue(1.0)
245+
.build()))
246+
.build()
247+
))
248+
.build(),
249+
Iterables.getOnlyElement(
250+
_parser.parse(ByteBuffer.wrap("users.online,service=statsd:1|c".getBytes(Charsets.UTF_8)))));
251+
252+
}
253+
254+
@Test(expected = ParsingException.class)
255+
public void testInfluxStyleTagFormatInvalid() throws ParsingException {
256+
Mockito.doReturn(0.52).when(_random).nextDouble();
257+
_parser.parse(ByteBuffer.wrap("users.online,service=statsd,tag2:1|c".getBytes(Charsets.UTF_8)));
258+
259+
}
260+
261+
@Test(expected = ParsingException.class)
262+
public void testInfluxStyleTagFormatInvalid2() throws ParsingException {
263+
Mockito.doReturn(0.53).when(_random).nextDouble();
264+
_parser.parse(ByteBuffer.wrap("users.online,:,service=statsd|c".getBytes(Charsets.UTF_8)));
265+
}
266+
223267
private void assertRecordEquality(final Record expected, final Record actual) {
224268
Assert.assertEquals(expected.getTime(), actual.getTime());
225269
Assert.assertEquals(expected.getAnnotations(), actual.getAnnotations());

0 commit comments

Comments
 (0)