fix(native): Change content type of endpoint /v1/info/metrics based on accept header

[nmahadevuni]

Description

Modifies the Content-Type of endpoint /v1/info/metrics to application/json or text/plain based on the request's ACCEPT header.

Motivation and Context

Earlier Prometheus versions were not strict about Content-Type header in the response for this endpoint which is currently application/json. Latest version requires it to be set correctly to what's expected by Prometheus while scraping the metrics which is text/plain

Impact

Changes the Content-Type of endpoint /v1/info/metrics from application/json to text/plain.

Test Plan

Tested manually running Prometheus in Docker and reproduced the error when Content-Type was set to json. Issue is fixed as shown in 2nd image below.

1. Failing case

2. Passing case

== RELEASE NOTES ==

General Changes
*  Fix to modify the Content-Type of endpoint /v1/info/metrics to application/json or text/plain based on the request's ACCEPT header

[sourcery-ai]

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Endpoint /v1/info/metrics now returns metrics with a text/plain Content-Type using ResponseBuilder, and a new MIME type constant has been introduced.

Sequence diagram for /v1/info/metrics response handling with updated Content-Type

sequenceDiagram
participant Client
participant PrestoServer
participant "BaseStatsReporter"
participant ResponseHandler

Client->>PrestoServer: GET /v1/info/metrics
PrestoServer->>"BaseStatsReporter": fetchMetrics()
"BaseStatsReporter"-->>PrestoServer: metrics (text format)
PrestoServer->>ResponseHandler: Build response
Note right of PrestoServer: Sets Content-Type: text/plain
ResponseHandler-->>Client: 200 OK, metrics (text/plain)

Loading

Class diagram for updated HTTP constants

classDiagram
class HttpConstants {
  +kMimeTypeApplicationJson : const char[]
  +kMimeTypeApplicationThrift : const char[]
  +kMimeTypeTextPlain : const char[]
  +kShuttingDown : const char[]
  +kPrestoInternalBearer : const char[]
}

Loading

File-Level Changes

Change	Details	Files
Refactor metrics endpoint response to set text/plain content type	Replaced http::sendOkResponse with proxygen::ResponseBuilder Set HTTP status to OK explicitly Added Content-Type header with text/plain Sent response body via sendWithEOM()	presto-native-execution/presto_cpp/main/PrestoServer.cpp
Introduce text/plain MIME type constant	Declared kMimeTypeTextPlain in HTTP constants	presto-native-execution/presto_cpp/main/http/HttpConstants.h

---

Tips and commands

Interacting with Sourcery

• Trigger a new review: Comment @sourcery-ai review on the pull request.
• Continue discussions: Reply directly to Sourcery's review comments.
• Generate a GitHub issue from a review comment: Ask Sourcery to create an
  issue from a review comment by replying to it. You can also reply to a
  review comment with @sourcery-ai issue to create an issue from it.
• Generate a pull request title: Write @sourcery-ai anywhere in the pull
  request title to generate a title at any time. You can also comment
  @sourcery-ai title on the pull request to (re-)generate the title at any time.
• Generate a pull request summary: Write @sourcery-ai summary anywhere in
  the pull request body to generate a PR summary at any time exactly where you
  want it. You can also comment @sourcery-ai summary on the pull request to
  (re-)generate the summary at any time.
• Generate reviewer's guide: Comment @sourcery-ai guide on the pull
  request to (re-)generate the reviewer's guide at any time.
• Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
  pull request to resolve all Sourcery comments. Useful if you've already
  addressed all the comments and don't want to see them anymore.
• Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
  request to dismiss all existing Sourcery reviews. Especially useful if you
  want to start fresh with a new review - don't forget to comment
  @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

• Enable or disable review features such as the Sourcery-generated pull request
  summary, the reviewer's guide, and others.
• Change the review language.
• Add, remove or edit custom review instructions.
• Adjust other review settings.

Getting Help

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

[sourcery-ai]

Hey there - I've reviewed your changes and they look great!

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[nmahadevuni]

hi @yingsu00 @czentgr , Please review this.

[czentgr]

You could add a new helper function along with the other sendOk functions.
sendOkTextResponse. PrestoC++ doesn't have too many possible content types.

[nmahadevuni]

@czentgr Thanks for the review. I have added the helper function.

[aditi-pandit]

@nmahadevuni : Thanks for this code.

Is it possible to add some test logic in PrometheusReporterTest ?

I'm trying to assess the impact of this solution. While its definitely okay to return text/plain content type, application/json would likely be the more suited format for the metrics. Though I also read that Prometheus prefers text/plain. Is there a way to customize this based on the specific StatsReporter ? I'm concerned that Meta/Uber prefer tools that prefer as application/json over text/plain.

@amitkdutta : Please can you tell us if this change is okay for Meta production.

[nmahadevuni]

Thanks @aditi-pandit for the review. I have modified the response content type based on the request's ACCEPT header value. Also added test in new filePrestoServerAPITests.cpp which can be used to add any other server level API tests.

[czentgr]

How does this differ to the Java Prometheus endpoint?
Can we update the description that the mime type returned is dependent on the ACCEPT request header? I assume this is what's being sent by the prometheus server in when it complains about the response mime type?
Either way, the body will be the "text format" from the Prometheus library.

[nmahadevuni]

How does this differ to the Java Prometheus endpoint? Can we update the description that the mime type returned is dependent on the ACCEPT request header? I assume this is what's being sent by the prometheus server in when it complains about the response mime type? Either way, the body will be the "text format" from the Prometheus library.

I don't see the same endpoint implemented in Java. Yes, prometheus sends ACCEPT header. And the response body will be text format in any case.

[aditi-pandit]

Thanks @nmahadevuni.

I'm more comfortable with this code change. Have a comment about the test.

[aditi-pandit]

On second thoughts this test doesn't really exercise any actual server code-path as such so its not worth it. We can remove it.

[aditi-pandit]

Thanks @nmahadevuni

[nmahadevuni]

Thanks @nmahadevuni

Thanks for the review @aditi-pandit

[aditi-pandit]

Thanks @nmahadevuni