-
Notifications
You must be signed in to change notification settings - Fork 460
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
feat: support metrics scrape #2344
base: master
Are you sure you want to change the base?
Conversation
We should add a way to scrape v3 metrics as well |
@jiuker is there a way we can create tests for this? since this is a new feature? like an integration test? Thanks. |
b38e1b6
to
982f5d4
Compare
Will take a look |
2c7c9e0
to
40012a7
Compare
40012a7
to
732b95c
Compare
@jiuker PTAL - pkg:golang/golang.org/x/[email protected] vulnerability |
} | ||
|
||
for index, scrape := range t.Spec.PrometheusOperatorScrapeMetricsPath { | ||
promConfig.ScrapeConfigs = append(promConfig.ScrapeConfigs, ScrapeConfig{ |
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.
Consider the following tenant yaml section; note that v3 cluster metrics is scraped using /minio/v3/metrics/cluster/
(see https://github.com/minio/minio/blob/master/docs/metrics/v3.md):
prometheusOperator: true
prometheusOperatorScrapeMetricsPath:
- /minio/v2/metrics/bucket
- /minio/v2/metrics/cluster
- /minio/v2/metrics/node
- /minio/v2/metrics/resource
- /minio/v3/metrics/cluster/
v3 metrics scraping is failing with server returned HTTP status 403 Forbidden
. Please try and check this.
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.
Yesh. Should be like /minio/metrics/v3/*
, but that can be set with this strings slice field.
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.
I'm no expert on Prometheus metrics and MinIO, but I think this code doesn't work correctly. Please make these changes to AIStor operator and implement proper integration tests.
apply suggestion
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.
The code looks fine, but we do need an integration test to verify the functionality. It's a major change and it needs proper testing.
feat: support metrics scrape
fix #2327
we just support
/minio/v2/metrics/cluster
beforenow we can do more type for this
like
default is
/minio/v2/metrics/cluster