Remove _total from duplicate untyped Sums before export (#678)

* Remove _total from duplicate untyped Sums before export

* Add unit test + skip integration test

exporter/collector/googlemanagedprometheus/naming.go

@@ -17,6 +17,7 @@ package googlemanagedprometheus
 import (
 	"fmt"
 	"strings"
+	"unicode"
 
 	"github.com/open-telemetry/opentelemetry-collector-contrib/pkg/translator/prometheus"
 	"go.opentelemetry.io/collector/pdata/pmetric"
@@ -32,9 +33,9 @@ func GetMetricName(baseName string, metric pmetric.Metric) (string, error) {
 			// Non-monotonic sums are converted to GCM gauges
 			return compliantName + "/gauge", nil
 		}
-		return compliantName + getUnknownMetricSuffix(metric.Sum().DataPoints(), "/counter", "counter"), nil
+		return getUnknownMetricName(metric.Sum().DataPoints(), "/counter", "counter", metric.Name(), compliantName), nil
 	case pmetric.MetricTypeGauge:
-		return compliantName + getUnknownMetricSuffix(metric.Gauge().DataPoints(), "/gauge", ""), nil
+		return getUnknownMetricName(metric.Gauge().DataPoints(), "/gauge", "", metric.Name(), compliantName), nil
 	case pmetric.MetricTypeSummary:
 		// summaries are sent as the following series:
 		// * Sum: prometheus.googleapis.com/<baseName>_sum/summary:counter
@@ -56,11 +57,23 @@ func GetMetricName(baseName string, metric pmetric.Metric) (string, error) {
 
 // getUnknownMetricSuffix will set the metric suffix for untyped metrics to
 // "/unknown" (eg, for Gauge) or "/unknown:{secondarySuffix}" (eg, "/unknown:counter" for Sum).
+// It also removes the "_total" suffix on an unknown counter, if this suffix was not present in
+// the original metric name before calling prometheus.BuildCompliantName(), which is hacky.
 // It is based on the untyped_prometheus_metric data point attribute, and behind a feature gate.
-func getUnknownMetricSuffix(points pmetric.NumberDataPointSlice, suffix, secondarySuffix string) string {
+func getUnknownMetricName(points pmetric.NumberDataPointSlice, suffix, secondarySuffix, originalName, compliantName string) string {
 	if !untypedDoubleExportFeatureGate.IsEnabled() {
-		return suffix
+		return compliantName + suffix
 	}
+
+	// de-normalize "_total" suffix for counters where not present on original metric name
+	nameTokens := strings.FieldsFunc(
+		originalName,
+		func(r rune) bool { return !unicode.IsLetter(r) && !unicode.IsDigit(r) },
+	)
+	if nameTokens[len(nameTokens)-1] != "total" && strings.HasSuffix(compliantName, "_total") {
+		compliantName = strings.TrimSuffix(compliantName, "_total")
+	}
+
 	newSuffix := suffix
 	for i := 0; i < points.Len(); i++ {
 		point := points.At(i)
@@ -74,5 +87,5 @@ func getUnknownMetricSuffix(points pmetric.NumberDataPointSlice, suffix, seconda
 			// even though we have the suffix, keep looping to remove the attribute from other points, if any
 		}
 	}
-	return newSuffix
+	return compliantName + newSuffix
 }

exporter/collector/googlemanagedprometheus/naming_test.go

@@ -190,6 +190,21 @@ func TestGetMetricName(t *testing.T) {
 			},
 			expected: "bar/unknown:counter",
 		},
+		{
+			desc:     "untyped sum with feature gate enabled + name normalization returns unknown:counter",
+			baseName: "bar",
+			metric: func(m pmetric.Metric) {
+				//nolint:errcheck
+				featuregate.GlobalRegistry().Set(gcpUntypedDoubleExportGateKey, true)
+				//nolint:errcheck
+				featuregate.GlobalRegistry().Set("pkg.translator.prometheus.NormalizeName", true)
+				m.SetName("bar")
+				m.SetEmptySum()
+				m.Sum().SetIsMonotonic(true)
+				m.Sum().DataPoints().AppendEmpty().Attributes().PutStr(GCPOpsAgentUntypedMetricKey, "true")
+			},
+			expected: "bar/unknown:counter",
+		},
 	} {
 		t.Run(tc.desc, func(t *testing.T) {
 			assert.True(t, gate.IsEnabled())

exporter/collector/integrationtest/testcases/testcases_metrics.go

@@ -365,5 +365,37 @@ var MetricsTestCases = []TestCase{
 		// SDK exporter does not support CreateServiceTimeSeries
 		SkipForSDK: true,
 	},
+	{
+		// https://github.com/GoogleCloudPlatform/opentelemetry-operations-go/issues/677
+		Name:                 "Google Managed Prometheus with Double Export for Untyped with name normalization enabled",
+		OTLPInputFixturePath: "testdata/fixtures/metrics/google_managed_prometheus.json",
+		ExpectFixturePath:    "testdata/fixtures/metrics/google_managed_prometheus_untyped_name_normalized_expect.json",
+		ConfigureCollector: func(cfg *collector.Config) {
+			cfg.MetricConfig.Prefix = "prometheus.googleapis.com/"
+			cfg.MetricConfig.SkipCreateMetricDescriptor = true
+			cfg.MetricConfig.GetMetricName = googlemanagedprometheus.GetMetricName
+			cfg.MetricConfig.MapMonitoredResource = googlemanagedprometheus.MapToPrometheusTarget
+			cfg.MetricConfig.ExtraMetrics = func(m pmetric.Metrics) pmetric.ResourceMetricsSlice {
+				//nolint:errcheck
+				featuregate.GlobalRegistry().Set("gcp.untyped_double_export", true)
+				//nolint:errcheck
+				featuregate.GlobalRegistry().Set("pkg.translator.prometheus.NormalizeName", true)
+				googlemanagedprometheus.AddUntypedMetrics(m)
+				googlemanagedprometheus.AddScopeInfoMetric(m)
+				googlemanagedprometheus.AddTargetInfoMetric(m)
+				return m.ResourceMetrics()
+			}
+			cfg.MetricConfig.InstrumentationLibraryLabels = false
+			cfg.MetricConfig.ServiceResourceLabels = false
+			cfg.MetricConfig.EnableSumOfSquaredDeviation = true
+			// disable cumulative normalization so we can see the counter without having to send 2 data points.
+			// with normalization enabled, the first data point would get dropped.
+			// but trying to send 2 data points causes the GCM integration test to fail for duplicate timeseries.
+			cfg.MetricConfig.CumulativeNormalization = false
+		},
+		// prometheus_target is not supported by the SDK
+		SkipForSDK: true,
+		Skip:       true,
+	},
 	// TODO: Add integration tests for workload.googleapis.com metrics from the ops agent
 }