flag includes invalid characters

[codeboten]

Component(s)

exporter/googlemanagedprometheus

What happened?

See https://github.com/open-telemetry/opentelemetry-collector-contrib/actions/runs/6852247654/job/18630332796?pr=29122

go test -race -timeout 300s -parallel 4 --tags="" -cover ./... -covermode=atomic -args -test.gocoverdir="/home/runner/work/opentelemetry-collector-contrib/opentelemetry-collector-contrib/coverage/unit"
?   	github.com/open-telemetry/opentelemetry-collector-contrib/exporter/googlemanagedprometheusexporter/internal/metadata	[no test files]
panic: invalid ID "gcp.untyped_double_export": invalid character(s) in ID

[codeboten]

@mx-psi

[github-actions]

Pinging code owners:

• exporter/googlemanagedprometheus: @aabmass @dashpole @jsuereth @punya @damemi @psx95

See Adding Labels via Comments if you do not have permissions to add labels yourself.

[mx-psi]

Flag was added on GoogleCloudPlatform/opentelemetry-operations-go/pull/668, so not recent. I didn't catch it while working on open-telemetry/opentelemetry-collector/pull/8766 because it was not defined on contrib itself but rather on an external repository, but I am still confused as to why contrib-tests passed 🤔

@damemi @dashpole what the status of this feature gate? For context, we were planning on disallowing _ on feature gates for now to have a conservative charset for 1.0. The simplest solution is to allow underscores, but would want to confirm first that this feature gate is not going away anytime soon.

[dashpole]

The feature gate is still needed. It is for an internal (to google) use of the collector, so we don't advertise it in our README. It will be removed without graduation after open-telemetry/opentelemetry-specification#3058 is fixed, and implemented in the collector.

Is there a way to exempt that feature gate? Otherwise, I can update the GCP exporter to use a different name. What characters are allowed?

[mx-psi]

@dashpole ASCII alphanumerics and dots are the allowed characters https://github.com/open-telemetry/opentelemetry-collector/blob/23500dda16b565d5f15c535e5a9d22927e4700d2/featuregate/registry.go#L21-L22

Using camel case or upper camel case (i.e. gcp.untypedDoubleExport or gcp.UntypedDoubleExport) would be consistent with all other feature gates in contrib at least. Would it be possible to change it to one of the two?

[codeboten]

@mx-psi @dashpole since this will be blocking the release, I would suggest allowing the underscores for now. WDYT?

[mx-psi]

Filed open-telemetry/opentelemetry-collector/pull/8865 in case we want to allow underscores instead of fixing this gate. If the fix by David can be filed by EOD, I think we should go with that, otherwise my PR should be mergeable as is.

[codeboten]

Adding @jpkrohling to ensure there isnt a release made until this is cleared up. I also labeled the PR as release:blocker

[codeboten]

Thanks for the quick fix @dashpole, I've updated the dependency here: 6b4398c