feat: promote feature flag removeStartTimeAdjustment to stable #44851

perebaj · 2025-12-09T15:30:34Z

Description

Promote feature flag removeStartTimeAdjustment to Stable

Link to tracking issue

Fixes #44180

Signed-off-by: perebaj <perebaj@gmail.com>

receiver/prometheusreceiver/internal/transaction.go

perebaj · 2025-12-09T16:13:39Z

receiver/prometheusreceiver/metrics_receiver_test.go

-// TestStartTimeMetric validates that timeseries have start time set to 'process_start_time_seconds'
-func TestStartTimeMetric(t *testing.T) {
-	err := featuregate.GlobalRegistry().Set("receiver.prometheusreceiver.RemoveStartTimeAdjustment", false)
-	require.NoError(t, err)
-	defer func() {
-		_ = featuregate.GlobalRegistry().Set("receiver.prometheusreceiver.RemoveStartTimeAdjustment", true)
-	}()
-	targets := []*testData{
-		{
-			name: "target1",
-			pages: []mockPrometheusResponse{
-				{code: 200, data: startTimeMetricPage},
-			},
-			validateFunc: verifyStartTimeMetricPage,
-		},
-	}
-	testComponent(t, targets, func(c *Config) {
-		c.UseStartTimeMetric = true
-	})
-}
-


Is that test makes sense for the current statement? I don't think so...

This function expects the receiver to adjust the StartTimestamp from process_start_time_seconds

Defined here -> https://github.com/perebaj/opentelemetry-collector-contrib/blob/849b9ec53a4fa0f5195e2bb2f3e7e58ae04e9506/receiver/prometheusreceiver/metrics_receiver_test.go#L1545

After receiver.prometheusreceiver.RemoveStartTimeAdjustment was promoted to stable, the transaction production flow does:

