Skip to content

Conversation

@turboFei
Copy link
Member

@turboFei turboFei commented Dec 25, 2024

Why are the changes needed?

For my use case, the instances are not human readable, so I prefer to return the FQDN.
image

How was this patch tested?

Integration testing.

(base) ➜  dist git:(prometheus_label_2) cat conf/kyuubi-defaults.conf
kyuubi.metrics.prometheus.metrics.instance.enabled=true
kyuubi.zookeeper.embedded.client.port.address=localhost
kyuubi.frontend.bind.host=localhost
image

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

No.

@turboFei turboFei changed the title Prometheus label 2 Support to return prometheus metrics with instance label Dec 25, 2024
@turboFei turboFei requested a review from pan3793 December 25, 2024 08:13
@github-actions github-actions bot added the kind:documentation Documentation is a feature! label Dec 25, 2024
@turboFei turboFei requested a review from pan3793 December 25, 2024 08:29
line
} else {
val Array(metrics, value) = line.split("\\s+", 2)
s"""$metrics${labelStr} $value"""
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a little bit hacky, but given the feature is disabled by default, let's merge it in first.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changed to

       line.split("\\s+", 2) match {
          case Array(metrics, rest) => s"""$metrics${labelStr} $rest"""
          case _ => line
        }

@pan3793 pan3793 added this to the v1.10.2 milestone Dec 25, 2024
@pan3793 pan3793 closed this in aa33521 Dec 25, 2024
@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 0% with 33 lines in your changes missing coverage. Please review.

Project coverage is 0.00%. Comparing base (d50cf17) to head (d24571c).
Report is 4 commits behind head on master.

Files with missing lines Patch % Lines
...che/kyuubi/metrics/PrometheusReporterService.scala 0.00% 28 Missing ⚠️
.../scala/org/apache/kyuubi/metrics/MetricsConf.scala 0.00% 5 Missing ⚠️
Additional details and impacted files
@@          Coverage Diff           @@
##           master   #6864   +/-   ##
======================================
  Coverage    0.00%   0.00%           
======================================
  Files         687     687           
  Lines       42473   42504   +31     
  Branches     5796    5798    +2     
======================================
- Misses      42473   42504   +31     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

pan3793 pushed a commit that referenced this pull request Dec 25, 2024
For my use case, the instances are not human readable, so I prefer to return the FQDN.
<img width="1483" alt="image" src="https://github.com/user-attachments/assets/92045517-456f-4087-8a36-9e3e4bea2f1d" />

Integration testing.
```
(base) ➜  dist git:(prometheus_label_2) cat conf/kyuubi-defaults.conf
kyuubi.metrics.prometheus.metrics.instance.enabled=true
kyuubi.zookeeper.embedded.client.port.address=localhost
kyuubi.frontend.bind.host=localhost
```

<img width="1692" alt="image" src="https://github.com/user-attachments/assets/0b60d504-62ec-418d-880b-f8a2f00d5550" />

No.

Closes #6864 from turboFei/prometheus_label_2.

Closes #6864

d24571c [Wang, Fei] match
6a6a511 [Wang, Fei] comments
c3046d4 [Wang, Fei] save
fb2021a [Wang, Fei] revert
4239594 [Wang, Fei] compatible
17b7007 [Wang, Fei] add instance label

Authored-by: Wang, Fei <[email protected]>
Signed-off-by: Cheng Pan <[email protected]>
(cherry picked from commit aa33521)
Signed-off-by: Cheng Pan <[email protected]>
@pan3793
Copy link
Member

pan3793 commented Dec 25, 2024

Thanks, merged to master/1.10

pan3793 pushed a commit that referenced this pull request Dec 26, 2024
### Why are the changes needed?

For histogram and timer metrics, it already has label, so need to support apply the instance label with existing ones.

For example:
```
# HELP kyuubi_backend_service_close_operation Generated from Dropwizard metric import (metric=kyuubi.backend_service.close_operation, type=com.codahale.metrics.Timer)
# TYPE kyuubi_backend_service_close_operation summary
kyuubi_backend_service_close_operation{quantile="0.5",}{instance="hadoopkyuubi-1.hadoopkyuubihl.hadoopmaster-dev.svc.140.tess.io:10019"} 0.032923216000000005
```

### How was this patch tested?

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

No.

Closes #6868 from turboFei/instance_label_follow.

Closes #6864

4894784 [Wang, Fei] ut
b8f227f [Wang, Fei] save

Authored-by: Wang, Fei <[email protected]>
Signed-off-by: Cheng Pan <[email protected]>
pan3793 pushed a commit that referenced this pull request Dec 26, 2024
### Why are the changes needed?

For histogram and timer metrics, it already has label, so need to support apply the instance label with existing ones.

For example:
```
# HELP kyuubi_backend_service_close_operation Generated from Dropwizard metric import (metric=kyuubi.backend_service.close_operation, type=com.codahale.metrics.Timer)
# TYPE kyuubi_backend_service_close_operation summary
kyuubi_backend_service_close_operation{quantile="0.5",}{instance="hadoopkyuubi-1.hadoopkyuubihl.hadoopmaster-dev.svc.140.tess.io:10019"} 0.032923216000000005
```

### How was this patch tested?

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

No.

Closes #6868 from turboFei/instance_label_follow.

Closes #6864

4894784 [Wang, Fei] ut
b8f227f [Wang, Fei] save

Authored-by: Wang, Fei <[email protected]>
Signed-off-by: Cheng Pan <[email protected]>
(cherry picked from commit fd53913)
Signed-off-by: Cheng Pan <[email protected]>
turboFei added a commit to turboFei/kyuubi that referenced this pull request Aug 27, 2025
…ce label

### Why are the changes needed?

For my use case, the instances are not human readable, so I prefer to return the FQDN.
<img width="1483" alt="image" src="https://github.com/user-attachments/assets/92045517-456f-4087-8a36-9e3e4bea2f1d" />

### How was this patch tested?

Integration testing.
```
(base) ➜  dist git:(prometheus_label_2) cat conf/kyuubi-defaults.conf
kyuubi.metrics.prometheus.metrics.instance.enabled=true
kyuubi.zookeeper.embedded.client.port.address=localhost
kyuubi.frontend.bind.host=localhost
```

<img width="1692" alt="image" src="https://github.com/user-attachments/assets/0b60d504-62ec-418d-880b-f8a2f00d5550" />

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

No.

Closes apache#6864 from turboFei/prometheus_label_2.

Closes apache#6864

d24571c [Wang, Fei] match
6a6a511 [Wang, Fei] comments
c3046d4 [Wang, Fei] save
fb2021a [Wang, Fei] revert
4239594 [Wang, Fei] compatible
17b7007 [Wang, Fei] add instance label

Authored-by: Wang, Fei <[email protected]>
Signed-off-by: Cheng Pan <[email protected]>
turboFei added a commit to turboFei/kyuubi that referenced this pull request Aug 27, 2025
…g labels

### Why are the changes needed?

For histogram and timer metrics, it already has label, so need to support apply the instance label with existing ones.

For example:
```
# HELP kyuubi_backend_service_close_operation Generated from Dropwizard metric import (metric=kyuubi.backend_service.close_operation, type=com.codahale.metrics.Timer)
# TYPE kyuubi_backend_service_close_operation summary
kyuubi_backend_service_close_operation{quantile="0.5",}{instance="hadoopkyuubi-1.hadoopkyuubihl.hadoopmaster-dev.svc.140.tess.io:10019"} 0.032923216000000005
```

### How was this patch tested?

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

No.

Closes apache#6868 from turboFei/instance_label_follow.

Closes apache#6864

4894784 [Wang, Fei] ut
b8f227f [Wang, Fei] save

Authored-by: Wang, Fei <[email protected]>
Signed-off-by: Cheng Pan <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind:documentation Documentation is a feature! module:metrics

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants