Skip to content

Commit fd96573

Browse files
authored
Widen PeriodicDataToProtoConverter's responsibilities to subsume a little more of the repeated code at its callers, and add a unit test (#204)
Co-authored-by: William Ehlhardt <[email protected]>
1 parent 004362d commit fd96573

File tree

4 files changed

+129
-51
lines changed

4 files changed

+129
-51
lines changed

src/main/java/com/arpnetworking/tsdcore/model/PeriodicDataToProtoConverter.java

Lines changed: 31 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -20,12 +20,11 @@
2020
import com.arpnetworking.metrics.mad.model.statistics.HistogramStatistic;
2121
import com.arpnetworking.metrics.mad.model.statistics.Statistic;
2222
import com.arpnetworking.metrics.mad.model.statistics.StatisticFactory;
23-
import com.google.common.collect.ImmutableMap;
23+
import com.google.common.collect.ImmutableList;
2424
import com.google.protobuf.ByteString;
2525

26-
import java.time.Duration;
27-
import java.time.ZonedDateTime;
2826
import java.util.Collection;
27+
import java.util.List;
2928
import java.util.Map;
3029
import java.util.Objects;
3130

@@ -38,43 +37,40 @@ public final class PeriodicDataToProtoConverter {
3837
private static final StatisticFactory STATISTIC_FACTORY = new StatisticFactory();
3938
private static final Statistic EXPRESSION_STATISTIC = STATISTIC_FACTORY.getStatistic("expression");
4039

41-
private final Duration _period;
42-
private final ZonedDateTime _periodStart;
43-
private final ImmutableMap<String, String> _dimensionParameters;
44-
private final String _cluster;
45-
private final String _service;
46-
4740
/**
48-
* Create a converter to generate protobuf messages for a given PeriodicData.
41+
* Convert a PeriodicData to a set of corresponding protobuf messages.
4942
*
50-
* @param periodicData Originating PeriodicData
43+
* @param periodicData PeriodicData being converted.
44+
* @return List of StatisticSetRecord protobufs corresponding to the above.
5145
*/
52-
public PeriodicDataToProtoConverter(final PeriodicData periodicData) {
53-
_period = periodicData.getPeriod();
54-
_periodStart = periodicData.getStart();
55-
_dimensionParameters = periodicData.getDimensions().getParameters();
56-
_cluster = periodicData.getDimensions().getCluster();
57-
_service = periodicData.getDimensions().getService();
46+
public static List<Messages.StatisticSetRecord> convert(
47+
final PeriodicData periodicData
48+
) {
49+
final ImmutableList.Builder<Messages.StatisticSetRecord> convertedData = ImmutableList.builder();
50+
for (final Map.Entry<String, Collection<AggregatedData>> entry : periodicData.getData().asMap().entrySet()) {
51+
final String metricName = entry.getKey();
52+
final Collection<AggregatedData> data = entry.getValue();
53+
if (!data.isEmpty()) {
54+
final Messages.StatisticSetRecord record = convertAggregatedData(
55+
periodicData, metricName, data);
56+
convertedData.add(record);
57+
}
58+
}
59+
return convertedData.build();
60+
5861
}
5962

60-
/**
61-
* Convert a metric's data to a StatisticSetRecord.
62-
*
63-
* @param metricName Name of metric being converted.
64-
* @param data Recorded metric data to serialize.
65-
* @return StatisticSetRecord protobuf corresponding to the above.
66-
*/
67-
public Messages.StatisticSetRecord convert(
63+
private static Messages.StatisticSetRecord convertAggregatedData(
64+
final PeriodicData periodicData,
6865
final String metricName,
6966
final Collection<AggregatedData> data) {
70-
7167
final Messages.StatisticSetRecord.Builder builder = Messages.StatisticSetRecord.newBuilder()
7268
.setMetric(metricName)
73-
.setPeriod(_period.toString())
74-
.setPeriodStart(_periodStart.toString())
75-
.putAllDimensions(_dimensionParameters)
76-
.setCluster(_cluster)
77-
.setService(_service);
69+
.setPeriod(periodicData.getPeriod().toString())
70+
.setPeriodStart(periodicData.getStart().toString())
71+
.putAllDimensions(periodicData.getDimensions().getParameters())
72+
.setCluster(periodicData.getDimensions().getCluster())
73+
.setService(periodicData.getDimensions().getService());
7874

7975
for (final AggregatedData datum : data) {
8076
if (Objects.equals(EXPRESSION_STATISTIC, datum.getStatistic())) {
@@ -103,6 +99,8 @@ public Messages.StatisticSetRecord convert(
10399
}
104100

105101
return builder.build();
102+
103+
106104
}
107105

108106
private static ByteString serializeSupportingData(final AggregatedData datum) {
@@ -134,4 +132,6 @@ private static ByteString serializeSupportingData(final AggregatedData datum) {
134132
}
135133
return byteString;
136134
}
135+
136+
private PeriodicDataToProtoConverter() {}
137137
}

src/main/java/com/arpnetworking/tsdcore/sinks/AggregationServerHttpSink.java

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717

1818
import com.arpnetworking.logback.annotations.LogValue;
1919
import com.arpnetworking.metrics.aggregation.protocol.Messages;
20-
import com.arpnetworking.metrics.mad.model.AggregatedData;
2120
import com.arpnetworking.steno.LogValueMapFactory;
2221
import com.arpnetworking.tsdcore.model.AggregationMessage;
2322
import com.arpnetworking.tsdcore.model.PeriodicData;
@@ -26,7 +25,6 @@
2625
import com.google.common.net.MediaType;
2726

2827
import java.util.Collection;
29-
import java.util.Map;
3028

3129
/**
3230
* Publisher to send data to an upstream aggregation server over HTTP.
@@ -50,15 +48,11 @@ protected String getContentType() {
5048

5149
@Override
5250
protected Collection<byte[]> serialize(final PeriodicData periodicData) {
51+
final Collection<Messages.StatisticSetRecord> records = PeriodicDataToProtoConverter.convert(periodicData);
52+
5353
final ImmutableList.Builder<byte[]> serializedPeriodicData = ImmutableList.builder();
54-
final PeriodicDataToProtoConverter converter = new PeriodicDataToProtoConverter(periodicData);
55-
for (final Map.Entry<String, Collection<AggregatedData>> entry : periodicData.getData().asMap().entrySet()) {
56-
final String metricName = entry.getKey();
57-
final Collection<AggregatedData> data = entry.getValue();
58-
if (!data.isEmpty()) {
59-
final Messages.StatisticSetRecord record = converter.convert(metricName, data);
60-
serializedPeriodicData.add(AggregationMessage.create(record).serializeToByteString().toArray());
61-
}
54+
for (final Messages.StatisticSetRecord record : records) {
55+
serializedPeriodicData.add(AggregationMessage.create(record).serializeToByteString().toArray());
6256
}
6357
return serializedPeriodicData.build();
6458
}

src/main/java/com/arpnetworking/tsdcore/sinks/AggregationServerSink.java

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717

1818
import com.arpnetworking.logback.annotations.LogValue;
1919
import com.arpnetworking.metrics.aggregation.protocol.Messages;
20-
import com.arpnetworking.metrics.mad.model.AggregatedData;
2120
import com.arpnetworking.steno.LogValueMapFactory;
2221
import com.arpnetworking.steno.Logger;
2322
import com.arpnetworking.steno.LoggerFactory;
@@ -27,7 +26,6 @@
2726

2827
import java.time.ZonedDateTime;
2928
import java.util.Collection;
30-
import java.util.Map;
3129

3230
/**
3331
* Publisher to send data to an upstream aggregation server.
@@ -44,14 +42,10 @@ public void recordAggregateData(final PeriodicData periodicData) {
4442
.addData("dataSize", periodicData.getData().size())
4543
.log();
4644

47-
final PeriodicDataToProtoConverter converter = new PeriodicDataToProtoConverter(periodicData);
48-
for (final Map.Entry<String, Collection<AggregatedData>> entry : periodicData.getData().asMap().entrySet()) {
49-
final String metricName = entry.getKey();
50-
final Collection<AggregatedData> data = entry.getValue();
51-
if (!data.isEmpty()) {
52-
final Messages.StatisticSetRecord record = converter.convert(metricName, data);
53-
enqueueData(AggregationMessage.create(record).serializeToBuffer());
54-
}
45+
final Collection<Messages.StatisticSetRecord> records = PeriodicDataToProtoConverter.convert(periodicData);
46+
47+
for (final Messages.StatisticSetRecord record : records) {
48+
enqueueData(AggregationMessage.create(record).serializeToBuffer());
5549
}
5650
}
5751

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
/*
2+
* Copyright 2020 Dropbox Inc.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package com.arpnetworking.tsdcore.model;
17+
18+
import com.arpnetworking.metrics.aggregation.protocol.Messages;
19+
import com.arpnetworking.metrics.mad.model.AggregatedData;
20+
import com.arpnetworking.metrics.mad.model.DefaultQuantity;
21+
import com.arpnetworking.metrics.mad.model.Unit;
22+
import com.arpnetworking.metrics.mad.model.statistics.Statistic;
23+
import com.arpnetworking.metrics.mad.model.statistics.StatisticFactory;
24+
import com.google.common.collect.ImmutableMap;
25+
import com.google.common.collect.ImmutableMultimap;
26+
import org.junit.Assert;
27+
import org.junit.Test;
28+
29+
import java.time.Duration;
30+
import java.time.ZonedDateTime;
31+
import java.util.List;
32+
33+
/**
34+
* Tests for the {@link PeriodicDataToProtoConverter} class.
35+
*
36+
* @author William Ehlhardt (whale at dropbox dot com)
37+
*/
38+
public class PeriodicDataToProtoConverterTest {
39+
private static final StatisticFactory STATISTIC_FACTORY = new StatisticFactory();
40+
private static final Statistic MEAN_STATISTIC = STATISTIC_FACTORY.getStatistic("mean");
41+
42+
@Test
43+
public void conversionTest() {
44+
final PeriodicData data = new PeriodicData.Builder()
45+
.setDimensions(
46+
new DefaultKey(
47+
ImmutableMap.of(
48+
Key.HOST_DIMENSION_KEY, "host-test",
49+
Key.SERVICE_DIMENSION_KEY, "service-test",
50+
Key.CLUSTER_DIMENSION_KEY, "cluster-test")))
51+
.setData(ImmutableMultimap.of("metric-test", new AggregatedData.Builder()
52+
.setStatistic(MEAN_STATISTIC)
53+
.setValue(new DefaultQuantity.Builder().setValue((double) 9999).setUnit(Unit.BIT).build())
54+
.setIsSpecified(true)
55+
.setPopulationSize((long) 1337).build()))
56+
.setPeriod(Duration.ofMinutes(5))
57+
.setStart(ZonedDateTime.parse("2020-06-04T16:04:38.424-07:00[America/Los_Angeles]"))
58+
.build();
59+
60+
final List<Messages.StatisticSetRecord> converted = PeriodicDataToProtoConverter.convert(data);
61+
62+
Assert.assertEquals(1, converted.size());
63+
Assert.assertEquals(
64+
"service: \"service-test\"\n"
65+
+ "cluster: \"cluster-test\"\n"
66+
+ "metric: \"metric-test\"\n"
67+
+ "period: \"PT5M\"\n"
68+
+ "period_start: \"2020-06-04T16:04:38.424-07:00[America/Los_Angeles]\"\n"
69+
+ "statistics {\n"
70+
+ " statistic: \"mean\"\n"
71+
+ " value: 1249.875\n"
72+
+ " unit: \"BYTE\"\n"
73+
+ " user_specified: true\n"
74+
+ "}\n"
75+
+ "dimensions {\n"
76+
+ " key: \"host\"\n"
77+
+ " value: \"host-test\"\n"
78+
+ "}\n"
79+
+ "dimensions {\n"
80+
+ " key: \"service\"\n"
81+
+ " value: \"service-test\"\n"
82+
+ "}\n"
83+
+ "dimensions {\n"
84+
+ " key: \"cluster\"\n"
85+
+ " value: \"cluster-test\"\n"
86+
+ "}\n",
87+
88+
converted.get(0).toString());
89+
}
90+
}

0 commit comments

Comments
 (0)