-
Notifications
You must be signed in to change notification settings - Fork 5.2k
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
Updated Prometheus metrics #11141
Updated Prometheus metrics #11141
Conversation
4cb6a44
to
4ed84b0
Compare
Testing the values reported and documentation are pending. |
9a2066c
to
a8eb657
Compare
Need to account for changes in #11196 -> done |
1d95df4
to
e941f5c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
initial comments, still testing
d021b1b
to
b97e1b8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kerneltime could you please fix the conflict here |
4aa12f3
to
90c8804
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems to be failing
$ curl localhost:9000/minio/prometheus/metrics/cluster
<?xml version="1.0" encoding="UTF-8"?>
<Error><Code>AllAccessDisabled</Code><Message>All access to this bucket has been disabled.</Message><Resource>/minio/prometheus/metrics/cluster</Resource><RequestId></RequestId><HostId>09025b64-2459-4812-a523-acab49f8cc4d</HostId></Error>
$ curl localhost:9000/minio/prometheus/metrics/node
<?xml version="1.0" encoding="UTF-8"?>
<Error><Code>AllAccessDisabled</Code><Message>All access to this bucket has been disabled.</Message><Resource>/minio/prometheus/metrics/node</Resource><RequestId></RequestId><HostId>09025b64-2459-4812-a523-acab49f8cc4d</HostId></Error>
It worked for me, will sync up with you to see what the issue is. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested, changes work fine. Except this case:
on a single node setup with 4 drives i.e. ./minio server /tmp/data{1...4}
, the fields minio_cluster_capacity_*
report 0.
apologies for confusion, this was working |
This works for distributed setup. Not sure if we want to report this metric for single node deployment.
|
Since it is a standard deployment pattern, IMO it would be nice to have. But we can look at this in a later PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM & Tested
@kerneltime please update the |
Mint Automation
11141-b4c1b3a/mint-gateway-azure.sh.log:
Deleting image on docker hub |
Description
Introduce the updated Prometheus metrics.
This change will expose 2 URLs for metrics collection
This change also cleans up the names for the metrics making it complaint to Prometheus best practices and groups them in a cleaner way.
TODO:
If any specific metrics need to be part of the Cluster URL that are now part of Node URL, it will be added and reported across the cluster.
Motivation and Context
How to test this PR?
Types of changes
Checklist:
commit-id
orPR #
here)