Skip to content

Commit 28b73c0

Browse files
committed
Change code, tests, and doc to first match algorithm
1 parent 279f896 commit 28b73c0

3 files changed

Lines changed: 56 additions & 36 deletions

File tree

docs/src/main/asciidoc/includes/metrics/metrics-config.adoc

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ See the <<../../se/open-telemetry.adoc#enabling-automatic-metrics-for-inbound-ht
8989
endif::se-flavor[]
9090
9191
==== Selecting REST Endpoints for Automatic Measurement
92-
You can choose which endpoints to include in Helidon's automatic measurements using the `auto` config section.
92+
You can choose which endpoints to include in Helidon's automatic measurements using the `auto-http-metrics` config section.
9393
9494
include::{rootdir}/config/io_helidon_webserver_observe_metrics_AutoHttpMetricsConfig.adoc[tag=config,leveloffset=+2]
9595
@@ -112,20 +112,20 @@ a| Path-matching expression:
112112
113113
| `methods` | {nbsp} | all HTTP method types | Which HTTP methods match this entry
114114
115-
| `enabled` | {nbsp} | `true` | Whether this entry votes to match requests which match the entry
115+
| `enabled` | {nbsp} | `true` | Whether requests that match this entry should be measured
116116
|====
117117
--
118118
119119
Helidon decides whether to measure incoming requests as follows:
120120
121-
* If you omit the `auto` configuration, Helidon measures all endpoints.
122-
* If you specify the `auto` configuration, by default Helidon does not measure built-in endpoints such as metrics, health, and openapi. You can add items under `auto.paths` to control more exactly which endpoints to measure.
121+
* If you omit the `auto-http-metrics` configuration, Helidon measures all endpoints.
122+
* If you specify the `auto-http-metrics` configuration, by default Helidon does not measure built-in endpoints such as metrics, health, and openapi. You can add items under `auto-http-metrics.paths` to control more exactly which endpoints to measure.
123123
* If you include the `paths` section, Helidon checks a request against the path entries in order. A given request matches an entry if its path matches the path pattern and its HTTP method is in the `methods` list. If there is no `methods` list for an entry, all HTTP methods match the entry.
124-
* If a request matches an entry, the entry "votes" whether the request should be measured based on the entry's `enabled` setting.
125-
* If a request matches multiple entries, the last one voting wins.
126-
* If a request matches no entries, it is measured.
124+
* If a request matches an entry, the entry's `enabled` setting determines if the request should be measured.
125+
* If a request matches multiple entries, the first match wins.
126+
* If a request matches no entry, it is measured.
127127
128-
The `auto.sockets` setting controls which sockets are included in the measurements; if not set, Helidon measures requests on all sockets.
128+
The `auto-http-metrics.sockets` setting controls which sockets are included in the measurements; if not set, Helidon measures requests on all sockets.
129129
130130
ifdef::se-flavor[]
131131
[source,yaml]
@@ -152,13 +152,13 @@ server:
152152
endif::se-flavor[]
153153
ifdef::mp-flavor[]
154154
----
155-
server.features.observe.observers.metrics.auto.paths.0.path=/greet # <1>
156-
server.features.observe.observers.metrics.auto.paths.0.methods=GET,HEAD
155+
server.features.observe.observers.metrics.auto-http-metrics.paths.0.path=/greet # <1>
156+
server.features.observe.observers.metrics.auto-http-metrics.paths.0.methods=GET,HEAD
157157
158-
server.features.observe.observers.metrics.auto.paths.1.path=/greet/{name} # <2>
159-
server.features.observe.observers.metrics.auto.paths.1.enabled=false
158+
server.features.observe.observers.metrics.auto-http-metrics.paths.1.path=/greet/{name} # <2>
159+
server.features.observe.observers.metrics.auto-http-metrics.paths.1.enabled=false
160160
161-
server.features.observe.observers.metrics.auto.sockets=@default,private # <3>
161+
server.features.observe.observers.metrics.auto-http-metrics.sockets=@default,private # <3>
162162
----
163163
endif::mp-flavor[]
164164
<1> Measure `/greet` for only `GET` and `HEAD` requests.

webserver/observe/metrics/src/main/java/io/helidon/webserver/observe/metrics/AutoHttpMetricsConfigSupport.java

