Skip to content

Commit 5cfa5c0

Browse files
zeitlingerzeitlinger
authored
add branch coverage, better javadoc lints (#1851)
Signed-off-by: Gregor Zeitlinger <gregor.zeitlinger@grafana.com>
1 parent 801277d commit 5cfa5c0

File tree

21 files changed

+469
-16
lines changed

21 files changed

+469
-16
lines changed

‎checkstyle-suppressions.xml‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,5 +5,20 @@
55
"https://checkstyle.org/dtds/suppressions_1_2.dtd">
66

77
<suppressions>
8-
<suppress checks="Javadoc" files="."/>
8+
<!-- Suppress Javadoc paragraph formatting (empty line before <p> tag) globally. This is a
9+
stylistic check that would require fixing hundreds of locations. We still enforce that
10+
public APIs have documentation via other Javadoc checks. -->
11+
<suppress checks="JavadocParagraph" files="."/>
12+
13+
<!-- Suppress all Javadoc checks for internal implementation packages -->
14+
<suppress checks="Javadoc" files=".*[/\\]internal[/\\].*"/>
15+
16+
<!-- Suppress all Javadoc checks for example code -->
17+
<suppress checks="Javadoc" files=".*[/\\]examples[/\\].*"/>
18+
19+
<!-- Suppress all Javadoc checks for benchmark code -->
20+
<suppress checks="Javadoc" files=".*[/\\]benchmarks[/\\].*"/>
21+
22+
<!-- Suppress all Javadoc checks for integration tests -->
23+
<suppress checks="Javadoc" files=".*[/\\]integration-tests[/\\].*"/>
924
</suppressions>

‎pom.xml‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -234,6 +234,8 @@
234234
<excludes>
235235
<exclude>**/generated/**</exclude>
236236
<exclude>**/*BlockingRejectedExecutionHandler*</exclude>
237+
<exclude>**/*AllocationCountingNotificationListener*</exclude>
238+
<exclude>**/*MapperConfig*</exclude>
237239
</excludes>
238240
</configuration>
239241
<executions>
@@ -264,6 +266,11 @@
264266
<value>COVEREDRATIO</value>
265267
<minimum>${jacoco.line-coverage}</minimum>
266268
</limit>
269+
<limit>
270+
<counter>BRANCH</counter>
271+
<value>COVEREDRATIO</value>
272+
<minimum>0.50</minimum>
273+
</limit>
267274
</limits>
268275
</rule>
269276
</rules>

‎prometheus-metrics-config/src/test/java/io/prometheus/metrics/config/ExporterOpenTelemetryPropertiesTest.java‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package io.prometheus.metrics.config;
22

33
import static org.assertj.core.api.Assertions.assertThat;
4+
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
45

56
import java.util.HashMap;
67
import java.util.Map;
@@ -68,4 +69,40 @@ void builder() {
6869
.build();
6970
assertValues(properties);
7071
}
72+
73+
@Test
74+
void builderWithHttpProtobuf() {
75+
ExporterOpenTelemetryProperties properties =
76+
ExporterOpenTelemetryProperties.builder().protocol("http/protobuf").build();
77+
assertThat(properties.getProtocol()).isEqualTo("http/protobuf");
78+
}
79+
80+
@Test
81+
void builderWithInvalidProtocol() {
82+
assertThatExceptionOfType(IllegalArgumentException.class)
83+
.isThrownBy(() -> ExporterOpenTelemetryProperties.builder().protocol("invalid"))
84+
.withMessage("invalid: Unsupported protocol. Expecting grpc or http/protobuf");
85+
}
86+
87+
@Test
88+
void builderWithInvalidIntervalSeconds() {
89+
assertThatExceptionOfType(IllegalArgumentException.class)
90+
.isThrownBy(() -> ExporterOpenTelemetryProperties.builder().intervalSeconds(0))
91+
.withMessage("0: Expecting intervalSeconds > 0");
92+
93+
assertThatExceptionOfType(IllegalArgumentException.class)
94+
.isThrownBy(() -> ExporterOpenTelemetryProperties.builder().intervalSeconds(-1))
95+
.withMessage("-1: Expecting intervalSeconds > 0");
96+
}
97+
98+
@Test
99+
void builderWithInvalidTimeoutSeconds() {
100+
assertThatExceptionOfType(IllegalArgumentException.class)
101+
.isThrownBy(() -> ExporterOpenTelemetryProperties.builder().timeoutSeconds(0))
102+
.withMessage("0: Expecting timeoutSeconds > 0");
103+
104+
assertThatExceptionOfType(IllegalArgumentException.class)
105+
.isThrownBy(() -> ExporterOpenTelemetryProperties.builder().timeoutSeconds(-1))
106+
.withMessage("-1: Expecting timeoutSeconds > 0");
107+
}
71108
}

