Register featuregate, otherwise does not work

[bogdandrutu]

No description provided.

[bogdandrutu]

@Aneurysm9 @mx-psi @codeboten I expect that we enforce correct usages of the featuregate, see #8057. For me this is very worry that 3 leads (who are also on the core leads) missed this.

Update: may mean that the API is not well designed, but we need to investigate a bit this.

[djaglowski]

may mean that the API is not well designed, but we need to investigate a bit this.

We almost want featuregate.GetRegistry().IsEnabled(id string) to panic if a gate is checked that is not registered, but there is risk that an unregistered gate is checked only in rare circumstances.

A minor improvement could have featuregate.GetRegistry().IsEnabled(id string) return an error as well as enabled.

There might be a solution that requires all feature gates to be resolved during collector startup. That way, we can panic/fail immediately without risk of rare circumstances. This probably means moving featuregate.GetRegistry() into an internal package and finding ways to provide controlled access at specific points in the code. For components, a factory option could act as the point of retrieval. Not sure about non-component packages in contrib though.

// component/processor.go
func WithFeatureGate(gateID string, callback(gateVal bool)) ProcessorFactoryOption {
	return processorFactoryOptionFunc(func(o *processorFactory) {
		enabled, err := featuregateinternal.GetRegistry().IsEnabled(gateID)
		if err != nil {
			o.unresolvedFeatureGateErr = err // fail gracefully later (or just panic now)
			return 
		}
		callback(enabled)
	})
}

// processor/myprocessor
type Config struct {
	myFeatureGate bool
}

func NewFactory() component.ProcessorFactory {
	cfg := createDefaultConfig
	return component.NewProcessorFactory(
		typeStr,
		cfg,
		component.WithTracesProcessor(createTracesProcessor, component.StabilityLevelStable),
		component.WithMetricsProcessor(createMetricsProcessor, component.StabilityLevelStable),
		component.WithLogsProcessor(createLogsProcessor, component.StabilityLevelStable)),
		component.WithFeatureGate("my.feature", func(gateVal bool) {
			cfg.myFeatureGate = gateVal
		})
}

[bogdandrutu]

@djaglowski will post a PR with a proposal soon in core, I do have some ideas, and I like also what you suggested.

[codeboten]

I'd missed that the unit test was not using the featuregate to turn on/off skipSanitizeLabel:

https://github.com/open-telemetry/opentelemetry-collector-contrib/pull/8057/files#diff-5a336e1c1dce702d5f77df0fbfc22adde8470d3521537ab9c38d7e2abc7f0d80R755

This should probably be a requirement of adding featuregates, that they be used in the unit tests.