Lines changed: 9 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -54,19 +54,13 @@ static class CustomMethods {
5454
/**
5555
* Decides whether the specified HTTP method and path should be measured.
5656
* <p>
57-
* Given a request (actually, its method and path pair), Helidon searches for a matching config element
58-
* based on path matching. Each time Helidon finds a path match, it then checks the HTTP methods associated with
59-
* that entry. If the request's method appears among the config entry's methods(or if there are no methods explicitly
60-
* configured for that path element), then Helidon considers the request a match to that entry. Helidon then saves
61-
* that entry's {@code enabled} settings as the latest result for the request and goes on to check the next
62-
* configured path entry.
57+
* Given a request, we search for a path config that matches the request's path and HTTP method.
58+
* We immediately return the {@code enabled} value for the first path config that the request matches.
6359
* <p>
64-
* A given request might match any number of entries, and the last match wins.
65-
* <p>
66-
* If a request's path matches no configured entry then the request is measured, subject to the next note.
60+
* If we find no match, we return true.
6761
* <p>
6862
* Helidon automatically prefixes the explicitly-configured path entries with implicit entries for Helidon-provided
69-
* endpoints with measurement disabled. Users can enable automatic metrics for sucb an endpoint
63+
* endpoints with measurement disabled. Users can enable automatic metrics for such an endpoint
7064
* by adding explicit configuration for it with {@code enabled} set to {@code true}.
7165
*
7266
* @param config automatic metrics configuration
@@ -81,14 +75,12 @@ static boolean isMeasured(AutoHttpMetricsConfig config, Method method, UriPath u
8175
return true;
8276
}
8377

84-
boolean latestResult = true;
85-
8678
for (AutoHttpMetricsPathConfig cfg : config.effectivePathConfigs()) {
87-
if (cfg.matchesPath(uriPath)) {
88-
latestResult = cfg.matchesMethod(method) == cfg.enabled();
79+
if (cfg.matchesPath(uriPath) && cfg.matchesMethod(method)) {
80+
return cfg.enabled();
8981
}
9082
}
91-
return latestResult;
83+
return true;
9284
}
9385

9486
/**
@@ -113,8 +105,8 @@ static class BuilderDecorator implements Prototype.BuilderDecorator<AutoHttpMetr
113105
@Override
114106
public void decorate(AutoHttpMetricsConfig.BuilderBase<?, ?> target) {
115107

116-
var fullList = new ArrayList<>(MEASUREMENT_DISABLED_HELIDON_ENDPOINTS);
117-
fullList.addAll(target.autoHttpMetricsPathConfigs());
108+
var fullList = new ArrayList<>(target.autoHttpMetricsPathConfigs());
109+
fullList.addAll(MEASUREMENT_DISABLED_HELIDON_ENDPOINTS);
118110
target.effectivePathConfigs(fullList);
119111
}
120112
}

webserver/observe/metrics/src/test/java/io/helidon/webserver/observe/metrics/TestAutoMetricsConfig.java

Lines changed: 34 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import io.helidon.config.Config;
2222
import io.helidon.config.ConfigSources;
2323
import io.helidon.http.Method;
24+
import io.helidon.http.PathMatchers;
2425

2526
import org.junit.jupiter.api.Test;
2627

@@ -39,24 +40,28 @@ void testAutoMetricsConfig() {
3940
metrics:
4041
auto-http-metrics:
4142
paths:
42-
- path: "/greet"
43-
methods: ["GET","HEAD"]
4443
- path: "/greet/{name}"
4544
methods: ["GET","OPTIONS"]
45+
- path: "/greet"
46+
methods: ["GET","HEAD"]
4647
- path: "/stuff"
4748
methods: ["HEAD","OPTIONS"]
4849
enabled: false
4950
- path: "/hi"
5051
enabled: false
52+
- path: "/another/{name}"
53+
methods: ["PUT","POST","OPTIONS"]
54+
enabled: false
55+
- path: "/another/{name}"
5156
""";
52-
var config = AutoHttpMetricsConfig.create(Config.just(ConfigSources.create(configText, MediaTypes.APPLICATION_X_YAML))
53-
.get("server.features.observe.observers.metrics.auto-http-metrics"));
57+
var configFromText = Config.just(ConfigSources.create(configText, MediaTypes.APPLICATION_X_YAML))
58+
.get("server.features.observe.observers.metrics.auto-http-metrics");
59+
var config = AutoHttpMetricsConfig.create(configFromText);
5460

5561
assertThat("GET /greet", config.isMeasured(Method.GET, UriPath.create("/greet")), is(true));
56-
assertThat("PUT /greet", config.isMeasured(Method.PUT, UriPath.create("/greet")), is(false));
62+
assertThat("PUT /greet", config.isMeasured(Method.PUT, UriPath.create("/greet")), is(true));
5763

5864
assertThat("GET /greet/Joe", config.isMeasured(Method.GET, UriPath.create("/greet/Joe")), is(true));
59-
assertThat("GET /other", config.isMeasured(Method.GET, UriPath.create("/other")), is(true));
6065

6166
assertThat("GET /hi", config.isMeasured(Method.GET, UriPath.create("/hi")), is(false));
6267

@@ -65,6 +70,29 @@ void testAutoMetricsConfig() {
6570
assertThat("GET /stuff", config.isMeasured(Method.GET, UriPath.create("/stuff")), is(true));
6671
assertThat("HEAD /stuff", config.isMeasured(Method.HEAD, UriPath.create("/stuff")), is(false));
6772

73+
assertThat("PUT /another/Joe", config.isMeasured(Method.PUT, UriPath.create("/another/Joe")), is(false));
74+
assertThat("GET /another/Joe", config.isMeasured(Method.GET, UriPath.create("/another/Joe")), is(true));
75+
76+
assertThat("GET /undeclared", config.isMeasured(Method.GET, UriPath.create("/unknown")), is(true));
77+
78+
assertThat("GET /observe/metrics",
79+
config.isMeasured(Method.GET, UriPath.create("/observe/metrics")),
80+
is(false));
81+
82+
assertThat("GET /metrics", config.isMeasured(Method.GET, UriPath.create("/metrics")), is(false));
83+
84+
var withCatchAll = AutoHttpMetricsConfig.builder()
85+
.config(configFromText)
86+
.addAutoHttpMetricsPathConfig(AutoHttpMetricsPathConfig.builder()
87+
.path(PathMatchers.create("/*"))
88+
.enabled(false)
89+
.build())
90+
.build();
91+
92+
assertThat("PUT /greet with catchall", withCatchAll.isMeasured(Method.PUT, UriPath.create("/greet")), is(false));
93+
assertThat("GET /undeclared with catchall", withCatchAll.isMeasured(Method.GET, UriPath.create("/undeclared")), is(false));
94+
assertThat("GET /another/Joe", config.isMeasured(Method.GET, UriPath.create("/another/Joe")), is(true));
95+
6896
}
6997

7098
@Test

0 commit comments

Comments
 (0)