[exporter/prometheusremotewrite] deprecate export_created_metric

[pokgak]

Description

Deprecate export_created_metric config option

Link to tracking issue

Fixes #35003

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: pokgak / name: Aiman Ismail (0e3b184, 31f8a02, 69c57e3, 810c216, 2806f5a, f11490c, 222332e, 9a5c587, 6ffc194)
• ✅ login: dashpole / name: David Ashpole (00e4e12, bc48b6a)

[ArthurSens]

Thank you @pokgak!

Not sure if this is documented well, but when deprecating we can put the functionality behind a feature gate that is enabled by default. One example of feature gate can be seen here. In the example though the stability is StageAlpha (disabled by default) and we want it to considered StageBeta so it's enabled by default.

In version 0.114.0 we'll finally change it to Alpha and in 0.115.0 finally remove the option.

[pokgak]

@ArthurSens correct me if I'm wrong but isn't the feature already disabled by default so we should be using StageAlpha instead?

opentelemetry-collector-contrib/exporter/prometheusremotewriteexporter/config.go

Lines 105 to 109 in 7ba5aa3

	if cfg.CreatedMetric == nil {
	cfg.CreatedMetric = &CreatedMetric{
	Enabled: false,
	}
	}

I pushed a commit using StageBeta but note that this will change the default value of the export_created_metric.enabled to true.

[ArthurSens]

@ArthurSens correct me if I'm wrong but isn't the feature already disabled by default so we should be using StageAlpha instead?

opentelemetry-collector-contrib/exporter/prometheusremotewriteexporter/config.go

Lines 105 to 109 in 7ba5aa3

	if cfg.CreatedMetric == nil {
	cfg.CreatedMetric = &CreatedMetric{
	Enabled: false,
	}
	}

I pushed a commit using StageBeta but note that this will change the default value of the export_created_metric.enabled to true.

Yes you're correct, I believe I wasn't clear with my previous comment, let me re-phrase it.

What we want to do is add yet another check when we're generating the created metric.

opentelemetry-collector-contrib/pkg/translator/prometheusremotewrite/helper.go

Lines 277 to 280 in 3458d51

	if settings.ExportCreatedMetric && startTimestamp != 0 {
	labels := createLabels(baseName+createdSuffix, baseLabels)
	c.addTimeSeriesIfNeeded(labels, startTimestamp, pt.Timestamp())
	}

Here we'd switch to:

 if settings.ExportCreatedMetric && startTimestamp != 0 && exportCreatedMetricGate.IsEnabled() { 
 	labels := createLabels(baseName+createdSuffix, baseLabels) 
 	c.addTimeSeriesIfNeeded(labels, startTimestamp, pt.Timestamp()) 
 } 

And then we'll have to update one test to make sure the feature-gate works as expected:

• TestPrometheusConverter_AddHistogramDataPoints

Add a new boolean to the test case struct, and in the the Run you can add:

oldValue := exportCreatedMetricGate.IsEnabled()
testutil.SetFeatureGateForTest(t, exportCreatedMetricGate, tt.isGateEnabled)
defer testutil.SetFeatureGateForTest(t, exportCreatedMetricGate, oldValue)

[ArthurSens]

LGTM, thank you!

Could a maintainer take a look and trigger CI?

[ArthurSens]

Actually, @pokgak could you rebase your PR on top of main? It looks like you've got some merge conflicts

[pokgak]

@ArthurSens thanks for the detailed steps.

In case you're reviewing this already, I only followed based on your steps for now. I think I need to also add test case for when the feature gate is false to make sure it's working correctly but haven't had time yet will do it tomorrow.

[ArthurSens]

@ArthurSens thanks for the detailed steps.

In case you're reviewing this already, I only followed based on your steps for now. I think I need to also add test case for when the feature gate is false to make sure it's working correctly but haven't had time yet will do it tomorrow.

That makes perfect sense, I'm glad you brought it up!

[pokgak]

@ArthurSens I added the test case and rebased to main. Also updated another test TestPrometheusConverter_AddSummaryDataPoints since it uses the exportCreatedMetric config option too. Should be ready for review again.

[ArthurSens]

Thanks!

[pokgak]

@dashpole i changed the default feature gate stage to alpha and updated the code and tests accordingly.

[ArthurSens]

Thank again!

[TylerHelmuth]

@pokgak please handle merge conflicts

[dashpole]

I tried resolving the conflicts manually. Lets see if it works

[pokgak]

thanks @dashpole @TylerHelmuth for the review and merge! also thanks to @ArthurSens for the guidance, if you got any other issues you need help with feel free to ping me.