-
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
[Metricbeat] Add support for multiple regions in GCP #32964
Changes from 7 commits
54164d3
f079f12
2f27442
31d8fa1
2f73eb0
f492e82
ecc40c6
c3866cd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -99,15 +99,81 @@ func (r *metricsRequester) Metrics(ctx context.Context, serviceName string, alig | |
return results, nil | ||
} | ||
|
||
func (r *metricsRequester) buildRegionsFilter(regions []string, label string) string { | ||
if len(regions) == 0 { | ||
return "" | ||
} | ||
|
||
var filter strings.Builder | ||
|
||
// No. of regions added to the filter string. | ||
var regionsCount uint | ||
|
||
for _, region := range regions { | ||
// If 1 region has been added and the iteration continues, add the OR operator. | ||
if regionsCount > 0 { | ||
filter.WriteString("OR") | ||
filter.WriteString(" ") | ||
} | ||
|
||
filter.WriteString(fmt.Sprintf("%s = starts_with(\"%s\")", label, strings.TrimSuffix(region, "*"))) | ||
filter.WriteString(" ") | ||
|
||
regionsCount++ | ||
} | ||
|
||
switch { | ||
// If the filter string has more than 1 region, parentheses are added for better filter readability. | ||
case regionsCount > 1: | ||
return fmt.Sprintf("(%s)", strings.TrimSpace(filter.String())) | ||
default: | ||
return strings.TrimSpace(filter.String()) | ||
} | ||
} | ||
|
||
// getFilterForMetric returns the filter associated with the corresponding filter. Some services like Pub/Sub fails | ||
// if they have a region specified. | ||
func (r *metricsRequester) getFilterForMetric(serviceName, m string) string { | ||
f := fmt.Sprintf(`metric.type="%s"`, m) | ||
if r.config.Zone == "" && r.config.Region == "" { | ||
if r.config.Zone == "" && r.config.Region == "" && len(r.config.Regions) == 0 { | ||
return f | ||
} | ||
|
||
switch serviceName { | ||
case gcp.ServiceCompute: | ||
if r.config.Region != "" && r.config.Zone != "" { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This condition should take into account the case where |
||
r.logger.Warnf("when region %s and zone %s config parameter "+ | ||
"both are provided, only use region", r.config.Regions, r.config.Zone) | ||
} | ||
|
||
if r.config.Region != "" && len(r.config.Regions) != 0 { | ||
r.logger.Warnf("when region %s and regions config parameters are both provided, use region", r.config.Region) | ||
} | ||
|
||
if r.config.Region != "" { | ||
f = fmt.Sprintf( | ||
"%s AND %s = starts_with(\"%s\")", | ||
f, | ||
gcp.ComputeResourceLabelZone, | ||
strings.TrimSuffix(r.config.Region, "*"), | ||
) | ||
break | ||
} | ||
|
||
if r.config.Zone != "" { | ||
f = fmt.Sprintf( | ||
"%s AND %s = starts_with(\"%s\")", | ||
f, | ||
gcp.ComputeResourceLabelZone, | ||
strings.TrimSuffix(r.config.Zone, "*"), | ||
) | ||
break | ||
} | ||
|
||
if len(r.config.Regions) != 0 { | ||
regionsFilter := r.buildRegionsFilter(r.config.Regions, gcp.ComputeResourceLabelZone) | ||
f = fmt.Sprintf("%s AND %s", f, regionsFilter) | ||
} | ||
case gcp.ServiceGKE: | ||
if r.config.Region != "" && r.config.Zone != "" { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This cThis condition should take into account the case where |
||
r.logger.Warnf("when region %s and zone %s config parameter "+ | ||
|
@@ -131,15 +197,27 @@ func (r *metricsRequester) getFilterForMetric(serviceName, m string) string { | |
// } | ||
zone = strings.TrimSuffix(zone, "*") | ||
f = fmt.Sprintf("%s AND resource.label.location=starts_with(\"%s\")", f, zone) | ||
break | ||
} | ||
|
||
if len(r.config.Regions) != 0 { | ||
regionsFilter := r.buildRegionsFilter(r.config.Regions, gcp.GKEResourceLabelLocation) | ||
f = fmt.Sprintf("%s AND %s", f, regionsFilter) | ||
} | ||
case gcp.ServicePubsub, gcp.ServiceLoadBalancing, gcp.ServiceCloudFunctions, gcp.ServiceFirestore, gcp.ServiceDataproc: | ||
return f | ||
case gcp.ServiceStorage: | ||
if r.config.Region == "" { | ||
return f | ||
if r.config.Region != "" && len(r.config.Regions) != 0 { | ||
r.logger.Warnf("when region and regions config parameters are both provided, use region", r.config.Region) | ||
} | ||
|
||
f = fmt.Sprintf(`%s AND resource.labels.location = "%s"`, f, r.config.Region) | ||
switch { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here should be added the warning about |
||
case r.config.Region != "": | ||
f = fmt.Sprintf(`%s AND resource.labels.location = "%s"`, f, r.config.Region) | ||
case len(r.config.Regions) != 0: | ||
regionsFilter := r.buildRegionsFilter(r.config.Regions, gcp.StorageResourceLabelLocation) | ||
f = fmt.Sprintf("%s AND %s", f, regionsFilter) | ||
} | ||
default: | ||
if r.config.Region != "" && r.config.Zone != "" { | ||
r.logger.Warnf("when region %s and zone %s config parameter "+ | ||
|
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.
Just saw this changelog is under developer changelog, should this be under CHANGELOG-next?
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.
You're right! @gpop63 may you please add this line to
CHANGELOG.next
with a separate PR?