Skip to content

Commit 2e4331f

Browse files
authored
improve code quality, part 1 (#1808)
Signed-off-by: Gregor Zeitlinger <gregor.zeitlinger@grafana.com>
1 parent 233df63 commit 2e4331f

File tree

18 files changed

+1675
-336
lines changed

18 files changed

+1675
-336
lines changed

.editorconfig

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,10 @@ root = true
44
max_line_length = 100
55
indent_size = 2
66

7-
[{version-rules.xml,maven-wrapper.properties,checkstyle.xml,docker-compose.yaml,docker-compose.yml,Dockerfile,example_target_info.json,mise.toml,mvnm,mvnw.cmd,generate-protobuf.sh,super-linter.env,.gitleaksignore,*.json5}]
7+
[{version-rules.xml,maven-wrapper.properties,checkstyle.xml,docker-compose.yaml,docker-compose.yml,Dockerfile,example_target_info.json,mise.toml,mvnm,mvnw.cmd,generate-protobuf.sh,.gitleaksignore,*.json5}]
88
max_line_length = 200
99

10-
[{grafana-dashboard-*.json,.editorconfig}]
10+
[{grafana-dashboard-*.json,.editorconfig,super-linter.env}]
1111
max_line_length = 300
1212

1313
[pom.xml]

.github/super-linter.env

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
FILTER_REGEX_EXCLUDE=mvnw|src/main/generated/.*|docs/themes/.*|keystore.pkcs12|.*.java|prometheus-metrics-exporter-opentelemetry-shaded/pom.xml|CODE_OF_CONDUCT.md
1+
FILTER_REGEX_EXCLUDE=mvnw|src/main/generated/.*|docs/themes/.*|keystore.pkcs12|.*.java|prometheus-metrics-exporter-opentelemetry-shaded/pom.xml|CODE_OF_CONDUCT.md|CLAUDE.md|CODE_QUALITY_IMPROVEMENTS.md
22
IGNORE_GITIGNORED_FILES=true
33
JAVA_FILE_NAME=google_checks.xml
44
LOG_LEVEL=ERROR

CLAUDE.md

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
# CLAUDE.md
2+
3+
This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository.
4+
5+
## Build Commands
6+
7+
This project uses Maven with mise for task automation. The Maven wrapper (`./mvnw`) is used for all builds.
8+
9+
```bash
10+
# Full CI build (clean + install + all checks)
11+
mise run ci
12+
13+
# Quick compile without tests or checks (fastest for development)
14+
mise run compile
15+
16+
# Run unit tests only (skips formatting/coverage/checkstyle checks)
17+
mise run test
18+
19+
# Run all tests including integration tests
20+
mise run test-all
21+
22+
# Format code with Google Java Format
23+
mise run format
24+
# or directly: ./mvnw spotless:apply
25+
26+
# Run a single test class
27+
./mvnw test -Dtest=CounterTest -Dspotless.check.skip=true -Dcoverage.skip=true -Dcheckstyle.skip=true
28+
29+
# Run a single test method
30+
./mvnw test -Dtest=CounterTest#testIncrement -Dspotless.check.skip=true -Dcoverage.skip=true -Dcheckstyle.skip=true
31+
32+
# Run tests in a specific module
33+
./mvnw test -pl prometheus-metrics-core -Dspotless.check.skip=true -Dcoverage.skip=true -Dcheckstyle.skip=true
34+
35+
# Regenerate protobuf classes (after protobuf dependency update)
36+
mise run generate
37+
```
38+
39+
## Architecture
40+
41+
The library follows a layered architecture where metrics flow from core types through a registry to exporters:
42+
43+
```
44+
prometheus-metrics-core (user-facing API)
45+
46+
▼ collect()
47+
prometheus-metrics-model (immutable snapshots)
48+
49+
50+
prometheus-metrics-exposition-formats (text/protobuf/OpenMetrics)
51+
52+
53+
Exporters (httpserver, servlet, pushgateway, opentelemetry)
54+
```
55+
56+
### Key Modules
57+
58+
- **prometheus-metrics-core**: User-facing metric types (Counter, Gauge, Histogram, Summary, Info, StateSet). All metrics implement the `Collector` interface with a `collect()` method.
59+
- **prometheus-metrics-model**: Internal read-only immutable snapshot types returned by `collect()`. Contains `PrometheusRegistry` for metric registration.
60+
- **prometheus-metrics-config**: Runtime configuration via properties files or system properties.
61+
- **prometheus-metrics-exposition-formats**: Converts snapshots to Prometheus exposition formats.
62+
- **prometheus-metrics-tracer**: Exemplar support with OpenTelemetry tracing integration.
63+
- **prometheus-metrics-simpleclient-bridge**: Allows legacy simpleclient 0.16.0 metrics to work with the new registry.
64+
65+
### Instrumentation Modules
66+
67+
Pre-built instrumentations: `prometheus-metrics-instrumentation-jvm`, `-caffeine`, `-guava`, `-dropwizard`, `-dropwizard5`.
68+
69+
## Code Style
70+
71+
- **Formatter**: Google Java Format (enforced via Spotless)
72+
- **Line length**: 100 characters
73+
- **Indentation**: 2 spaces
74+
- **Static analysis**: Error Prone with NullAway (`io.prometheus.metrics` package)
75+
- **Logger naming**: Logger fields must be named `logger` (not `log`, `LOG`, or `LOGGER`)
76+
- **Assertions in tests**: Use static imports from AssertJ (`import static org.assertj.core.api.Assertions.assertThat`)
77+
- **Empty catch blocks**: Use `ignored` as the exception variable name
78+
79+
## Linting and Validation
80+
81+
- **IMPORTANT**: Always run `mise run build` after modifying Java files to ensure all lints, code formatting (Spotless), static analysis (Error Prone), and checkstyle checks pass
82+
- **IMPORTANT**: Always run `mise run lint:super-linter` after modifying non-Java files (YAML, Markdown, shell scripts, JSON, etc.)
83+
- Super-linter is configured to only show ERROR-level messages via `LOG_LEVEL=ERROR` in `.github/super-linter.env`
84+
- Local super-linter version is pinned to match CI (see `.mise/tasks/lint/super-linter.sh`)
85+
86+
## Testing
87+
88+
- JUnit 5 (Jupiter) with `@Test` annotations
89+
- AssertJ for fluent assertions
90+
- Mockito for mocking
91+
- Integration tests are in `integration-tests/` and run during `verify` phase
92+
- Acceptance tests use OATs framework: `mise run acceptance-test`
93+
94+
## Java Version
95+
96+
Source compatibility: Java 8. Tests run on Java 25 (configured in `mise.toml`).

CODE_QUALITY_IMPROVEMENTS.md

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
# Code Quality Improvement Plan
2+
3+
This document tracks code quality improvements for the Prometheus Java Client library. Work through these items incrementally across sessions.
4+
5+
## High Priority
6+
7+
### 1. Add Missing Test Coverage for Exporter Modules
8+
- [x] `prometheus-metrics-exporter-common` - base module, no tests
9+
- [x] `prometheus-metrics-exporter-servlet-jakarta` - no tests
10+
- [x] `prometheus-metrics-exporter-servlet-javax` - no tests
11+
- [x] `prometheus-metrics-exporter-opentelemetry-otel-agent-resources` - no tests
12+
13+
### 2. Eliminate Dropwizard Module Duplication
14+
- [x] Create shared base class or use generics for `prometheus-metrics-instrumentation-dropwizard` and `prometheus-metrics-instrumentation-dropwizard5` (~297 lines each, nearly identical)
15+
16+
### 3. Address Technical Debt (TODOs)
17+
- [ ] `prometheus-metrics-core/src/main/java/io/prometheus/metrics/core/metrics/Histogram.java:965` - "reset interval isn't tested yet"
18+
- [ ] `prometheus-metrics-core/src/main/java/io/prometheus/metrics/core/metrics/Summary.java:205` - "Exemplars (are hard-coded as empty)"
19+
- [ ] `prometheus-metrics-core/src/main/java/io/prometheus/metrics/core/metrics/SlidingWindow.java:18` - "synchronized implementation, room for optimization"
20+
- [ ] `prometheus-metrics-config/src/main/java/io/prometheus/metrics/config/PrometheusPropertiesLoader.java:105` - "Add environment variables like EXEMPLARS_ENABLED"
21+
- [ ] `prometheus-metrics-exporter-opentelemetry/src/main/java/io/prometheus/metrics/exporter/opentelemetry/PrometheusMetricProducer.java:44` - "filter configuration for OpenTelemetry exporter"
22+
- [ ] `prometheus-metrics-config/src/main/java/io/prometheus/metrics/config/ExporterOpenTelemetryProperties.java:7` - "JavaDoc missing"
23+
24+
### 4. Improve Exception Handling
25+
Replace broad `catch (Exception e)` with specific exception types:
26+
- [ ] `prometheus-metrics-instrumentation-dropwizard5/src/main/java/.../DropwizardExports.java:237`
27+
- [ ] `prometheus-metrics-instrumentation-caffeine/src/main/java/.../CacheMetricsCollector.java:229`
28+
- [ ] `prometheus-metrics-exporter-opentelemetry/src/main/java/.../PrometheusInstrumentationScope.java:47`
29+
- [ ] `prometheus-metrics-exporter-opentelemetry/src/main/java/.../OtelAutoConfig.java:115`
30+
- [ ] `prometheus-metrics-instrumentation-jvm/src/main/java/.../JvmNativeMemoryMetrics.java:166`
31+
- [ ] `prometheus-metrics-exporter-httpserver/src/main/java/.../HttpExchangeAdapter.java:115`
32+
33+
## Medium Priority
34+
35+
### 5. Add Branch Coverage to JaCoCo
36+
- [ ] Update `pom.xml` to add branch coverage requirement (~50% minimum)
37+
```xml
38+
<limit>
39+
<counter>BRANCH</counter>
40+
<value>COVEREDRATIO</value>
41+
<minimum>0.50</minimum>
42+
</limit>
43+
```
44+
45+
### 6. Raise Minimum Coverage Thresholds
46+
Current thresholds to review:
47+
- [ ] `prometheus-metrics-exporter-httpserver` - 45% (raise to 60%)
48+
- [ ] `prometheus-metrics-instrumentation-dropwizard5` - 50% (raise to 60%)
49+
- [ ] `prometheus-metrics-exposition-textformats` - 50% (raise to 60%)
50+
- [ ] `prometheus-metrics-instrumentation-jvm` - 55% (raise to 60%)
51+
52+
### 7. Add SpotBugs
53+
- [ ] Add `spotbugs-maven-plugin` to `pom.xml`
54+
- [ ] Configure with appropriate rule set
55+
56+
### 8. Narrow Checkstyle Suppressions
57+
- [ ] Review `checkstyle-suppressions.xml` - currently suppresses ALL Javadoc checks globally
58+
- [ ] Narrow to specific packages/classes that need exceptions
59+
60+
## Lower Priority
61+
62+
### 9. Refactor Large Classes
63+
- [ ] `prometheus-metrics-core/src/main/java/.../Histogram.java` (978 lines) - consider extracting native histogram logic
64+
65+
### 10. Document Configuration Classes
66+
- [ ] `PrometheusPropertiesLoader` - add JavaDoc
67+
- [ ] `ExporterProperties` and related classes - add JavaDoc
68+
- [ ] `ExporterOpenTelemetryProperties` - add JavaDoc (noted in TODO)
69+
70+
### 11. Consolidate Servlet Exporter Duplication
71+
- [ ] Extract common logic from `servlet-jakarta` and `servlet-javax` into `exporter-common`
72+
73+
### 12. Add Mutation Testing
74+
- [ ] Add Pitest (`pitest-maven`) for critical modules
75+
- [ ] Start with `prometheus-metrics-core` and `prometheus-metrics-model`
76+
77+
---
78+
79+
## Progress Notes
80+
81+
_Add notes here as items are completed:_
82+
83+
| Date | Item | Notes |
84+
|------|------|-------|
85+
| 2026-01-24 | Missing Test Coverage for Exporter Modules | Added 55 tests across 4 modules: exporter-common (22 tests), servlet-jakarta (14 tests), servlet-javax (14 tests), otel-agent-resources (5 tests). All tests passing. |
86+
| 2026-01-24 | Eliminate Dropwizard Module Duplication | Created AbstractDropwizardExports base class (267 lines) with generic type parameters. Reduced dropwizard module from 297 to 209 lines (-88 lines, -30%), dropwizard5 module from 297 to 212 lines (-85 lines, -29%). All tests passing (32 tests dropwizard5, 13 tests dropwizard). |

prometheus-metrics-exporter-common/pom.xml

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,14 @@
3838
<version>${project.version}</version>
3939
<scope>runtime</scope>
4040
</dependency>
41+
42+
<!-- test dependencies -->
43+
<dependency>
44+
<groupId>io.prometheus</groupId>
45+
<artifactId>prometheus-metrics-core</artifactId>
46+
<version>${project.version}</version>
47+
<scope>test</scope>
48+
</dependency>
4149
</dependencies>
4250

4351
</project>
Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,120 @@
1+
package io.prometheus.metrics.exporter.common;
2+
3+
import static org.assertj.core.api.Assertions.assertThat;
4+
5+
import java.util.Collections;
6+
import java.util.Enumeration;
7+
import org.junit.jupiter.api.Test;
8+
9+
class PrometheusHttpRequestTest {
10+
11+
@Test
12+
public void testGetHeaderReturnsFirstValue() {
13+
PrometheusHttpRequest request =
14+
new TestPrometheusHttpRequest("name[]=metric1&name[]=metric2", "gzip");
15+
assertThat(request.getHeader("Accept-Encoding")).isEqualTo("gzip");
16+
}
17+
18+
@Test
19+
public void testGetHeaderReturnsNullWhenNoHeaders() {
20+
PrometheusHttpRequest request = new TestPrometheusHttpRequest("", null);
21+
assertThat(request.getHeader("Accept-Encoding")).isNull();
22+
}
23+
24+
@Test
25+
public void testGetParameterReturnsFirstValue() {
26+
PrometheusHttpRequest request = new TestPrometheusHttpRequest("name[]=metric1&name[]=metric2");
27+
assertThat(request.getParameter("name[]")).isEqualTo("metric1");
28+
}
29+
30+
@Test
31+
public void testGetParameterReturnsNullWhenNotPresent() {
32+
PrometheusHttpRequest request = new TestPrometheusHttpRequest("other=value");
33+
assertThat(request.getParameter("name[]")).isNull();
34+
}
35+
36+
@Test
37+
public void testGetParameterValuesReturnsMultipleValues() {
38+
PrometheusHttpRequest request = new TestPrometheusHttpRequest("name[]=metric1&name[]=metric2");
39+
String[] values = request.getParameterValues("name[]");
40+
assertThat(values).containsExactly("metric1", "metric2");
41+
}
42+
43+
@Test
44+
public void testGetParameterValuesReturnsNullWhenNotPresent() {
45+
PrometheusHttpRequest request = new TestPrometheusHttpRequest("other=value");
46+
assertThat(request.getParameterValues("name[]")).isNull();
47+
}
48+
49+
@Test
50+
public void testGetParameterValuesWithEmptyQueryString() {
51+
PrometheusHttpRequest request = new TestPrometheusHttpRequest("");
52+
assertThat(request.getParameterValues("name[]")).isNull();
53+
}
54+
55+
@Test
56+
public void testGetParameterValuesWithNullQueryString() {
57+
PrometheusHttpRequest request = new TestPrometheusHttpRequest(null);
58+
assertThat(request.getParameterValues("name[]")).isNull();
59+
}
60+
61+
@Test
62+
public void testGetParameterValuesWithUrlEncodedValues() {
63+
PrometheusHttpRequest request = new TestPrometheusHttpRequest("name=hello%20world");
64+
String[] values = request.getParameterValues("name");
65+
assertThat(values).containsExactly("hello world");
66+
}
67+
68+
@Test
69+
public void testGetParameterValuesWithSpecialCharacters() {
70+
PrometheusHttpRequest request = new TestPrometheusHttpRequest("name=%2Ffoo%2Fbar");
71+
String[] values = request.getParameterValues("name");
72+
assertThat(values).containsExactly("/foo/bar");
73+
}
74+
75+
@Test
76+
public void testGetParameterValuesIgnoresParametersWithoutEquals() {
77+
PrometheusHttpRequest request =
78+
new TestPrometheusHttpRequest("name[]=value1&invalid&name[]=value2");
79+
String[] values = request.getParameterValues("name[]");
80+
assertThat(values).containsExactly("value1", "value2");
81+
}
82+
83+
/** Test implementation of PrometheusHttpRequest for testing default methods. */
84+
private static class TestPrometheusHttpRequest implements PrometheusHttpRequest {
85+
private final String queryString;
86+
private final String acceptEncoding;
87+
88+
TestPrometheusHttpRequest(String queryString) {
89+
this(queryString, null);
90+
}
91+
92+
TestPrometheusHttpRequest(String queryString, String acceptEncoding) {
93+
this.queryString = queryString;
94+
this.acceptEncoding = acceptEncoding;
95+
}
96+
97+
@Override
98+
public String getQueryString() {
99+
return queryString;
100+
}
101+
102+
@Override
103+
public Enumeration<String> getHeaders(String name) {
104+
if (acceptEncoding != null && name.equals("Accept-Encoding")) {
105+
return Collections.enumeration(Collections.singletonList(acceptEncoding));
106+
}
107+
return null;
108+
}
109+
110+
@Override
111+
public String getMethod() {
112+
return "GET";
113+
}
114+
115+
@Override
116+
public String getRequestPath() {
117+
return "/metrics";
118+
}
119+
}
120+
}

0 commit comments

Comments
 (0)