Ignore accept header in requests to metrics service #1345
Conversation
Signed-off-by: Adrian Sutton <[email protected]>
Signed-off-by: Adrian Sutton <[email protected]>
Signed-off-by: Adrian Sutton <[email protected]>
ajsutton
force-pushed
the
metrics-ignore-accept
branch
from
027cc2a
to
1d2dc7e
Compare
|
Has this been field tested against a Prometheus instance? IIRC when I first implemented this Prometheus was very particular about their Is there a way to produce this content type without vertx automatically freaking out about the accepts header not matching perfectly? |
|
|
||
| // We should have JVM metrics already loaded, verify a simple key. | ||
| assertThat(props).containsKey("jvm_threads_deadlocked"); | ||
| } |
There was a problem hiding this comment.
| } | |
| // Assert we still get the Content-Type Prometheus likes | |
| assertThat(resp.header("Content-Type")).isEqualTo(TextFormat.CONTENT_TYPE_004); | |
| } |
You will also need import io.prometheus.client.exporter.common.TextFormat; but GitHub won't let me suggest that far our of the code diff.
| .method(HttpMethod.GET) | ||
| .produces(TextFormat.CONTENT_TYPE_004) | ||
| .handler(this::metricsRequest); | ||
| router.route("/metrics").method(HttpMethod.GET).handler(this::metricsRequest); |
There was a problem hiding this comment.
Maybe just check for "text/plain" instead of completely dropping Prometheus' very specific version?
There was a problem hiding this comment.
Given we only have one representation to offer we shouldn't be doing any content negotiation (which is what the Accept header is for). If we had multiple representations, then yes filtering to a particular Accept makes sense but if there's a default version, it should just be returned even if it doesn't match Accept.
For example this works fine:
curl --fail -v -H 'Accept: foo/foo' https://www.google.com/
The caller can then decide if they want to process the response based on the Content-Type header.
Signed-off-by: Adrian Sutton <[email protected]>
PR description
The metrics HTTP service only has one supported representation for content but is applying overly strict content type negotiation and refusing to return any content if the Accept header is present and set to anything other than
text/plain; version=0.0.4; charset=utf-8. This makes it incompatible with tools like DataDog which specifyAccept: text/plain.Fixed Issue(s)
Consensys/teku#2678
Changelog