Skip to content

Commit 3da793e

Browse files
authored
Port improvements to Hocon parsing and HTTP route metrics to master from master-2.0 (#142)
1 parent 82a51c9 commit 3da793e

File tree

4 files changed

+80
-14
lines changed

4 files changed

+80
-14
lines changed

src/main/java/com/arpnetworking/configuration/jackson/HoconFileSource.java

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import com.typesafe.config.Config;
2424
import com.typesafe.config.ConfigFactory;
2525
import com.typesafe.config.ConfigRenderOptions;
26+
import com.typesafe.config.ConfigResolveOptions;
2627
import net.sf.oval.constraint.NotNull;
2728

2829
import java.io.File;
@@ -69,8 +70,12 @@ private HoconFileSource(final Builder builder) {
6970
if (_file.canRead()) {
7071
try {
7172
final Config config = ConfigFactory.parseFile(_file);
72-
final String hoconAsJson = config.resolve().root().render(ConfigRenderOptions.concise());
73-
jsonNode = _objectMapper.readTree(hoconAsJson);
73+
final String configHoconAsJson = config.withFallback(ConfigFactory.systemProperties())
74+
.resolve(ConfigResolveOptions.defaults())
75+
.root()
76+
.render(ConfigRenderOptions.concise());
77+
78+
jsonNode = _objectMapper.readTree(configHoconAsJson);
7479
} catch (final IOException e) {
7580
throw new RuntimeException(e);
7681
}

src/main/java/com/arpnetworking/http/Routes.java

Lines changed: 46 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -62,9 +62,12 @@
6262

6363
import java.util.Objects;
6464
import java.util.Optional;
65+
import java.util.UUID;
6566
import java.util.concurrent.CompletableFuture;
6667
import java.util.concurrent.CompletionStage;
6768
import java.util.concurrent.TimeUnit;
69+
import java.util.stream.Collectors;
70+
import java.util.stream.StreamSupport;
6871

6972
/**
7073
* Http server routes.
@@ -117,7 +120,21 @@ public CompletionStage<HttpResponse> apply(final HttpRequest request) {
117120
createMetricName(request, BODY_SIZE_METRIC),
118121
request.entity().getContentLengthOption().orElse(0L),
119122
Optional.of(Units.BYTE));
120-
// TODO(vkoskela): Add a request UUID and include in MDC. [MAI-462]
123+
final UUID requestId = UUID.randomUUID();
124+
if (LOGGER.isTraceEnabled()) {
125+
LOGGER.trace()
126+
.setEvent("http.in.start")
127+
.addContext("requestId", requestId)
128+
.addData("method", request.method().toString())
129+
.addData("url", request.getUri().toString())
130+
.addData(
131+
"headers",
132+
StreamSupport.stream(request.getHeaders().spliterator(), false)
133+
.map(h -> h.name() + "=" + h.value())
134+
.collect(Collectors.toList()))
135+
.log();
136+
}
137+
// TODO(ville): Add a request UUID and include in MDC.
121138
LOGGER.trace()
122139
.setEvent("http.in.start")
123140
.addData("method", request.method())
@@ -132,23 +149,41 @@ public CompletionStage<HttpResponse> apply(final HttpRequest request) {
132149
requestTimer.elapsed(TimeUnit.NANOSECONDS),
133150
Optional.of(Units.NANOSECOND));
134151

135-
final int responseStatusClass = response.status().intValue() / 100;
152+
final int responseStatus;
153+
if (response != null) {
154+
responseStatus = response.status().intValue();
155+
} else {
156+
// TODO(ville): Figure out how to intercept post-exception mapping.
157+
responseStatus = 599;
158+
}
159+
final int responseStatusClass = responseStatus / 100;
136160
for (final int i : STATUS_CLASSES) {
137161
_metrics.recordCounter(
138162
createMetricName(request, String.format("%s/%dxx", STATUS_METRIC, i)),
139163
responseStatusClass == i ? 1 : 0);
140164
}
141165

142-
final LogBuilder log = LOGGER.trace()
143-
.setEvent("http.in")
144-
.addData("method", request.method())
145-
.addData("url", request.getUri())
146-
.addData("status", response.status().intValue())
147-
.addData("headers", request.getHeaders());
148-
if (failure != null) {
149-
log.setEvent("http.in.error").addData("exception", failure);
166+
final LogBuilder log;
167+
if (failure != null || responseStatusClass == 5) {
168+
log = LOGGER.info().setEvent("http.in.failure");
169+
if (failure != null) {
170+
log.setThrowable(failure);
171+
}
172+
if (!LOGGER.isTraceEnabled() && LOGGER.isInfoEnabled()) {
173+
log.addData("method", request.method().toString())
174+
.addData("url", request.getUri().toString())
175+
.addData(
176+
"headers",
177+
StreamSupport.stream(request.getHeaders().spliterator(), false)
178+
.map(h -> h.name() + "=" + h.value())
179+
.collect(Collectors.toList()));
180+
}
181+
} else {
182+
log = LOGGER.trace().setEvent("http.in.complete");
150183
}
151-
log.log();
184+
log.addContext("requestId", requestId)
185+
.addData("status", responseStatus)
186+
.log();
152187
});
153188
}
154189

src/main/java/com/arpnetworking/metrics/mad/Aggregator.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,7 @@ private List<PeriodWorker> createPeriodWorkers(final Key key) {
161161
periodWorkerList.add(periodWorker);
162162
_periodWorkerExecutor.execute(periodWorker);
163163
}
164-
LOGGER.info()
164+
LOGGER.debug()
165165
.setMessage("Created period workers")
166166
.addData("key", key)
167167
.addData("periodWorkersSize", periodWorkerList.size())

src/test/java/com/arpnetworking/configuration/jackson/HoconFileSourceTest.java

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

1818
import com.google.common.base.Charsets;
1919
import com.google.common.collect.ImmutableSet;
20+
import com.typesafe.config.impl.ConfigImpl;
2021
import org.junit.Assert;
2122
import org.junit.BeforeClass;
2223
import org.junit.Test;
@@ -36,6 +37,9 @@ public class HoconFileSourceTest {
3637
@BeforeClass
3738
public static void setUpClass() throws IOException {
3839
Files.createDirectories(new File("./target/tmp/filter/HoconFileSourceTest").toPath());
40+
System.setProperty("HoconFileSourceTest_testSystemPropertyDirect_foo", "bar");
41+
System.setProperty("HoconFileSourceTest_testSystemPropertyReference_foo", "bar");
42+
ConfigImpl.reloadSystemPropertiesConfig();
3943
}
4044

4145
@Test
@@ -70,6 +74,28 @@ public void testValidHocon() throws IOException {
7074
Assert.assertFalse(source.getValue("does-not-exist").isPresent());
7175
}
7276

77+
@Test
78+
public void testSystemPropertyDirect() throws IOException {
79+
final File file = new File("./target/tmp/filter/HoconFileSourceTest/testSystemPropertyDirect.hocon");
80+
Files.write(file.toPath(), "".getBytes(Charsets.UTF_8));
81+
final HoconFileSource source = new HoconFileSource.Builder()
82+
.setFile(file)
83+
.build();
84+
Assert.assertTrue(source.getJsonNode().isPresent());
85+
Assert.assertEquals("bar", source.getValue("HoconFileSourceTest_testSystemPropertyDirect_foo").get().textValue());
86+
}
87+
88+
@Test
89+
public void testSystemPropertyReference() throws IOException {
90+
final File file = new File("./target/tmp/filter/HoconFileSourceTest/testSystemPropertyReference.hocon");
91+
Files.write(file.toPath(), "foo:${HoconFileSourceTest_testSystemPropertyReference_foo}".getBytes(Charsets.UTF_8));
92+
final HoconFileSource source = new HoconFileSource.Builder()
93+
.setFile(file)
94+
.build();
95+
Assert.assertTrue(source.getJsonNode().isPresent());
96+
Assert.assertEquals("bar", source.getValue("foo").get().textValue());
97+
}
98+
7399
@Test(expected = RuntimeException.class)
74100
public void testInvalidHocon() throws IOException {
75101
final File file = new File("./target/tmp/filter/HoconFileSourceTest/testInvalidHocon.json");

0 commit comments

Comments
 (0)