‎prometheus-metrics-config/src/test/java/io/prometheus/metrics/config/ExporterPropertiesTest.java‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,5 +56,35 @@ void builder() {
5656
.build();
5757
assertThat(properties.getIncludeCreatedTimestamps()).isTrue();
5858
assertThat(properties.getExemplarsOnAllMetricTypes()).isTrue();
59+
assertThat(properties.getPrometheusTimestampsInMs()).isFalse();
60+
}
61+
62+
@Test
63+
void defaultValues() {
64+
ExporterProperties properties = ExporterProperties.builder().build();
65+
assertThat(properties.getIncludeCreatedTimestamps()).isFalse();
66+
assertThat(properties.getExemplarsOnAllMetricTypes()).isFalse();
67+
assertThat(properties.getPrometheusTimestampsInMs()).isFalse();
68+
}
69+
70+
@Test
71+
void prometheusTimestampsInMs() {
72+
ExporterProperties properties =
73+
ExporterProperties.builder().prometheusTimestampsInMs(true).build();
74+
assertThat(properties.getPrometheusTimestampsInMs()).isTrue();
75+
76+
properties =
77+
load(new HashMap<>(Map.of("io.prometheus.exporter.prometheus_timestamps_in_ms", "true")));
78+
assertThat(properties.getPrometheusTimestampsInMs()).isTrue();
79+
80+
assertThatExceptionOfType(PrometheusPropertiesException.class)
81+
.isThrownBy(
82+
() ->
83+
load(
84+
new HashMap<>(
85+
Map.of("io.prometheus.exporter.prometheus_timestamps_in_ms", "invalid"))))
86+
.withMessage(
87+
"io.prometheus.exporter.prometheus_timestamps_in_ms: Expecting 'true' or 'false'. Found:"
88+
+ " invalid");
5989
}
6090
}

‎prometheus-metrics-config/src/test/java/io/prometheus/metrics/config/ExporterPushgatewayPropertiesTest.java‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,50 @@ void load() {
3030
+ " Found: foo");
3131
}
3232

