Skip to content

Fix metrics filter with colon : in query value #428

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -165,16 +165,18 @@ Example: \

The `filter_query` will be applied to a final metrics API query when querying for metric data. You can read more about the metric API filter options in GCPs documentation https://cloud.google.com/monitoring/api/v3/filters

The final query sent to the metrics API already includes filters for project and metric type. Each applicable `filter_query` will be appended to the query with an AND
The final query sent to the metrics API already includes filters for project and metric type. Each applicable `filter_query` will be appended to the query with an `AND`. String filter values that contain special characters (e.g. `:` colon) must be quoted with quotation marks `"`. Please always check logs for potential syntax errors from GCP.

Full example
```
stackdriver_exporter \
--google.project-ids=my-test-project \
--monitoring.metrics-prefixes='pubsub.googleapis.com/subscription' \
--monitoring.metrics-prefixes='compute.googleapis.com/instance/cpu' \
--monitoring.metrics-prefixes='cloudsql.googleapis.com/database' \
--monitoring.filters='pubsub.googleapis.com/subscription:resource.labels.subscription_id=monitoring.regex.full_match("us-west4.*my-team-subs.*")' \
--monitoring.filters='compute.googleapis.com/instance/cpu:resource.labels.instance=monitoring.regex.full_match("us-west4.*my-team-subs.*")'
--monitoring.filters='compute.googleapis.com/instance/cpu:resource.labels.instance=monitoring.regex.full_match("us-west4.*my-team-subs.*")' \
--monitoring.filters='cloudsql.googleapis.com/database:resource.labels.database_id="my-test-project:my-database-name"'
```

Using projects filter:
Expand Down
6 changes: 3 additions & 3 deletions utils/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,11 @@ func NormalizeMetricName(metricName string) string {
}

func SplitExtraFilter(extraFilter string, separator string) (string, string) {
mPrefix := strings.Split(extraFilter, separator)
if mPrefix[0] == extraFilter {
mPrefix := strings.SplitN(extraFilter, separator, 2)
if len(mPrefix) != 2 {
return "", ""
}
return mPrefix[0], strings.Join(mPrefix[1:], "")
return mPrefix[0], mPrefix[1]
}

func ProjectResource(projectID string) string {
Expand Down
20 changes: 20 additions & 0 deletions utils/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,3 +31,23 @@ var _ = Describe("ProjectResource", func() {
Expect(ProjectResource("fake-project-1")).To(Equal("projects/fake-project-1"))
})
})

var _ = Describe("SplitExtraFilter", func() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add a test for the case where there's no : in the filter as well please?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No problem. Also added one for the incomplete/missing filter.
I don't have much experience with ginkgo but it seems that with v2 (or extensions/table) we could use DescribeTable for nicer parametrized tests. But I didn't want to introduce/change dependencies here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! The testing framework is rather outdated 😬 . I imagine we would probably look to remove the library in favor of more basic std lib based tests but never really got the time to do it.

It("returns an empty string from incomplete filter", func() {
metricPrefix, filterQuery := SplitExtraFilter("This_is__a-MetricName.Example/with/no/filter", ":")
Expect(metricPrefix).To(Equal(""))
Expect(filterQuery).To(Equal(""))
})

It("returns a metric prefix and filter query from basic filter", func() {
metricPrefix, filterQuery := SplitExtraFilter("This_is__a-MetricName.Example/with:filter.name=filter_value", ":")
Expect(metricPrefix).To(Equal("This_is__a-MetricName.Example/with"))
Expect(filterQuery).To(Equal("filter.name=filter_value"))
})

It("returns a metric prefix and filter query filter with colon", func() {
metricPrefix, filterQuery := SplitExtraFilter(`This_is__a-MetricName.Example/with:filter.name="filter:value"`, ":")
Expect(metricPrefix).To(Equal("This_is__a-MetricName.Example/with"))
Expect(filterQuery).To(Equal("filter.name=\"filter:value\""))
})
})