Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21723 )

Change subject: Add Prometheus HTTP service discovery
......................................................................


Patch Set 11:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/21723/10/src/kudu/master/master_path_handlers.cc
File src/kudu/master/master_path_handlers.cc:

http://gerrit.cloudera.org:8080/#/c/21723/10/src/kudu/master/master_path_handlers.cc@984
PS10, Line 984: resp->status_code = HttpStatusCode::Ser
> I've checked the Prometheus source and as expected prometheus short circuit
Thank you for clarifying on this.


http://gerrit.cloudera.org:8080/#/c/21723/10/src/kudu/master/master_path_handlers.cc@988
PS10, Line 988:     WriteEmptyPrometheusSDResponse(output);
              :     return;
> Ah, yes—nice catch, thank you!
It's nice that you took a closer look.

Having a test instance of Prometheus would be great, indeed.

Meanwhile, instead of responding with HTTP 200 that has semantics of valid data 
provided with the response, wouldn't it be safer to implement one of the 
following:
  * respond with HTTP 204 and no body (i.e. not even empty JSON object)
  * respond with HTTP 503 and no body

There could be an option to respond with HTTP 307 (temporary redirect) to the 
currently known leader master, but it (1) might be confusing (b) relies on the 
HTTP client config settings they have in their SD config (c) required leader 
master to be established and known to a particular master sending out the 
response


http://gerrit.cloudera.org:8080/#/c/21723/11/src/kudu/master/master_path_handlers.cc
File src/kudu/master/master_path_handlers.cc:

http://gerrit.cloudera.org:8080/#/c/21723/11/src/kudu/master/master_path_handlers.cc@995
PS11, Line 995:     WriteEmptyPrometheusSDResponse(output);
If not sending a valid empty JSON object in case of HTTP 503 in lines 982-986 
above, why to send one here?  Maybe, remove this as well?



--
To view, visit http://gerrit.cloudera.org:8080/21723
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I931aa72a7567c0dde43d7b7ed53a2dd0fa8bc1fe
Gerrit-Change-Number: 21723
Gerrit-PatchSet: 11
Gerrit-Owner: Marton Greber <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Attila Bukor <[email protected]>
Gerrit-Reviewer: Gabriella Lotz <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <[email protected]>
Gerrit-Reviewer: Wang Xixu <[email protected]>
Gerrit-Reviewer: Zoltan Chovan <[email protected]>
Gerrit-Reviewer: Zoltan Martonka <[email protected]>
Gerrit-Comment-Date: Thu, 07 Aug 2025 18:47:46 +0000
Gerrit-HasComments: Yes

Reply via email to