[KYUUBI #5381] Change the default metrics reporter to Prometheus #5344

zhaohehuhu · 2023-09-28T09:45:57Z

Why are the changes needed?

Close #5381

change default metrics reporter to prometheus since Kyuubi 1.8

How was this patch tested?

Add some test cases that check the changes thoroughly including negative and positive cases if possible
Add screenshots for manual tests if appropriate
Run test locally before make a pull request

Was this patch authored or co-authored using generative AI tooling?

kyuubi-metrics/src/main/scala/org/apache/kyuubi/metrics/MetricsConf.scala

pan3793 · 2023-09-28T09:48:07Z

Could you please mention this change in docs/deployment/migration-guide.md too?

zhaohehuhu · 2023-09-28T11:23:16Z

Could you please mention this change in docs/deployment/migration-guide.md too?

got it

pan3793 · 2023-09-28T11:47:37Z

docs/deployment/migration-guide.md

  Both Derby and SQLite are mainly for testing purposes, and they're not supposed to be used in production.
  To restore previous behavior, set `kyuubi.metadata.store.jdbc.database.type=DERBY` and
  `kyuubi.metadata.store.jdbc.url=jdbc:derby:memory:kyuubi_state_store_db;create=true`.

+* Since Kyuubi 1.8, PROMETHEUS is changed as the default metrics reporter, because PROMETHEUS is a popular and widely


we don't need to mention "because ..."

pan3793 · 2023-09-28T13:38:37Z

docs/configuration/settings.md

+| kyuubi.metrics.json.location    | metrics    | Where the JSON metrics file located                                                                                                                                                                                                                                                                                                                                                                                                                                                                  | string   | 1.2.0 |
+| kyuubi.metrics.prometheus.path  | /metrics   | URI context path of prometheus metrics HTTP server                                                                                                                                                                                                                                                                                                                                                                                                                                                   | string   | 1.2.0 |
+| kyuubi.metrics.prometheus.port  | 10019      | Prometheus metrics HTTP server port                                                                                                                                                                                                                                                                                                                                                                                                                                                                  | int      | 1.2.0 |
+| kyuubi.metrics.reporters        | PROMETHEUS | A comma-separated list for all metrics reporters<ul> <li>CONSOLE - ConsoleReporter which outputs measurements to CONSOLE periodically.</li> <li>JMX - JmxReporter which listens for new metrics and exposes them as MBeans.</li>  <li>JSON - JsonReporter which outputs measurements to json file periodically.</li> <li>PROMETHEUS - PrometheusReporter which exposes metrics in Prometheus format.</li> <li>SLF4J - Slf4jReporter which outputs measurements to system log periodically.</li></ul> | set      | 1.8.0 |


the version seems incorrect, have you regenerated it?

Thanks. already corrected to right version.

pan3793 · 2023-09-28T15:12:36Z

The test failure is related, could you fix it?

zhaohehuhu · 2023-09-29T03:21:48Z

The test failure is related, could you fix it?

looks like the default port 10019 for prometheus was already in use.

codecov-commenter · 2023-09-29T04:59:41Z

Codecov Report

Merging #5344 (84f4c82) into master (99789a8) will not change coverage.
Report is 12 commits behind head on master.
The diff coverage is 0.00%.

@@          Coverage Diff           @@
##           master   #5344   +/-   ##
======================================
  Coverage    0.00%   0.00%           
======================================
  Files         590     588    -2     
  Lines       33493   33427   -66     
  Branches     4424    4393   -31     
======================================
+ Misses      33493   33427   -66

Files	Coverage Δ
.../scala/org/apache/kyuubi/metrics/MetricsConf.scala	`0.00% <0.00%> (ø)`

... and 8 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

pan3793 · 2023-10-04T05:04:52Z

The test failure is related, could you fix it?

looks like the default port 10019 for prometheus was already in use.

check if there is leaked resorces in test, or change the binding port by overriding confs in test

zhaohehuhu · 2023-10-07T02:00:48Z

The test failure is related, could you fix it?

looks like the default port 10019 for prometheus was already in use.

check if there is leaked resorces in test, or change the binding port by overriding confs in test

I already changed the port to 0 but got another Flink issue then. I'm not sure if it's related to prometheus port as the error message is not showing the binding port was used anymore.

...ver/src/test/scala/org/apache/kyuubi/events/handler/ServerJsonLoggingEventHandlerSuite.scala

pan3793 · 2023-10-08T14:03:15Z

@zhaohehuhu the flink test failure is irrelevant, the fix takes effect.

…ServerJsonLoggingEventHandlerSuite.scala

...ver/src/test/scala/org/apache/kyuubi/events/handler/ServerJsonLoggingEventHandlerSuite.scala

…ServerJsonLoggingEventHandlerSuite.scala

zhaohehuhu · 2023-10-11T05:22:31Z

@zhaohehuhu the flink test failure is irrelevant, the fix takes effect.

got it. Thanks.

bowenliang123

LGTM.

### _Why are the changes needed?_ Close #5381 change default metrics reporter to prometheus since Kyuubi 1.8 ### _How was this patch tested?_ - [ ] Add some test cases that check the changes thoroughly including negative and positive cases if possible - [ ] Add screenshots for manual tests if appropriate - [ ] [Run test](https://kyuubi.readthedocs.io/en/master/contributing/code/testing.html#running-tests) locally before make a pull request ### _Was this patch authored or co-authored using generative AI tooling?_ Closes #5344 from zhaohehuhu/Improvement-0928. Closes #5381 84f4c82 [hezhao2] reset METRICS_REPORTERS for test case b9ee5f7 [Cheng Pan] Update kyuubi-server/src/test/scala/org/apache/kyuubi/events/handler/ServerJsonLoggingEventHandlerSuite.scala 86165a6 [Cheng Pan] Update kyuubi-server/src/test/scala/org/apache/kyuubi/events/handler/ServerJsonLoggingEventHandlerSuite.scala a3605b6 [hezhao2] set METRICS_PROMETHEUS_PORT to 0 for test cases f1a4d28 [hezhao2] restore version number for kyuubi.metrics.reporters in doc dae40e1 [hezhao2] change default metrics reporter to prometheus Lead-authored-by: hezhao2 <[email protected]> Co-authored-by: Cheng Pan <[email protected]> Signed-off-by: Cheng Pan <[email protected]> (cherry picked from commit 4bb67bd) Signed-off-by: Cheng Pan <[email protected]>

github-actions bot added kind:documentation Documentation is a feature! module:metrics labels Sep 28, 2023

pan3793 reviewed Sep 28, 2023

View reviewed changes

kyuubi-metrics/src/main/scala/org/apache/kyuubi/metrics/MetricsConf.scala Outdated Show resolved Hide resolved

pan3793 changed the title ~~change default metrics reporter to prometheus~~ Change default metrics reporter to prometheus Sep 28, 2023

zhaohehuhu force-pushed the Improvement-0928 branch from f7404ab to 4227338 Compare September 28, 2023 11:22

pan3793 reviewed Sep 28, 2023

View reviewed changes

change default metrics reporter to prometheus

dae40e1

zhaohehuhu force-pushed the Improvement-0928 branch from 26bbb69 to dae40e1 Compare September 28, 2023 13:32

pan3793 reviewed Sep 28, 2023

View reviewed changes

restore version number for kyuubi.metrics.reporters in doc

f1a4d28

pan3793 approved these changes Sep 28, 2023

View reviewed changes

pan3793 assigned zhaohehuhu Sep 28, 2023

pan3793 added this to the v1.8.0 milestone Sep 28, 2023

set METRICS_PROMETHEUS_PORT to 0 for test cases

a3605b6

github-actions bot added the module:server label Sep 29, 2023

pan3793 changed the title ~~Change default metrics reporter to prometheus~~ [KYUUBI #5381] Change default metrics reporter to prometheus Oct 8, 2023

pan3793 reviewed Oct 8, 2023

View reviewed changes

...ver/src/test/scala/org/apache/kyuubi/events/handler/ServerJsonLoggingEventHandlerSuite.scala Outdated Show resolved Hide resolved

pan3793 mentioned this pull request Oct 10, 2023

[TASK][EASY] Change default metrics reporter to prometheus #5381

Closed

3 tasks

Update kyuubi-server/src/test/scala/org/apache/kyuubi/events/handler/…

86165a6

…ServerJsonLoggingEventHandlerSuite.scala

pan3793 reviewed Oct 10, 2023

View reviewed changes

...ver/src/test/scala/org/apache/kyuubi/events/handler/ServerJsonLoggingEventHandlerSuite.scala Outdated Show resolved Hide resolved

Update kyuubi-server/src/test/scala/org/apache/kyuubi/events/handler/…

b9ee5f7

…ServerJsonLoggingEventHandlerSuite.scala

reset METRICS_REPORTERS for test case

84f4c82

pan3793 approved these changes Oct 11, 2023

View reviewed changes

bowenliang123 changed the title ~~[KYUUBI #5381] Change default metrics reporter to prometheus~~ [KYUUBI #5381] Change the default metrics reporter to Prometheus Oct 12, 2023

bowenliang123 approved these changes Oct 12, 2023

View reviewed changes

pan3793 closed this in 4bb67bd Oct 16, 2023

bowenliang123 mentioned this pull request Jan 5, 2024

[KYUUBI #5834] Add Grafana dashboard template #5147

Open

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[KYUUBI #5381] Change the default metrics reporter to Prometheus #5344

[KYUUBI #5381] Change the default metrics reporter to Prometheus #5344

zhaohehuhu commented Sep 28, 2023 •

edited by pan3793

Loading

pan3793 commented Sep 28, 2023

zhaohehuhu commented Sep 28, 2023

pan3793 Sep 28, 2023

zhaohehuhu Sep 28, 2023

pan3793 Sep 28, 2023

zhaohehuhu Sep 28, 2023

pan3793 commented Sep 28, 2023

zhaohehuhu commented Sep 29, 2023

codecov-commenter commented Sep 29, 2023 •

edited

Loading

pan3793 commented Oct 4, 2023

zhaohehuhu commented Oct 7, 2023 •

edited

Loading

pan3793 commented Oct 8, 2023

zhaohehuhu commented Oct 11, 2023

bowenliang123 left a comment

[KYUUBI #5381] Change the default metrics reporter to Prometheus #5344

[KYUUBI #5381] Change the default metrics reporter to Prometheus #5344

Conversation

zhaohehuhu commented Sep 28, 2023 • edited by pan3793 Loading

Why are the changes needed?

How was this patch tested?

Was this patch authored or co-authored using generative AI tooling?

pan3793 commented Sep 28, 2023

zhaohehuhu commented Sep 28, 2023

pan3793 Sep 28, 2023

Choose a reason for hiding this comment

zhaohehuhu Sep 28, 2023

Choose a reason for hiding this comment

pan3793 Sep 28, 2023

Choose a reason for hiding this comment

zhaohehuhu Sep 28, 2023

Choose a reason for hiding this comment

pan3793 commented Sep 28, 2023

zhaohehuhu commented Sep 29, 2023

codecov-commenter commented Sep 29, 2023 • edited Loading

Codecov Report

pan3793 commented Oct 4, 2023

zhaohehuhu commented Oct 7, 2023 • edited Loading

pan3793 commented Oct 8, 2023

zhaohehuhu commented Oct 11, 2023

bowenliang123 left a comment

Choose a reason for hiding this comment

zhaohehuhu commented Sep 28, 2023 •

edited by pan3793

Loading

codecov-commenter commented Sep 29, 2023 •

edited

Loading

zhaohehuhu commented Oct 7, 2023 •

edited

Loading