if !removeStartTimeAdjustment.IsEnabled() { // only here would it call the adjuster (including the StartTimeMetricAdjuster) t.metricAdjuster.AdjustMetrics(md)}

Since the gate is stable, it is always enabled and cannot be turned off; then this block never runs and no start time adjustment is made, even with UseStartTimeMetric = true.

yes, you can remove this test.

ArthurSens

I was going through the changes and realized I have different expectations for a stable feature gate.

I thought that meant we could delete the codebase related to the metrics adjuster because a stable feature gate will never decrease its stability. I'm wondering if my expectations are correct or if we need to keep that code around for some reason 🤔

ArthurSens · 2025-12-10T13:32:23Z

receiver/prometheusreceiver/internal/metrics_adjuster_test.go

 	"go.opentelemetry.io/collector/pdata/pmetric"
 	semconv "go.opentelemetry.io/otel/semconv/v1.27.0"
 	"go.uber.org/zap"
-


If we're removing metrics adjuster, should we even keep this file? If I understand it correctly it's only testing starttimeadjuster

ArthurSens · 2025-12-10T13:32:48Z

receiver/prometheusreceiver/internal/starttimemetricadjuster.go

 	}
 }

 func (stma *startTimeMetricAdjuster) AdjustMetrics(metrics pmetric.Metrics) error {


Shouldn't we delete this whole codebase?

ArthurSens · 2025-12-10T13:34:22Z

receiver/prometheusreceiver/internal/transaction_test.go

 func TestTransactionCommitErrorWhenAdjusterError(t *testing.T) {
 	for _, enableNativeHistograms := range []bool{true, false} {
 		t.Run(fmt.Sprintf("enableNativeHistograms=%v", enableNativeHistograms), func(t *testing.T) {
-			defer testutil.SetFeatureGateForTest(t, removeStartTimeAdjustment, false)()
 			testTransactionCommitErrorWhenAdjusterError(t, enableNativeHistograms)
 		})
 	}


This tests when the adjuster fails. Shouldn't we delete this test?

Makes sense, thanks!

removed here ->
80d0584

Signed-off-by: perebaj <perebaj@gmail.com>

perebaj · 2025-12-10T22:40:39Z

receiver/prometheusreceiver/internal/appendable.go

-	useCreatedMetric bool,
 	enableNativeHistograms bool,
 	useMetadata bool,
 	externalLabels labels.Labels,
 	trimSuffixes bool,
 ) (storage.Appendable, error) {
-	var metricAdjuster MetricsAdjuster
-	if !useStartTimeMetric {
-		metricAdjuster = NewInitialPointAdjuster(set.Logger, gcInterval, useCreatedMetric)
-	} else {
-		metricAdjuster = NewStartTimeMetricAdjuster(set.Logger, startTimeMetricRegex, gcInterval)
-	}
-


Since we don't need of this code anymore, the feature
var useCreatedMetricGate = featuregate.GlobalRegistry().MustRegister can be removed. But I don't know if should I remove it or just omit the var declaration, as I did here

Talking about this change here -> https://github.com/open-telemetry/opentelemetry-collector-contrib/pull/44851/files#r2608462092

perebaj · 2025-12-10T22:41:46Z

receiver/prometheusreceiver/factory.go


 // This file implements config for Prometheus receiver.
-var useCreatedMetricGate = featuregate.GlobalRegistry().MustRegister(
+var _ = featuregate.GlobalRegistry().MustRegister(


Should we remove this feature gate, or this should be done in a different PR? We are basically deprecating the necessity to use it.

done in a different PR I think. I believe you should use featuregate.WithRegisterToVersion, with a version a few releases in the future.

oh, wait. I didn't realize this isn't a different feature gate. This is unrelated to this PR. Please make the change to the created timestamp gate in a new PR if thats ok.

but is it ok to leave var definition as I did? ⬇️

var _ = featuregate.GlobalRegistry().MustRegister(

With that, I think that we are basically deprecating the variable, because we aren't using it anymore

ah, I see. This is a sub-feature of the metrics adjuster.

Final update, I promise :). You should set this to featuregate.StageDeprecated, and add WithRegisterToVersion, but leave it around.

Please also add to the release notes that this disables the created metric feature gate. However, this does not mean that the _created series is always ignored. Prometheus parses the _created series as the start timestamp, but only when using OpenMetrics 1.0. So this change is removing support for the _created series only from other scrape protocols.

Signed-off-by: perebaj <perebaj@gmail.com>

dashpole · 2025-12-11T16:36:47Z

@ArthurSens the stable gate can't be disabled, so it is OK to remove code: https://github.com/open-telemetry/opentelemetry-collector/blob/main/featuregate/README.md#feature-lifecycle

A generally available or stable stage where the feature is permanently enabled.

ArthurSens

LGTM, just need to resolve the conflicts :)

Always good seeing this 😁

perebaj · 2025-12-12T20:33:45Z

isn't better to wait for this PR here?

I think that it will add more conflicts 🫠

perebaj added 5 commits December 9, 2025 12:30

feat: promote feature flag removeStartTimeAdjustment to stable

a54e2c5

Signed-off-by: perebaj <perebaj@gmail.com>

remove unused defer

f8a5850

Signed-off-by: perebaj <perebaj@gmail.com>

remove removeStartTimeAdjustment var since it will not be used anymore

a2a92ba

Signed-off-by: perebaj <perebaj@gmail.com>

remove feature flag from readme

27d5786

Signed-off-by: perebaj <perebaj@gmail.com>

change WithRegisterFromVersion version and remove test

849b9ec

Signed-off-by: perebaj <perebaj@gmail.com>

perebaj commented Dec 9, 2025

View reviewed changes

perebaj marked this pull request as ready for review December 9, 2025 17:06

perebaj requested review from a team, ArthurSens and dashpole as code owners December 9, 2025 17:06

github-actions bot added the receiver/prometheus Prometheus receiver label Dec 9, 2025

github-actions bot requested review from Aneurysm9 and krajorama December 9, 2025 17:06

github-actions bot assigned ChrsMark Dec 9, 2025

ChrsMark added the waiting-for-code-owners label Dec 10, 2025

ArthurSens reviewed Dec 10, 2025

View reviewed changes

remove unused code

cdb8e56

Signed-off-by: perebaj <perebaj@gmail.com>

perebaj commented Dec 10, 2025

View reviewed changes

perebaj added 3 commits December 10, 2025 19:46

remove unused tests

80d0584

Signed-off-by: perebaj <perebaj@gmail.com>

go mod tidy

f202107

Signed-off-by: perebaj <perebaj@gmail.com>

Merge branch 'main' into removeStartTimeAdjustment-to-stable

775dc37

ArthurSens approved these changes Dec 12, 2025

View reviewed changes

feat: promote feature flag removeStartTimeAdjustment to stable #44851

Are you sure you want to change the base?

feat: promote feature flag removeStartTimeAdjustment to stable #44851

Uh oh!

Conversation

perebaj commented Dec 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Link to tracking issue

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ArthurSens left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dashpole commented Dec 11, 2025

Uh oh!

ArthurSens left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

perebaj commented Dec 12, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

perebaj commented Dec 9, 2025 •

edited

Loading

ArthurSens left a comment •

edited

Loading