Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add response to HttpServerMetricsTagsContributor.Context #45717

Closed
AndreasPetersen opened this issue Jan 20, 2025 · 13 comments · Fixed by #45744
Closed

Add response to HttpServerMetricsTagsContributor.Context #45717

AndreasPetersen opened this issue Jan 20, 2025 · 13 comments · Fixed by #45744
Milestone

Comments

@AndreasPetersen
Copy link

Description

I want to be able to add tags to the HTTP server metric based on the contents of the response body.

HttpServerMetricsTagsContributor.Context only contains the request, however.

Implementation ideas

I can see that the HttpResponse is already available in

public void responseEnd(HttpRequestMetric requestMetric, HttpResponse response, long bytesWritten) {
log.debugf("responseEnd %s, %s", response, requestMetric);
String path = requestMetric.getNormalizedUriPath(
config.getServerMatchPatterns(),
config.getServerIgnorePatterns());
if (path != null) {
Timer.Sample sample = requestMetric.getSample();
Tags allTags = Tags.of(
VertxMetricsTags.method(requestMetric.request().method()),
HttpCommonTags.uri(path, requestMetric.initialPath, response.statusCode(),
config.isServerSuppress4xxErrors()),
VertxMetricsTags.outcome(response),
HttpCommonTags.status(response.statusCode()));
if (!httpServerMetricsTagsContributors.isEmpty()) {
HttpServerMetricsTagsContributor.Context context = new DefaultContext(requestMetric.request());
for (int i = 0; i < httpServerMetricsTagsContributors.size(); i++) {
try {
Tags additionalTags = httpServerMetricsTagsContributors.get(i).contribute(context);
allTags = allTags.and(additionalTags);
} catch (Exception e) {
log.debug("Unable to obtain additional tags", e);
}
}
}
openTelemetryContextUnwrapper.executeInContext(
sample::stop,
requestsTimer.withTags(allTags),
requestMetric.request().context());
}
requestMetric.requestEnded();
}
and that some tags are already using the HttpResponse. Would it be possible to add HttpResponse to the HttpServerMetricsTagsContributor.Context? However, it seems that the HttpResponse does not contain the body.

Copy link

quarkus-bot bot commented Jan 20, 2025

/cc @ebullient (metrics), @jmartisk (metrics)

@geoand
Copy link
Contributor

geoand commented Jan 20, 2025

Using the contents of the response body can be very tricky. What exactly do you need from the body?

@AndreasPetersen
Copy link
Author

Thanks for the quick response!

I want to add a tag based on what caused a 401 response code. We are returning the cause in the response body, but I want to be able to tell the cause in metrics as well.

Alternatively I could use a request scoped bean, set a value on it, and then use that in a MeterFilter, but as mentioned in the Quarkus 3.14 migration guide, that's not going to work.

@geoand
Copy link
Contributor

geoand commented Jan 20, 2025

The way Vert.x operates, that won't really work unfortunately

@AndreasPetersen
Copy link
Author

Is there any other way where I can add request specific tags, based on what happens in the request?

@geoand
Copy link
Contributor

geoand commented Jan 21, 2025

Can you give me an example of what you would use that for?

@AndreasPetersen
Copy link
Author

I want to be able to tell the cause of a 401 status code, so that I can filter metrics based on the cause. For example:

http_server_requests_count{status="401", cause="myCause"}

@geoand
Copy link
Contributor

geoand commented Jan 21, 2025

We can certainly add HttpResponse to HttpServerMetricsTagsContributor.Context, but as mentioned above that would not give you access to the body.
What you could do with it however is add a custom HTTP response header that you would then read in the contributor.

WDYT?

@AndreasPetersen
Copy link
Author

I suppose that could work 👍

geoand added a commit to geoand/quarkus that referenced this issue Jan 21, 2025
This is useful for creating tags based on HTTP
response headers for example

Closes: quarkusio#45717
@geoand
Copy link
Contributor

geoand commented Jan 21, 2025

#45744 adds that feature

@brunobat
Copy link
Contributor

I want to be able to tell the cause of a 401 status code, so that I can filter metrics based on the cause. For example:

http_server_requests_count{status="401", cause="myCause"}

I would advise against this because it will explode the cardinality of the metric.
This type of data should be retrieved from traces, not the metrics themselves.

@AndreasPetersen
Copy link
Author

#45744 adds that feature

Thanks!

@AndreasPetersen
Copy link
Author

I would advise against this because it will explode the cardinality of the metric.

We are of course aware of this, and are taking steps to ensure it won't be an issue for us. 🙂

This type of data should be retrieved from traces, not the metrics themselves.

We need the data in our metrics 🙂.

@geoand geoand closed this as completed in 9ac63d5 Jan 21, 2025
geoand added a commit that referenced this issue Jan 21, 2025
Add HTTP response to HttpServerMetricsTagsContributor.Context
@quarkus-bot quarkus-bot bot added this to the 3.19 - main milestone Jan 21, 2025
@gsmet gsmet modified the milestones: 3.19 - main, 3.18.0 Jan 21, 2025
gsmet pushed a commit to gsmet/quarkus that referenced this issue Jan 21, 2025
This is useful for creating tags based on HTTP
response headers for example

Closes: quarkusio#45717
(cherry picked from commit 9ac63d5)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants