-
Notifications
You must be signed in to change notification settings - Fork 227
Add support for entity type and name in custom monitored resource #1024
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
base: master
Are you sure you want to change the base?
Conversation
| // NewPodConfig returns a PodConfig which uses for the provided pod, namespace and container label values, | ||
| // if found, and falls back to the podId and namespaceId. | ||
| func NewPodConfig(podId, namespaceId, podIdLabel, namespaceIdLabel, containerNameLabel, tenantUIDLabel string) PodConfig { | ||
| func NewPodConfig(podId, namespaceId, podIdLabel, namespaceIdLabel, containerNameLabel, tenantUIDLabel, entityTypeLabel, entityNameLabel string) PodConfig { |
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.
This looks a bit off. It just doesn't really look like it belongs to pod config.
| CustomLabels: map[string]string{ | ||
| "project_id": "", | ||
| "cluster_name": "", | ||
| "location": "", | ||
| "instance_id": "", | ||
| "node_name": "", | ||
| "entity_type": "test", | ||
| }, | ||
| PodConfig: config.NewPodConfig("", "", "", "", "", "", "entityTypeLabel", ""), | ||
| }, | ||
| }, | ||
| []*dto.LabelPair{ | ||
| { | ||
| Name: stringPtr("entityTypeLabel"), | ||
| Value: stringPtr("test"), | ||
| }, |
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.
My understanding is that entity_type and entity_name will be in SourceConfig.CustomLabels and will be just taken from LabelPairs, so that we just need to adjust getCustomMonitoredResource function to sth like this:
func getCustomMonitoredResource(config *config.CommonConfig, tenantUID string, , labels []*dto.LabelPair) *monitoredres.MonitoredResource {
resourceLabels := config.SourceConfig.CustomLabels
applyDefaultIfEmpty(resourceLabels, "instance_id", config.GceConfig.InstanceId)
applyDefaultIfEmpty(resourceLabels, "project_id", config.GceConfig.Project)
applyDefaultIfEmpty(resourceLabels, "cluster_name", config.GceConfig.Cluster)
applyDefaultIfEmpty(resourceLabels, "location", config.GceConfig.ClusterLocation)
applyDefaultIfEmpty(resourceLabels, "node_name", config.GceConfig.Instance)
takeFromLabelsIfEmptyAndExistsInCustomLabels(resourceLabels, "entity_type", labels)
takeFromLabelsIfEmptyAndExistsInCustomLabels(resourceLabels, "entity_name", labels) // or more generic solution to do that for all empty custom labels
return &monitoredres.MonitoredResource{
Type: config.SourceConfig.CustomResourceType,
// Need to clone the map because timeseries created using this label can
// have its label value overwritten by a newer value of tenant_uid before
// it was even exported since map would have pointed to the same address.
Labels: cloneAndApplyDefaultIfFound(resourceLabels, "tenant_uid", tenantUID),
}
}
But maybe I am missing sth.
@x13n could you help here?
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 haven't touched this repo for years, so I'm afraid I don't have good intuition here. Current component owners should be able to address deep code questions much better.
No description provided.