Skip to content

Commit 2ff372c

Browse files
tjquinnotjquinno
authored
Metrics and routings (helidon-io#3260)
* Handle the case where no downstream handler marks the explicit start of processing of a request * xxxSupport#postConfigureEndpoint accepts both default and endpoint routing rules; metrics impl uses them appropriately now; add mp/metrics test with metrics on its own socket Signed-off-by: tim.quinn@oracle.com <tim.quinn@oracle.com>
1 parent db8b976 commit 2ff372c

7 files changed

Lines changed: 177 additions & 37 deletions

File tree

‎integrations/micrometer/micrometer/src/main/java/io/helidon/integrations/micrometer/MicrometerSupport.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -95,12 +95,12 @@ public MeterRegistry registry() {
9595

9696
@Override
9797
public void update(Routing.Rules rules) {
98-
configureEndpoint(rules);
98+
configureEndpoint(rules, rules);
9999
}
100100

101101
@Override
102-
protected void postConfigureEndpoint(Routing.Rules rules) {
103-
rules
102+
protected void postConfigureEndpoint(Routing.Rules defaultRules, Routing.Rules serviceEndpointRoutingRules) {
103+
defaultRules
104104
.any(new MetricsContextHandler(meterRegistryFactory.meterRegistry()))
105105
.get(context(), this::getOrOptions)
106106
.options(context(), this::getOrOptions);

‎metrics/metrics/src/main/java/io/helidon/metrics/MetricsSupport.java‎

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -379,30 +379,34 @@ public void configureVendorMetrics(String routingName,
379379
* {@link #update(io.helidon.webserver.Routing.Rules)} (e.g. you should not
380380
* use both, as otherwise you would register the endpoint twice)
381381
*
382-
* @param rules routing rules (also accepts
383-
* {@link io.helidon.webserver.Routing.Builder}
382+
* @param defaultRules routing rules for default routing (also accepts {@link io.helidon.webserver.Routing.Builder})
383+
* @param serviceEndpointRoutingRules possibly different rules for the metrics endpoint routing
384384
*/
385385
@Override
386-
protected void postConfigureEndpoint(Routing.Rules rules) {
386+
protected void postConfigureEndpoint(Routing.Rules defaultRules, Routing.Rules serviceEndpointRoutingRules) {
387387
Registry base = rf.getARegistry(MetricRegistry.Type.BASE);
388388
Registry vendor = rf.getARegistry(MetricRegistry.Type.VENDOR);
389389
Registry app = rf.getARegistry(MetricRegistry.Type.APPLICATION);
390390

391391
// register the metric registry and factory to be available to all
392-
rules.any(new MetricsContextHandler(app, rf));
392+
MetricsContextHandler metricsContextHandler = new MetricsContextHandler(app, rf);
393+
defaultRules.any(metricsContextHandler);
394+
if (defaultRules != serviceEndpointRoutingRules) {
395+
serviceEndpointRoutingRules.any(metricsContextHandler);
396+
}
393397

394-
configureVendorMetrics(null, rules);
398+
configureVendorMetrics(null, defaultRules);
395399

396400
// routing to root of metrics
397-
rules.get(context(), (req, res) -> getMultiple(req, res, base, app, vendor))
401+
serviceEndpointRoutingRules.get(context(), (req, res) -> getMultiple(req, res, base, app, vendor))
398402
.options(context(), (req, res) -> optionsMultiple(req, res, base, app, vendor));
399403

400404
// routing to each scope
401405
Stream.of(app, base, vendor)
402406
.forEach(registry -> {
403407
String type = registry.type();
404408

405-
rules.get(context() + "/" + type, (req, res) -> getAll(req, res, registry))
409+
serviceEndpointRoutingRules.get(context() + "/" + type, (req, res) -> getAll(req, res, registry))
406410
.get(context() + "/" + type + "/{metric}", (req, res) -> getByName(req, res, registry))
407411
.options(context() + "/" + type, (req, res) -> optionsAll(req, res, registry))
408412
.options(context() + "/" + type + "/{metric}", (req, res) -> optionsOne(req, res, registry));
@@ -415,15 +419,15 @@ protected void postConfigureEndpoint(Routing.Rules rules) {
415419
* {@link io.helidon.webserver.Routing.Builder#register(io.helidon.webserver.Service...)}
416420
* rather than calling this method directly. If multiple sockets (and
417421
* routings) should be supported, you can use the
418-
* {@link #configureEndpoint(io.helidon.webserver.Routing.Rules)}, and
422+
* {@link #configureEndpoint(io.helidon.webserver.Routing.Rules, io.helidon.webserver.Routing.Rules)}, and
419423
* {@link #configureVendorMetrics(String, io.helidon.webserver.Routing.Rules)}
420424
* methods.
421425
*
422426
* @param rules a routing rules to update
423427
*/
424428
@Override
425429
public void update(Routing.Rules rules) {
426-
configureEndpoint(rules);
430+
configureEndpoint(rules, rules);
427431
}
428432

429433
private static KeyPerformanceIndicatorSupport.Context kpiContext(ServerRequest request) {

‎microprofile/metrics/pom.xml‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,7 @@
9494
<configuration>
9595
<excludes>
9696
<exclude>**/HelloWorldAsyncResponseTest.java</exclude>
97+
<exclude>**/TestMetricsOnOwnSocket.java</exclude>
9798
</excludes>
9899
</configuration>
99100
</execution>
@@ -106,6 +107,7 @@
106107
<configuration>
107108
<includes>
108109
<include>**/HelloWorldAsyncResponseTest.java</include>
110+
<include>**/TestMetricsOnOwnSocket.java</include>
109111
</includes>
110112
</configuration>
111113
</execution>

‎microprofile/metrics/src/test/java/io/helidon/microprofile/metrics/TestMetricsOnOwnSocket.java‎

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
/*
2+
* Copyright (c) 2021 Oracle and/or its affiliates.
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 io.helidon.microprofile.metrics;
17+
18+
import javax.inject.Inject;
19+
import javax.json.JsonObject;
20+
import javax.ws.rs.client.ClientBuilder;
21+
import javax.ws.rs.client.Invocation;
22+
import javax.ws.rs.client.WebTarget;
23+
import javax.ws.rs.core.MediaType;
24+
import javax.ws.rs.core.Response;
25+
26+
import io.helidon.common.http.Http;
27+
import io.helidon.microprofile.server.ServerCdiExtension;
28+
import io.helidon.microprofile.tests.junit5.AddConfig;
29+
import io.helidon.microprofile.tests.junit5.HelidonTest;
30+
31+
import org.junit.jupiter.api.MethodOrderer;
32+
import org.junit.jupiter.api.Order;
33+
import org.junit.jupiter.api.Test;
34+
import org.junit.jupiter.api.TestMethodOrder;
35+
36+
import static org.hamcrest.MatcherAssert.assertThat;
37+
import static org.hamcrest.Matchers.greaterThanOrEqualTo;
38+
import static org.hamcrest.Matchers.is;
39+
40+
@HelidonTest()
41+
// Set up the metrics endpoint on its own socket
42+
@AddConfig(key = "server.sockets.0.name", value = "metrics")
43+
// No port setting, so use any available one
44+
@AddConfig(key = "server.sockets.0.bind-address", value = "0.0.0.0")
45+
@AddConfig(key = "metrics.routing", value = "metrics")
46+
@AddConfig(key = "metrics.key-performance-indicators.extended", value = "true")
47+
@TestMethodOrder(MethodOrderer.OrderAnnotation.class)
48+
public class TestMetricsOnOwnSocket {
49+
50+
private Invocation metricsInvocation= null;
51+
52+
private static int loadCountBeforePingingMetrics = -1;
53+
54+
@Inject
55+
private ServerCdiExtension serverCdiExtension;
56+
57+
@Inject
58+
private WebTarget webTarget;
59+
60+
Invocation metricsInvocation() {
61+
if (metricsInvocation == null) {
62+
int metricsPort = serverCdiExtension.port("metrics");
63+
metricsInvocation = ClientBuilder.newClient()
64+
.target(String.format("http://localhost:%d/metrics/vendor", metricsPort))
65+
.request(MediaType.APPLICATION_JSON_TYPE)
66+
.buildGet();
67+
}
68+
return metricsInvocation;
69+
}
70+
71+
@Order(0)
72+
@Test
73+
void checkMetricsBeforeRequests() {
74+
// Just record the load count value. Other tests might have run earlier so we cannot assume the count is exactly 0.
75+
loadCountBeforePingingMetrics = getRequestsLoadCount("Pre-test");
76+
assertThat("Pre-test load count", loadCountBeforePingingMetrics, is(greaterThanOrEqualTo(0)));
77+
78+
}
79+
80+
@Order(1)
81+
@Test
82+
void checkMessageFromDefaultRouting() {
83+
try (Response r = webTarget
84+
.path("helloworld")
85+
.request(MediaType.TEXT_PLAIN_TYPE)
86+
.get()) {
87+
88+
assertThat("Response code getting greeting", r.getStatus(), is(Http.Status.OK_200.code()));
89+
}
90+
}
91+
92+
@Order(2)
93+
@Test
94+
void checkMetricsAfterGet() {
95+
int load = getRequestsLoadCount("Post-test");
96+
assertThat("Change in load count", load - loadCountBeforePingingMetrics, is(1));
97+
98+
}
99+
100+
private int getRequestsLoadCount(String descr) {
101+
try (Response r = metricsInvocation().invoke()) {
102+
assertThat(descr + " metrics sampling response", r.getStatus(), is(Http.Status.OK_200.code()));
103+
104+
JsonObject metrics = r.readEntity(JsonObject.class);
105+
assertThat("Check for requests.load", metrics.containsKey("requests.load"), is(true));
106+
JsonObject load = metrics.getJsonObject("requests.load");
107+
assertThat("JSON requests.load contains count", load.containsKey("count"), is(true));
108+
109+
return load.getInt("count");
110+
}
111+
}
112+
}

‎service-common/rest-cdi/src/main/java/io/helidon/servicecommon/restcdi/HelidonRestCdiExtension.java‎

Lines changed: 4 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@
4646
import javax.interceptor.Interceptor;
4747

4848
import io.helidon.config.Config;
49-
import io.helidon.config.ConfigValue;
49+
import io.helidon.microprofile.server.RoutingBuilders;
5050
import io.helidon.microprofile.server.ServerCdiExtension;
5151
import io.helidon.servicecommon.rest.HelidonRestServiceSupport;
5252
import io.helidon.webserver.Routing;
@@ -239,23 +239,11 @@ protected Routing.Builder registerService(
239239
Config config = ((Config) ConfigProvider.getConfig()).get(configPrefix);
240240
serviceSupport = serviceSupportFactory.apply(config);
241241

242-
ConfigValue<String> routingNameConfig = config.get("routing")
243-
.asString();
244-
Routing.Builder defaultRouting = server.serverRoutingBuilder();
242+
RoutingBuilders routingBuilders = RoutingBuilders.create(config);
245243

246-
Routing.Builder endpointRouting = defaultRouting;
244+
serviceSupport.configureEndpoint(routingBuilders.defaultRoutingBuilder(), routingBuilders.routingBuilder());
247245

248-
if (routingNameConfig.isPresent()) {
249-
String routingName = routingNameConfig.get();
250-
// support for overriding this back to default routing using config
251-
if (!"@default".equals(routingName)) {
252-
endpointRouting = server.serverNamedRoutingBuilder(routingName);
253-
}
254-
}
255-
256-
serviceSupport.configureEndpoint(endpointRouting);
257-
258-
return defaultRouting;
246+
return routingBuilders.defaultRoutingBuilder();
259247
}
260248

261249
protected T serviceSupport() {

‎service-common/rest/src/main/java/io/helidon/servicecommon/rest/HelidonRestServiceSupport.java‎

Lines changed: 25 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,8 @@
3535
* </ul>
3636
*
3737
* <p>
38-
* Concrete implementations must implement {@link #postConfigureEndpoint(Routing.Rules)} to do any service-specific routing.
38+
* Concrete implementations must implement {@link #postConfigureEndpoint(Routing.Rules, Routing.Rules)} to do any
39+
* service-specific routing.
3940
* See also the {@link Builder} information for possible additional overrides.
4041
* </p>
4142
*
@@ -59,28 +60,44 @@ protected HelidonRestServiceSupport(Logger logger, Builder<?, ?> builder, String
5960
corsEnabledServiceHelper = CorsEnabledServiceHelper.create(serviceName, builder.crossOriginConfig);
6061
}
6162

63+
/**
64+
* Avoid using this obsolete method. Use {@link #configureEndpoint(Routing.Rules, Routing.Rules)} instead. (Neither method
65+
* should typically invoked directly from user code.)
66+
*
67+
* @param rules routing rules (also accepts
68+
* {@link io.helidon.webserver.Routing.Builder}
69+
*/
70+
@Deprecated
71+
public final void configureEndpoint(Routing.Rules rules) {
72+
configureEndpoint(rules, rules);
73+
}
74+
6275
/**
6376
* Configures service endpoint on the provided routing rules. This method
6477
* just adds the endpoint path (as defaulted or configured).
6578
* This method is exclusive to
6679
* {@link #update(io.helidon.webserver.Routing.Rules)} (e.g. you should not
6780
* use both, as otherwise you would register the endpoint twice)
6881
*
69-
* @param rules routing rules (also accepts
70-
* {@link io.helidon.webserver.Routing.Builder}
82+
* @param defaultRules default routing rules (also accepts {@link io.helidon.webserver.Routing.Builder}
83+
* @param serviceEndpointRoutingRules actual rules (if different from default) for the service endpoint
7184
*/
72-
public final void configureEndpoint(Routing.Rules rules) {
85+
public final void configureEndpoint(Routing.Rules defaultRules, Routing.Rules serviceEndpointRoutingRules) {
7386
// CORS first
74-
rules.any(context, corsEnabledServiceHelper.processor());
75-
postConfigureEndpoint(rules);
87+
defaultRules.any(context, corsEnabledServiceHelper.processor());
88+
if (defaultRules != serviceEndpointRoutingRules) {
89+
serviceEndpointRoutingRules.any(context, corsEnabledServiceHelper.processor());
90+
}
91+
postConfigureEndpoint(defaultRules, serviceEndpointRoutingRules);
7692
}
7793

7894
/**
7995
* Concrete implementations override this method to perform any service-specific routing set-up.
8096
*
81-
* @param rules {@code Routing.Rules} to be updated
97+
* @param defaultRules default {@code Routing.Rules} to be updated
98+
* @param serviceEndpointRoutingRules actual rules (if different from the default ones) to be updated for the service endpoint
8299
*/
83-
protected abstract void postConfigureEndpoint(Routing.Rules rules);
100+
protected abstract void postConfigureEndpoint(Routing.Rules defaultRules, Routing.Rules serviceEndpointRoutingRules);
84101

85102
protected String context() {
86103
return context;

‎webserver/webserver/src/main/java/io/helidon/webserver/KeyPerformanceIndicatorContextFactory.java‎

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,15 +69,32 @@ protected Metrics kpiMetrics() {
6969
private static class DeferrableRequestContext extends ImmediateRequestContext
7070
implements KeyPerformanceIndicatorSupport.DeferrableRequestContext {
7171

72+
private boolean isStartRecorded = false;
73+
7274
@Override
7375
public void requestHandlingStarted(Metrics kpiMetrics) {
7476
kpiMetrics(kpiMetrics);
77+
recordStartTime(); // In case no handler in the chain manages the start-of-processing moment.
7578
kpiMetrics.onRequestReceived();
7679
}
7780

7881
@Override
7982
public void requestProcessingStarted() {
80-
recordStartTime();
83+
recordStartTime(); // Overwrite the previously-recorded, provisional start time, now that we have a real one.
84+
recordProcessingStarted();
85+
}
86+
87+
@Override
88+
public void requestProcessingCompleted(boolean isSuccessful) {
89+
// No handler explicitly dealt with start-of-processing, so approximate it based on request receipt time.
90+
if (!isStartRecorded) {
91+
recordProcessingStarted();
92+
}
93+
super.requestProcessingCompleted(isSuccessful);
94+
}
95+
96+
private void recordProcessingStarted() {
97+
isStartRecorded = true;
8198
Metrics kpiMetrics = kpiMetrics();
8299
if (kpiMetrics != null) {
83100
kpiMetrics().onRequestStarted();

0 commit comments

Comments
 (0)