33+
@Test
34+
void loadWithHttps() {
35+
ExporterPushgatewayProperties properties =
36+
load(Map.of("io.prometheus.exporter.pushgateway.scheme", "https"));
37+
assertThat(properties.getScheme()).isEqualTo("https");
38+
}
39+
40+
@Test
41+
void loadWithEscapingSchemes() {
42+
ExporterPushgatewayProperties properties =
43+
load(Map.of("io.prometheus.exporter.pushgateway.escaping_scheme", "allow-utf-8"));
44+
assertThat(properties.getEscapingScheme()).isEqualTo(EscapingScheme.ALLOW_UTF8);
45+
46+
properties = load(Map.of("io.prometheus.exporter.pushgateway.escaping_scheme", "values"));
47+
assertThat(properties.getEscapingScheme()).isEqualTo(EscapingScheme.VALUE_ENCODING_ESCAPING);
48+
49+
properties = load(Map.of("io.prometheus.exporter.pushgateway.escaping_scheme", "underscores"));
50+
assertThat(properties.getEscapingScheme()).isEqualTo(EscapingScheme.UNDERSCORE_ESCAPING);
51+
52+
properties = load(Map.of("io.prometheus.exporter.pushgateway.escaping_scheme", "dots"));
53+
assertThat(properties.getEscapingScheme()).isEqualTo(EscapingScheme.DOTS_ESCAPING);
54+
}
55+
56+
@Test
57+
void loadWithInvalidEscapingScheme() {
58+
assertThatExceptionOfType(PrometheusPropertiesException.class)
59+
.isThrownBy(
60+
() -> load(Map.of("io.prometheus.exporter.pushgateway.escaping_scheme", "invalid")))
61+
.withMessage(
62+
"io.prometheus.exporter.pushgateway.escaping_scheme: Illegal value. Expecting"
63+
+ " 'allow-utf-8', 'values', 'underscores', or 'dots'. Found: invalid");
64+
}
65+
66+
@Test
67+
void loadWithTimeouts() {
68+
ExporterPushgatewayProperties properties =
69+
load(
70+
Map.of(
71+
"io.prometheus.exporter.pushgateway.connect_timeout_seconds", "5",
72+
"io.prometheus.exporter.pushgateway.read_timeout_seconds", "10"));
73+
assertThat(properties.getConnectTimeout()).isEqualTo(Duration.ofSeconds(5));
74+
assertThat(properties.getReadTimeout()).isEqualTo(Duration.ofSeconds(10));
75+
}
76+
3377
private static ExporterPushgatewayProperties load(Map<String, String> map) {
3478
Map<Object, Object> regularProperties = new HashMap<>(map);
3579
PropertySource propertySource = new PropertySource(regularProperties);

‎prometheus-metrics-config/src/test/java/io/prometheus/metrics/config/PrometheusPropertiesTest.java‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,4 +70,62 @@ void testBuilder() {
7070
assertThat(result.getMetricProperties("unknown_metric")).isNull();
7171
assertThat(result.getExporterProperties()).isSameAs(defaults.getExporterProperties());
7272
}
73+
74+
@Test
75+
void testMetricNameNormalization() {
76+
PrometheusProperties.Builder builder = PrometheusProperties.builder();
77+
MetricsProperties customProps =
78+
MetricsProperties.builder().histogramClassicUpperBounds(0.1, 0.5).build();
79+
80+
// Test that metric names with dots are normalized to underscores
81+
builder.putMetricProperty("my.metric.name", customProps);
82+
PrometheusProperties result = builder.build();
83+
84+
// Should be able to retrieve with dots
85+
assertThat(result.getMetricProperties("my.metric.name")).isSameAs(customProps);
86+
// Should also be able to retrieve with underscores
87+
assertThat(result.getMetricProperties("my_metric_name")).isSameAs(customProps);
88+
}
89+
90+
@Test
91+
void testMetricNameWithInvalidCharacters() {
92+
PrometheusProperties.Builder builder = PrometheusProperties.builder();
93+
MetricsProperties customProps =
94+
MetricsProperties.builder().histogramClassicUpperBounds(0.1, 0.5).build();
95+
96+
// Test that invalid characters are converted to underscores
97+
builder.putMetricProperty("metric-name@with#invalid$chars", customProps);
98+
PrometheusProperties result = builder.build();
99+
100+
// Should normalize invalid characters to underscores
101+
assertThat(result.getMetricProperties("metric-name@with#invalid$chars")).isSameAs(customProps);
102+
assertThat(result.getMetricProperties("metric_name_with_invalid_chars")).isSameAs(customProps);
103+
}
104+
105+
@Test
106+
void testMetricNameWithValidCharacters() {
107+
PrometheusProperties.Builder builder = PrometheusProperties.builder();
108+
MetricsProperties customProps =
109+
MetricsProperties.builder().histogramClassicUpperBounds(0.1, 0.5).build();
110+
111+
// Test valid characters: letters, numbers (not at start), underscore, colon
112+
builder.putMetricProperty("my_metric:name123", customProps);
113+
PrometheusProperties result = builder.build();
114+
115+
assertThat(result.getMetricProperties("my_metric:name123")).isSameAs(customProps);
116+
}
117+
118+
@Test
119+
void testMetricNameStartingWithNumber() {
120+
PrometheusProperties.Builder builder = PrometheusProperties.builder();
121+
MetricsProperties customProps =
122+
MetricsProperties.builder().histogramClassicUpperBounds(0.1, 0.5).build();
123+
124+
// First digit is invalid (i=0), but subsequent digits are valid (i>0)
125+
builder.putMetricProperty("123metric", customProps);
126+
PrometheusProperties result = builder.build();
127+
128+
assertThat(result.getMetricProperties("123metric")).isSameAs(customProps);
129+
assertThat(result.getMetricProperties("_23metric")).isSameAs(customProps);
130+
}
73131
}

