Fix metrics filter with colon `:` in query value #428

dn0 · 2025-04-15T12:08:26Z

Example of a metric filter affected by this:

cloudsql.googleapis.com/database:resource.labels.database_id="project-id:database-name"

dn0 · 2025-04-15T12:37:24Z

@SuperQ @kgeckhart Feel free to close this one in favor of anything that will fix this :)

The problem is visible in debug logs

level=INFO source=stackdriver_exporter.go:366 msg="Starting stackdriver_exporter" version="(version=0.18.0, branch=HEAD, revision=3b1a6e9ebb85a176b8ab48639f8e0a2a4a2f7bcf)" build_context="...." metric_prefixes="[... cloudsql.googleapis.com/database/disk/bytes_used cloudsql.googleapis.com/database/disk/quota cloudsql.googleapis.com/database/up]" extra_filters="cloudsql.googleapis.com/database:resource.labels.database_id=my-project-id:my-database" projectIDs=[my-project-id] projectsFilter=""

level=DEBUG source=monitoring_collector.go:285 msg="retrieving Google Stackdriver Monitoring metrics for descriptor" project_id=my-project-id descriptor=cloudsql.googleapis.com/database/disk/bytes_used

level=DEBUG source=monitoring_collector.go:315 msg="retrieving Google Stackdriver Monitoring metrics with filter" project_id=my-project-id filter="metric.type=\"cloudsql.googleapis.com/database/disk/bytes_used\" AND (resource.labels.database_id=my-project-idmy-database)"

dn0 · 2025-04-18T11:06:57Z

After testing this I should probably mention that for this to work the quotes around the filter value "project-id:database-name" are very important. Maybe there should be some validation method to check this during start time.

kgeckhart

Thanks for this! I had a few comments and a question.

After testing this I should probably mention that for this to work the quotes around the filter value "project-id:database-name" are very important.

Can you add some context about why the quotes are very important?

kgeckhart · 2025-04-18T12:28:09Z

utils/utils.go

@@ -46,7 +46,7 @@ func SplitExtraFilter(extraFilter string, separator string) (string, string) {
 	if mPrefix[0] == extraFilter {
 		return "", ""
 	}
-	return mPrefix[0], strings.Join(mPrefix[1:], "")
+	return mPrefix[0], strings.Join(mPrefix[1:], separator)


Perhaps we could change the strings.Split on line 45 -> strings.SplitN(extraFIlter, separator, 2) and then we have no need for the join here.

Thanks, I changed it to strings.SplitN().
Also slightly changed the guard if to check for slice length. Hope that's OK.

Absolutely, thank you!

kgeckhart · 2025-04-18T12:28:40Z

utils/utils_test.go

@@ -31,3 +31,11 @@ var _ = Describe("ProjectResource", func() {
 		Expect(ProjectResource("fake-project-1")).To(Equal("projects/fake-project-1"))
 	})
 })
