-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Add missing cluster metadata to multiple k8s metricsests #32979
Add missing cluster metadata to multiple k8s metricsests #32979
Conversation
Signed-off-by: Tetiana Kravchenko <[email protected]>
Signed-off-by: Tetiana Kravchenko <[email protected]>
This pull request does not have a backport label.
To fixup this pull request, you need to add the backport labels for the needed
|
Signed-off-by: Tetiana Kravchenko <[email protected]>
Signed-off-by: Tetiana Kravchenko <[email protected]>
/package |
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, left one minor comment.
@@ -574,3 +574,24 @@ func GetClusterECSMeta(cfg *conf.C, client k8sclient.Interface, logger *logp.Log | |||
} | |||
return ecsClusterMeta, nil | |||
} | |||
|
|||
// AddClusterECSMeta adds ECS orchestrator fields | |||
func AddClusterECSMeta(base mb.BaseMetricSet) mapstr.M { |
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.
Can this be also used at https://github.com/elastic/beats/blob/main/metricbeat/module/kubernetes/event/event.go#L102
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.
@ChrsMark I think no, for event is used different default config - https://github.com/elastic/beats/blob/main/metricbeat/module/kubernetes/event/event.go#L65, comparing to other metricsets.
I've replaced adding ECS orchestratop for volume
- afa7fc6, as it is the same as for proxy
, system
, apiserver
, scheduler
and controllermanager
"apiserver_audit_requests_rejected_total": prometheus.Metric("audit.rejected.count"), | ||
"rest_client_requests_total": prometheus.Metric("client.request.count"), | ||
}, | ||
var mapping = &prometheus.MetricsMapping{ |
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.
As long as you correctly moved them out just you can consider use the bulk var definition as we have them here: https://github.com/elastic/beats/blob/main/metricbeat/module/kubernetes/state_pod/state_pod.go#L36
Just to be the same.
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.
as we have here just one variable mapping
I would keep it as is
…dClusterECSMeta Signed-off-by: Tetiana Kravchenko <[email protected]>
* add missing field Signed-off-by: Tetiana Kravchenko <[email protected]> * update log message Signed-off-by: Tetiana Kravchenko <[email protected]> * fix tests Signed-off-by: Tetiana Kravchenko <[email protected]> * add pr number Signed-off-by: Tetiana Kravchenko <[email protected]> * volume metricset: replace adding ECS orchestrator fields with util.AddClusterECSMeta Signed-off-by: Tetiana Kravchenko <[email protected]> Signed-off-by: Tetiana Kravchenko <[email protected]> (cherry picked from commit 1f5b863) # Conflicts: # metricbeat/module/kubernetes/apiserver/_meta/test/metrics.1.14.expected # metricbeat/module/kubernetes/apiserver/apiserver.go # metricbeat/module/kubernetes/apiserver/metricset.go # metricbeat/module/kubernetes/controllermanager/_meta/test/metrics.1.20.expected # metricbeat/module/kubernetes/controllermanager/controllermanager.go
* add missing field Signed-off-by: Tetiana Kravchenko <[email protected]> * update log message Signed-off-by: Tetiana Kravchenko <[email protected]> * fix tests Signed-off-by: Tetiana Kravchenko <[email protected]> * add pr number Signed-off-by: Tetiana Kravchenko <[email protected]> * volume metricset: replace adding ECS orchestrator fields with util.AddClusterECSMeta Signed-off-by: Tetiana Kravchenko <[email protected]> Signed-off-by: Tetiana Kravchenko <[email protected]> (cherry picked from commit 1f5b863)
…3020) * add missing field Signed-off-by: Tetiana Kravchenko <[email protected]> * update log message Signed-off-by: Tetiana Kravchenko <[email protected]> * fix tests Signed-off-by: Tetiana Kravchenko <[email protected]> * add pr number Signed-off-by: Tetiana Kravchenko <[email protected]> * volume metricset: replace adding ECS orchestrator fields with util.AddClusterECSMeta Signed-off-by: Tetiana Kravchenko <[email protected]> Signed-off-by: Tetiana Kravchenko <[email protected]> (cherry picked from commit 1f5b863) Co-authored-by: Tetiana Kravchenko <[email protected]>
…etricsests (#33019) * Add missing cluster metadata to multiple k8s metricsests (#32979) * add missing field Signed-off-by: Tetiana Kravchenko <[email protected]> * update log message Signed-off-by: Tetiana Kravchenko <[email protected]> * fix tests Signed-off-by: Tetiana Kravchenko <[email protected]> * add pr number Signed-off-by: Tetiana Kravchenko <[email protected]> * volume metricset: replace adding ECS orchestrator fields with util.AddClusterECSMeta Signed-off-by: Tetiana Kravchenko <[email protected]> Signed-off-by: Tetiana Kravchenko <[email protected]> (cherry picked from commit 1f5b863) # Conflicts: # metricbeat/module/kubernetes/apiserver/_meta/test/metrics.1.14.expected # metricbeat/module/kubernetes/apiserver/apiserver.go # metricbeat/module/kubernetes/apiserver/metricset.go # metricbeat/module/kubernetes/controllermanager/_meta/test/metrics.1.20.expected # metricbeat/module/kubernetes/controllermanager/controllermanager.go * Update CHANGELOG.next.asciidoc * Update metrics.1.14.expected * Update apiserver.go * Update metricset.go * Update controllermanager.go * Update metrics.1.8.expected * Update metrics.1.20.expected * Update metrics.1.14.expected * Update metrics.1.8.expected * Update metrics.1.14.expected * Update metrics.1.14.expected * Delete metrics.1.20.expected * Update metrics.1.8.expected * Update metrics.1.8.expected * Update metrics.1.14.expected * Update metrics.controllermanager.1.14.expected Co-authored-by: Tetiana Kravchenko <[email protected]>
* add missing field Signed-off-by: Tetiana Kravchenko <[email protected]> * update log message Signed-off-by: Tetiana Kravchenko <[email protected]> * fix tests Signed-off-by: Tetiana Kravchenko <[email protected]> * add pr number Signed-off-by: Tetiana Kravchenko <[email protected]> * volume metricset: replace adding ECS orchestrator fields with util.AddClusterECSMeta Signed-off-by: Tetiana Kravchenko <[email protected]> Signed-off-by: Tetiana Kravchenko <[email protected]>
What does this PR do?
Add
orchestrator.cluster.name
andorchestrator.cluster.ip
fields to the datasets:proxy
,system
,apiserver
,scheduler
andcontrollermanager
.Why is it important?
orchestrator.cluster.name
is used for kubernetes dashboardsChecklist
CHANGELOG.next.asciidoc
orCHANGELOG-developer.next.asciidoc
.Author's Checklist
How to test this PR locally
Related issues
Use cases
Screenshots
Logs