‎prometheus-metrics-exporter-httpserver/pom.xml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919

2020
<properties>
2121
<automatic.module.name>io.prometheus.metrics.exporter.httpserver</automatic.module.name>
22-
<jacoco.line-coverage>0.45</jacoco.line-coverage>
22+
<jacoco.line-coverage>0.60</jacoco.line-coverage>
2323
</properties>
2424

2525
<dependencies>

‎prometheus-metrics-exporter-httpserver/src/test/java/io/prometheus/metrics/exporter/httpserver/HTTPServerTest.java‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,29 @@ public Result authenticate(HttpExchange exchange) {
8585
run(server, "/", 204, "");
8686
}
8787

88+
@Test
89+
void testSubjectDoAsWithInvalidSubject() throws Exception {
90+
Authenticator authenticator =
91+
new Authenticator() {
92+
@Override
93+
public Result authenticate(HttpExchange exchange) {
94+
exchange.setAttribute("aa", "not-a-subject");
95+
return new Success(new HttpPrincipal("user", "/"));
96+
}
97+
};
98+
99+
HttpHandler handler = exchange -> exchange.sendResponseHeaders(204, -1);
100+
HTTPServer server =
101+
HTTPServer.builder()
102+
.port(0)
103+
.authenticator(authenticator)
104+
.defaultHandler(handler)
105+
.authenticatedSubjectAttributeName("aa")
106+
.buildAndStart();
107+
108+
run(server, "/", 403, "");
109+
}
110+
88111
@Test
89112
void defaultHandler() throws Exception {
90113
run(

‎prometheus-metrics-exporter-httpserver/src/test/java/io/prometheus/metrics/exporter/httpserver/HttpExchangeAdapterTest.java‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
package io.prometheus.metrics.exporter.httpserver;
2+
3+
import static org.assertj.core.api.Assertions.assertThat;
4+
import static org.mockito.Mockito.mock;
5+
import static org.mockito.Mockito.when;
6+
7+
import com.sun.net.httpserver.Headers;
8+
import com.sun.net.httpserver.HttpExchange;
9+
import java.net.URI;
10+
import java.util.List;
11+
import org.junit.jupiter.api.Test;
12+
13+
class HttpExchangeAdapterTest {
14+
15+
@Test
16+
void getRequestPath() {
17+
HttpExchange httpExchange = mock(HttpExchange.class);
18+
when(httpExchange.getRequestURI()).thenReturn(URI.create("/metrics?name=test"));
19+
HttpExchangeAdapter adapter = new HttpExchangeAdapter(httpExchange);
20+
assertThat(adapter.getRequest().getRequestPath()).isEqualTo("/metrics");
21+
}
22+
23+
@Test
24+
void getRequestPathWithoutQueryString() {
25+
HttpExchange httpExchange = mock(HttpExchange.class);
26+
when(httpExchange.getRequestURI()).thenReturn(URI.create("/metrics"));
27+
HttpExchangeAdapter adapter = new HttpExchangeAdapter(httpExchange);
28+
assertThat(adapter.getRequest().getRequestPath()).isEqualTo("/metrics");
29+
}
30+
31+
@Test
32+
void getHeadersWhenPresent() {
33+
HttpExchange httpExchange = mock(HttpExchange.class);
34+
Headers headers = new Headers();
35+
headers.put("Accept", List.of("text/plain"));
36+
when(httpExchange.getRequestHeaders()).thenReturn(headers);
37+
HttpExchangeAdapter adapter = new HttpExchangeAdapter(httpExchange);
38+
assertThat(adapter.getRequest().getHeaders("Accept").nextElement()).isEqualTo("text/plain");
39+
}
40+
41+
@Test
42+
void getHeadersWhenNotPresent() {
43+
HttpExchange httpExchange = mock(HttpExchange.class);
44+
Headers headers = new Headers();
45+
when(httpExchange.getRequestHeaders()).thenReturn(headers);
46+
HttpExchangeAdapter adapter = new HttpExchangeAdapter(httpExchange);
47+
assertThat(adapter.getRequest().getHeaders("Accept").hasMoreElements()).isFalse();
48+
}
49+
}

0 commit comments

Comments
 (0)