Skip to content

Commit 0f11ca9

Browse files
committed
refactor: make HTTP error policy builder-based
Signed-off-by: Gregor Zeitlinger <gregor.zeitlinger@grafana.com>
1 parent 9ae3d4c commit 0f11ca9

8 files changed

Lines changed: 105 additions & 63 deletions

File tree

docs/apidiffs/current_vs_latest/prometheus-metrics-exporter-httpserver.txt

Lines changed: 7 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

docs/content/exporters/httpserver.md

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -38,18 +38,28 @@ Configure a reporter to send exception details to an appropriate logging or tele
3838
HTTPServer server = HTTPServer.builder()
3939
.port(9400)
4040
.errorHandlingPolicy(
41-
HttpErrorHandlingPolicy.genericResponseWithReporter(
42-
error -> logger.log(Level.SEVERE, "Prometheus scrape failed", error)))
41+
HttpErrorHandlingPolicy.builder()
42+
.errorReporter(error -> logger.log(Level.SEVERE, "Prometheus scrape failed", error))
43+
.build())
4344
.buildAndStart();
4445
```
4546

4647
The reporter runs synchronously on the request thread and may be called concurrently. Reporter
4748
runtime exceptions do not prevent the generic HTTP 500 response from being sent. Rate limiting
4849
or deduplication can be implemented in the reporter when needed.
4950

50-
`HttpErrorHandlingPolicy.legacyDetailedResponse()` restores the previous response containing the
51-
full exception stack trace. This can disclose application internals and must not be used for an
52-
endpoint reachable by untrusted clients.
51+
For local debugging, an unsafe response containing the full exception stack trace can be enabled
52+
explicitly:
53+
54+
```java
55+
HttpErrorHandlingPolicy.builder()
56+
.unsafeDebugResponse(true)
57+
.build()
58+
```
59+
60+
This setting is independent of the error reporter, so both can be configured when needed. The
61+
unsafe debug response can disclose application internals and must not be enabled for an endpoint
62+
reachable by untrusted clients.
5363

5464
## Authentication and HTTPS
5565

prometheus-metrics-exporter-httpserver/src/main/java/io/prometheus/metrics/exporter/httpserver/HTTPServer.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -212,7 +212,7 @@ public static class Builder {
212212
@Nullable private HttpHandler defaultHandler = null;
213213
@Nullable private String metricsHandlerPath = null;
214214
@Nullable private Boolean registerHealthHandler = null;
215-
private HttpErrorHandlingPolicy errorHandlingPolicy = HttpErrorHandlingPolicy.genericResponse();
215+
private HttpErrorHandlingPolicy errorHandlingPolicy = HttpErrorHandlingPolicy.builder().build();
216216

217217
private Builder(PrometheusProperties config) {
218218
this.config = config;
@@ -301,7 +301,7 @@ public Builder registerHealthHandler(boolean registerHealthHandler) {
301301
* Configure how exceptions raised while scraping metrics are reported to the client and
302302
* optionally to a caller-supplied diagnostic sink.
303303
*
304-
* <p>Default is {@link HttpErrorHandlingPolicy#genericResponse()}.
304+
* <p>Default is {@code HttpErrorHandlingPolicy.builder().build()}.
305305
*/
306306
public Builder errorHandlingPolicy(HttpErrorHandlingPolicy errorHandlingPolicy) {
307307
if (errorHandlingPolicy == null) {

prometheus-metrics-exporter-httpserver/src/main/java/io/prometheus/metrics/exporter/httpserver/HttpErrorHandlingPolicy.java

Lines changed: 53 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,9 @@
1010
/**
1111
* Controls how the {@link HTTPServer} handles exceptions raised while scraping metrics.
1212
*
13-
* <p>The default policy is {@link #genericResponse()}, which does not expose exception details and
14-
* does not report the exception. Use {@link #genericResponseWithReporter(Consumer)} to route
15-
* diagnostic details to an application-appropriate sink.
13+
* <p>The default policy built by {@link #builder()} does not expose exception details and does not
14+
* report the exception. Configure the builder to route diagnostic details to an
15+
* application-appropriate sink.
1616
*/
1717
@StableApi
1818
public final class HttpErrorHandlingPolicy {
@@ -22,54 +22,28 @@ public final class HttpErrorHandlingPolicy {
2222
+ "Configure an HTTP error reporter for details.\n")
2323
.getBytes(StandardCharsets.UTF_8);
2424

25-
private final boolean detailedResponse;
25+
private final boolean unsafeDebugResponse;
2626
@Nullable private final Consumer<Throwable> errorReporter;
2727

2828
private HttpErrorHandlingPolicy(
29-
boolean detailedResponse, @Nullable Consumer<Throwable> errorReporter) {
30-
this.detailedResponse = detailedResponse;
29+
boolean unsafeDebugResponse, @Nullable Consumer<Throwable> errorReporter) {
30+
this.unsafeDebugResponse = unsafeDebugResponse;
3131
this.errorReporter = errorReporter;
3232
}
3333

3434
/**
35-
* Returns the secure default policy.
35+
* Returns a builder for configuring scrape error handling.
3636
*
37-
* <p>Scrape exceptions produce a generic HTTP 500 response and are not reported. This avoids
37+
* <p>The builder defaults to a generic HTTP 500 response with no error reporter. This avoids
3838
* exposing exception details to scrape clients or adding an implicit dependency on an
3939
* application's logging configuration.
4040
*/
41-
public static HttpErrorHandlingPolicy genericResponse() {
42-
return new HttpErrorHandlingPolicy(false, null);
43-
}
44-
45-
/**
46-
* Returns a policy that produces a generic HTTP 500 response and passes scrape exceptions to
47-
* {@code errorReporter}.
48-
*
49-
* <p>The reporter runs synchronously on the HTTP request thread. It should return promptly and
50-
* must be safe to call concurrently. Runtime exceptions thrown by the reporter are isolated from
51-
* HTTP response handling.
52-
*/
53-
public static HttpErrorHandlingPolicy genericResponseWithReporter(
54-
Consumer<Throwable> errorReporter) {
55-
if (errorReporter == null) {
56-
throw new NullPointerException("errorReporter");
57-
}
58-
return new HttpErrorHandlingPolicy(false, errorReporter);
59-
}
60-
61-
/**
62-
* Returns a policy that includes the full exception stack trace in the HTTP 500 response.
63-
*
64-
* <p><strong>Security warning:</strong> This legacy behavior exposes internal exception
65-
* information to scrape clients. Do not use it for endpoints reachable by untrusted clients.
66-
*/
67-
public static HttpErrorHandlingPolicy legacyDetailedResponse() {
68-
return new HttpErrorHandlingPolicy(true, null);
41+
public static Builder builder() {
42+
return new Builder();
6943
}
7044

7145
byte[] getErrorResponse(Exception exception) {
72-
if (!detailedResponse) {
46+
if (!unsafeDebugResponse) {
7347
return GENERIC_RESPONSE;
7448
}
7549
StringWriter stringWriter = new StringWriter();
@@ -84,4 +58,46 @@ void report(Throwable error) {
8458
errorReporter.accept(error);
8559
}
8660
}
61+
62+
/** Builder for {@link HttpErrorHandlingPolicy}. */
63+
public static final class Builder {
64+
65+
private boolean unsafeDebugResponse = false;
66+
@Nullable private Consumer<Throwable> errorReporter;
67+
68+
private Builder() {}
69+
70+
/**
71+
* Pass scrape exceptions to {@code errorReporter}.
72+
*
73+
* <p>The reporter runs synchronously on the HTTP request thread. It should return promptly and
74+
* must be safe to call concurrently. Runtime exceptions thrown by the reporter are isolated
75+
* from HTTP response handling.
76+
*/
77+
public Builder errorReporter(Consumer<Throwable> errorReporter) {
78+
if (errorReporter == null) {
79+
throw new NullPointerException("errorReporter");
80+
}
81+
this.errorReporter = errorReporter;
82+
return this;
83+
}
84+
85+
/**
86+
* Configure whether the HTTP 500 response includes the full exception stack trace.
87+
*
88+
* <p><strong>Security warning:</strong> Setting this to {@code true} exposes internal exception
89+
* information to scrape clients. Do not enable it for endpoints reachable by untrusted clients.
90+
*
91+
* <p>This setting is independent of {@link #errorReporter(Consumer)}.
92+
*/
93+
public Builder unsafeDebugResponse(boolean unsafeDebugResponse) {
94+
this.unsafeDebugResponse = unsafeDebugResponse;
95+
return this;
96+
}
97+
98+
/** Build the policy. */
99+
public HttpErrorHandlingPolicy build() {
100+
return new HttpErrorHandlingPolicy(unsafeDebugResponse, errorReporter);
101+
}
102+
}
87103
}

prometheus-metrics-exporter-httpserver/src/main/java/io/prometheus/metrics/exporter/httpserver/HttpExchangeAdapter.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ public class HttpExchangeAdapter implements PrometheusHttpExchange {
2424
private volatile boolean responseSent = false;
2525

2626
public HttpExchangeAdapter(HttpExchange httpExchange) {
27-
this(httpExchange, HttpErrorHandlingPolicy.genericResponse());
27+
this(httpExchange, HttpErrorHandlingPolicy.builder().build());
2828
}
2929

3030
HttpExchangeAdapter(HttpExchange httpExchange, HttpErrorHandlingPolicy errorHandlingPolicy) {

prometheus-metrics-exporter-httpserver/src/main/java/io/prometheus/metrics/exporter/httpserver/MetricsHandler.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,19 +16,19 @@ public class MetricsHandler implements HttpHandler {
1616
private final HttpErrorHandlingPolicy errorHandlingPolicy;
1717

1818
public MetricsHandler() {
19-
this(new PrometheusScrapeHandler(), HttpErrorHandlingPolicy.genericResponse());
19+
this(new PrometheusScrapeHandler(), HttpErrorHandlingPolicy.builder().build());
2020
}
2121

2222
public MetricsHandler(PrometheusRegistry registry) {
23-
this(new PrometheusScrapeHandler(registry), HttpErrorHandlingPolicy.genericResponse());
23+
this(new PrometheusScrapeHandler(registry), HttpErrorHandlingPolicy.builder().build());
2424
}
2525

2626
public MetricsHandler(PrometheusProperties config) {
27-
this(new PrometheusScrapeHandler(config), HttpErrorHandlingPolicy.genericResponse());
27+
this(new PrometheusScrapeHandler(config), HttpErrorHandlingPolicy.builder().build());
2828
}
2929

3030
public MetricsHandler(PrometheusProperties config, PrometheusRegistry registry) {
31-
this(new PrometheusScrapeHandler(config, registry), HttpErrorHandlingPolicy.genericResponse());
31+
this(new PrometheusScrapeHandler(config, registry), HttpErrorHandlingPolicy.builder().build());
3232
}
3333

3434
MetricsHandler(

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

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -179,7 +179,7 @@ void registryExceptionIsPassedToConfiguredReporter() throws Exception {
179179
.port(0)
180180
.registry(throwingRegistry())
181181
.errorHandlingPolicy(
182-
HttpErrorHandlingPolicy.genericResponseWithReporter(reportedError::set))
182+
HttpErrorHandlingPolicy.builder().errorReporter(reportedError::set).build())
183183
.buildAndStart();
184184

185185
run(
@@ -194,12 +194,13 @@ void registryExceptionIsPassedToConfiguredReporter() throws Exception {
194194
}
195195

196196
@Test
197-
void registryExceptionCanUseLegacyDetailedResponse() throws Exception {
197+
void registryExceptionCanUseUnsafeDebugResponse() throws Exception {
198198
HTTPServer server =
199199
HTTPServer.builder()
200200
.port(0)
201201
.registry(throwingRegistry())
202-
.errorHandlingPolicy(HttpErrorHandlingPolicy.legacyDetailedResponse())
202+
.errorHandlingPolicy(
203+
HttpErrorHandlingPolicy.builder().unsafeDebugResponse(true).build())
203204
.buildAndStart();
204205

205206
run(

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

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,8 @@ void handleExceptionInvokesConfiguredReporter() {
8484
AtomicReference<Throwable> reportedError = new AtomicReference<>();
8585
HttpExchangeAdapter adapter =
8686
new HttpExchangeAdapter(
87-
httpExchange, HttpErrorHandlingPolicy.genericResponseWithReporter(reportedError::set));
87+
httpExchange,
88+
HttpErrorHandlingPolicy.builder().errorReporter(reportedError::set).build());
8889
IllegalStateException scrapeException = new IllegalStateException("secret failure");
8990

9091
adapter.handleException(scrapeException);
@@ -102,10 +103,12 @@ void reporterFailureDoesNotPreventGenericResponse() {
102103
HttpExchangeAdapter adapter =
103104
new HttpExchangeAdapter(
104105
httpExchange,
105-
HttpErrorHandlingPolicy.genericResponseWithReporter(
106-
ignored -> {
107-
throw new IllegalStateException("reporter failed");
108-
}));
106+
HttpErrorHandlingPolicy.builder()
107+
.errorReporter(
108+
ignored -> {
109+
throw new IllegalStateException("reporter failed");
110+
})
111+
.build());
109112

110113
adapter.handleException(new IllegalStateException("secret failure"));
111114

@@ -117,21 +120,29 @@ void reporterFailureDoesNotPreventGenericResponse() {
117120
}
118121

119122
@Test
120-
void legacyDetailedResponseIncludesStackTrace() {
123+
void unsafeDebugResponseIncludesStackTraceAndInvokesReporter() {
121124
HttpExchange httpExchange = mock(HttpExchange.class);
122125
Headers headers = new Headers();
123126
ByteArrayOutputStream responseBody = new ByteArrayOutputStream();
124127
when(httpExchange.getResponseHeaders()).thenReturn(headers);
125128
when(httpExchange.getResponseBody()).thenReturn(responseBody);
129+
AtomicReference<Throwable> reportedError = new AtomicReference<>();
126130
HttpExchangeAdapter adapter =
127-
new HttpExchangeAdapter(httpExchange, HttpErrorHandlingPolicy.legacyDetailedResponse());
131+
new HttpExchangeAdapter(
132+
httpExchange,
133+
HttpErrorHandlingPolicy.builder()
134+
.unsafeDebugResponse(true)
135+
.errorReporter(reportedError::set)
136+
.build());
128137

129-
adapter.handleException(new IllegalStateException("diagnostic detail"));
138+
IllegalStateException scrapeException = new IllegalStateException("diagnostic detail");
139+
adapter.handleException(scrapeException);
130140

131141
String body = new String(responseBody.toByteArray(), StandardCharsets.UTF_8);
132142
assertThat(body)
133143
.contains("An Exception occurred while scraping metrics:")
134144
.contains("IllegalStateException: diagnostic detail")
135145
.contains("at ");
146+
assertThat(reportedError.get()).isSameAs(scrapeException);
136147
}
137148
}

0 commit comments

Comments
 (0)