ROX-28151: Custom Prometheus metrics configuration#15742
ROX-28151: Custom Prometheus metrics configuration#15742parametalol wants to merge 4 commits intomasterfrom
Conversation
parametalol
changed the title
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @parametalol - I've reviewed your changes and they look great!
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
Images are ready for the commit at 0772cf4. To use with deploy scripts, first |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #15742 +/- ##
==========================================
+ Coverage 48.80% 48.82% +0.02%
==========================================
Files 2590 2595 +5
Lines 190492 190629 +137
==========================================
+ Hits 92968 93079 +111
- Misses 90227 90246 +19
- Partials 7297 7304 +7
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@parametalol: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
| // } | ||
| // } | ||
| // } | ||
| Metrics image_vulnerabilities = 1; |
There was a problem hiding this comment.
Avoid plural if there is no repeated. How about MetricGroup?
vikin91
left a comment
vikin91
left a comment
There was a problem hiding this comment.
Posting comments as meeting notes, not an actual review.
|
|
||
| message Metrics { | ||
| // The period (in minutes) at which vulnerability data is gathered from the DB. | ||
| uint32 gathering_period_minutes = 1; |
There was a problem hiding this comment.
Consider using duration type to avoid setting like 67392 and get 345h instead.
| label, metric, slices.Sorted(maps.Keys(labelOrder))) | ||
| } | ||
|
|
||
| var expr Expression |
There was a problem hiding this comment.
Why can't we use []*Condition here? It makes the reader to first search what is an Expression to understand what is happening here.
| // data from another. | ||
| string registry_name = 3; | ||
| } | ||
|
|
There was a problem hiding this comment.
Notes from simplification discussion:
message Metric {
repeated string labels = 1; // that strings would be hardcoded by us
}
message MetricsGroup {
repeated Metric metrics = 1;
uint32 gathering_period = 2;
string filter = 3;
}
3 participants
Description
change me!
User-facing documentation
Testing and quality
Automated testing
How I validated my change
change me!
Current dependencies on/for this PR: