Add citadel metricset for Istio Metricbeat module #15990
Conversation
Signed-off-by: chrismark <[email protected]>
ChrsMark
added
enhancement
review
Metricbeat
ChrsMark
added
Team:Integrations
zube
bot
added
[zube]: Inbox
[zube]: In Review
and removed
[zube]: In Review
[zube]: Inbox
labels
zube
bot
added
[zube]: Inbox
[zube]: In Review
and removed
[zube]: In Review
[zube]: Inbox
labels
| description: > | ||
| The type of the respective grpc service | ||
|
|
||
| - name: secret_controller_svc_acc_created_cert_count |
There was a problem hiding this comment.
This must be secret_controller_svc_acc_created_cert.count to follow naming conventions
There was a problem hiding this comment.
Thanks for mentioning. Will change it.
| type: long | ||
| description: > | ||
| The number of certificates created due to service account creation. | ||
| - name: server_root_cert_expiry_timestamp |
There was a problem hiding this comment.
I'd call it server_root_cert_expiry_seconds
There was a problem hiding this comment.
Thanks for suggesting. Will change it.
| description: > | ||
| Total number of RPCs started on the server. | ||
|
|
||
| - name: grpc.server.handling.latency.ms.bucket.* |
There was a problem hiding this comment.
2 things here:
- Usually the unit comes at the end of the field, it's tricky in this case because of the wildcard anyways:
grpc.server.handling.latency.bucket.ms.* - I wonder if we should store this operations instead of "doing" them in Kibana.
There was a problem hiding this comment.
This is common pattern so far for this kind of Metrics (Prometheus histograms). See
There was a problem hiding this comment.
Great, thank you for explaining :)
| object_type: long | ||
| description: > | ||
| The response latency (milliseconds) of gRPC that had been application-level handled by the server. | ||
| - name: grpc.server.handling.latency.ms.sum |
There was a problem hiding this comment.
Unit at the end :) grpc.server.handling.latency.sum.ms, same thing with the sum operation
There was a problem hiding this comment.
same as above this is a prometheus spesific concern.
| format: duration | ||
| description: > | ||
| The response latency of gRPC, sum of latencies in milliseconds | ||
| - name: grpc.server.handling.latency.ms.count |
There was a problem hiding this comment.
same as above this is a prometheus spesific concern.
|
Thank you for reviewing @sayden ! Will push the adjustments soonish. Regarding the Prometheus Histogram types as I mentioned inline, this is something that we follow already for this kind of metrics. However this might be a subject of refactoring anyway in the near feature while we are working on #14843. In this case I would suggest we proceed with this currently and have all of them to be refactored with the new type introduction. wdyt? Regarding the Dashboard, it is planned already and listed in the "TODOs" #15505 (comment). |
Yeap, yeap, let's proceed. I just wanted to make sure that we weren't missing anything :) |
Signed-off-by: chrismark <[email protected]>
(cherry picked from commit e9b0be3)
ChrsMark
added
test-plan
This PR adds citadel metricset for istio Module.
The metrics will be collected from the respective Prometheus exporter endpoint:
istio-citadel.istio-system:15014: The istio-citadel job returns all Citadel-generated metrics.Part of: #15505
Manual testing
kubectl -n istio-system port-forward svc/istio-citadel 15014:15014istiomodule, configure it in metricbeat.yaml and start monitoring ISTIOcitadel