+
+var _ = Describe("SplitExtraFilter", func() {


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

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.

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.

dn0 · 2025-04-18T14:26:07Z

@kgeckhart

After testing this I should probably mention that for this to work the quotes around the filter value "project-id:database-name" are very important.

Can you add some context about why the quotes are very important?

Otherwise some parser would fail. Full error:

level=ERROR source=monitoring_collector.go:239 msg="Error while getting Google Stackdriver Monitoring metrics" project_id=project-id err="googleapi: Error 400: Field filter had an invalid value of \"metric.type=\"cloudsql.googleapis.com/database/disk/quota\" AND (resource.labels.database_id=project-id:database-name)\": Could not parse filter \"metric.type=\\\"cloudsql.googleapis.com/database/disk/quota\\\" AND (resource.labels.database_id=project-id:database-name)\"; syntax error at line 1, column 107, token ':', badRequest"

kgeckhart · 2025-04-18T18:40:03Z

@kgeckhart

After testing this I should probably mention that for this to work the quotes around the filter value "project-id:database-name" are very important.

Can you add some context about why the quotes are very important?

Otherwise some parser would fail. Full error:
level=ERROR source=monitoring_collector.go:239 msg="Error while getting Google Stackdriver Monitoring metrics" project_id=project-id err="googleapi: Error 400: Field filter had an invalid value of \"metric.type=\"cloudsql.googleapis.com/database/disk/quota\" AND (resource.labels.database_id=project-id:database-name)\": Could not parse filter \"metric.type=\\\"cloudsql.googleapis.com/database/disk/quota\\\" AND (resource.labels.database_id=project-id:database-name)\"; syntax error at line 1, column 107, token ':', badRequest"

Got it, if you think you can add an extra guard that could produce a more helpful error go for it. I tried to quickly scan the sdk for a function that could validate the filter but didn't find anything.

dn0 · 2025-04-18T20:48:05Z

Otherwise some parser would fail. Full error:
level=ERROR source=monitoring_collector.go:239 msg="Error while getting Google Stackdriver Monitoring metrics" project_id=project-id err="googleapi: Error 400: Field filter had an invalid value of \"metric.type=\"cloudsql.googleapis.com/database/disk/quota\" AND (resource.labels.database_id=project-id:database-name)\": Could not parse filter \"metric.type=\\\"cloudsql.googleapis.com/database/disk/quota\\\" AND (resource.labels.database_id=project-id:database-name)\"; syntax error at line 1, column 107, token ':', badRequest"
Got it, if you think you can add an extra guard that could produce a more helpful error go for it. I tried to quickly scan the sdk for a function that could validate the filter but didn't find anything.

Hmm, I also didn't find anything useful in the SDK. The error is coming either from a serializer or from some DSL parsing happening on Google servers.

Looking at https://cloud.google.com/monitoring/api/v3/filters the filters can be quite complex. I was hoping that a check could be implemented somewhere in/around parseMetricExtraFilters() to fail early, but I can only think of a regexp and even that could become pretty complex. Based on this, I'm not sure it's worth implementing a custom validation. Relying on this error log message from Google now feels like the simplest possible approach :/

We could mention it in the README (Using filters section) - link the official filters spec, maybe mention to use quotes around strings (not only filter values but also user label keys) and mention to check logs when implementing complex filters.

kgeckhart · 2025-04-25T20:42:55Z

We could mention it in the README (Using filters section) - link the official filters spec, maybe mention to use quotes around strings (not only filter values but also user label keys) and mention to check logs when implementing complex filters.

I think that's a great idea and could hopefully help someone who gets the rather unhelpful error from GCP.

Example of a metric filter affected by this: ``` cloudsql.googleapis.com/database:resource.labels.database_id="project-id:database-name" ``` + Added/updated tests + Updated README with this example and a note about quoting special characters Signed-off-by: Daniel Kontsek <[email protected]>

dn0 · 2025-04-27T11:04:44Z

I just noticed that the link the the official official filters spec is already there :) So I only updated the full example in README and mentioned the need for quotes.

dn0 changed the title ~~Fix metrics filter with : in query value~~ Fix metrics filter with colon : in query value Apr 15, 2025

dn0 force-pushed the master branch from 9509fc4 to 2e9d43e Compare April 15, 2025 12:13

kgeckhart reviewed Apr 18, 2025

View reviewed changes

dn0 force-pushed the master branch from 2e9d43e to f818662 Compare April 18, 2025 14:24

dn0 force-pushed the master branch from f818662 to 83c4635 Compare April 27, 2025 11:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix metrics filter with colon `:` in query value #428

Fix metrics filter with colon `:` in query value #428

dn0 commented Apr 15, 2025 •

edited

Loading

dn0 commented Apr 15, 2025

dn0 commented Apr 18, 2025

kgeckhart left a comment

kgeckhart Apr 18, 2025

dn0 Apr 18, 2025

kgeckhart Apr 18, 2025

kgeckhart Apr 18, 2025

dn0 Apr 18, 2025

kgeckhart Apr 18, 2025

dn0 commented Apr 18, 2025

kgeckhart commented Apr 18, 2025

dn0 commented Apr 18, 2025

kgeckhart commented Apr 25, 2025

dn0 commented Apr 27, 2025

Fix metrics filter with colon : in query value #428

Are you sure you want to change the base?

Fix metrics filter with colon : in query value #428

Conversation

dn0 commented Apr 15, 2025 • edited Loading

dn0 commented Apr 15, 2025

dn0 commented Apr 18, 2025

kgeckhart left a comment

Choose a reason for hiding this comment

kgeckhart Apr 18, 2025

Choose a reason for hiding this comment

dn0 Apr 18, 2025

Choose a reason for hiding this comment

kgeckhart Apr 18, 2025

Choose a reason for hiding this comment

kgeckhart Apr 18, 2025

Choose a reason for hiding this comment

dn0 Apr 18, 2025

Choose a reason for hiding this comment

kgeckhart Apr 18, 2025

Choose a reason for hiding this comment

dn0 commented Apr 18, 2025

kgeckhart commented Apr 18, 2025

dn0 commented Apr 18, 2025

kgeckhart commented Apr 25, 2025

dn0 commented Apr 27, 2025

Fix metrics filter with colon `:` in query value #428

Fix metrics filter with colon `:` in query value #428

dn0 commented Apr 15, 2025 •

edited

Loading