From 86ed1423b338f5f8b4801556c2836910b0b975fe Mon Sep 17 00:00:00 2001
From: Sean Liao <sean@liao.dev>
Date: Tue, 26 Nov 2024 22:01:37 +0000
Subject: [PATCH] fix(influx2otel): only map to histogram when all fields match

When AddPoint is called with the unknown metric type, it tries to
infer the type from the given field names. Right now it infers histogram
when any of the fields matches the histogram keys, but that leads to a
lot of errors when your data isn't actually a histogram. Instead, only
infer a histogram if all fields match the expected histogram keys.
---
 influx2otel/metrics_telegraf_prometheus_v1.go | 15 ++++---
 .../metrics_telegraf_prometheus_v1_test.go    | 41 +++++++++++++++++++
 2 files changed, 50 insertions(+), 6 deletions(-)

diff --git a/influx2otel/metrics_telegraf_prometheus_v1.go b/influx2otel/metrics_telegraf_prometheus_v1.go
index 31152e2a..765796fb 100644
--- a/influx2otel/metrics_telegraf_prometheus_v1.go
+++ b/influx2otel/metrics_telegraf_prometheus_v1.go
@@ -49,13 +49,18 @@ func (b *MetricsBatch) inferMetricValueTypeV1(vType common.InfluxMetricValueType
 	if _, found := fields[common.MetricCounterFieldKey]; found {
 		return common.InfluxMetricValueTypeSum
 	}
+	isHistogram := true
 	for k := range fields {
-		if k == common.MetricHistogramCountFieldKey || k == common.MetricHistogramSumFieldKey || isStringNumeric(k) {
-			// We cannot reliably distinguish between histogram and summary
-			// without knowing we have all points, so here we assume histogram.
-			return common.InfluxMetricValueTypeHistogram
+		if k != common.MetricHistogramCountFieldKey && k != common.MetricHistogramSumFieldKey && k != common.AttributeStartTimeStatsd && !isStringNumeric(k) {
+			isHistogram = false
+			break
 		}
 	}
+	if isHistogram {
+		// We cannot reliably distinguish between histogram and summary
+		// without knowing we have all points, so here we assume histogram.
+		return common.InfluxMetricValueTypeHistogram
+	}
 	return common.InfluxMetricValueTypeUntyped
 }
 
@@ -274,7 +279,6 @@ func (b *MetricsBatch) convertHistogramV1(measurement string, tags map[string]st
 				explicitBounds = append(explicitBounds, explicitBound)
 				bucketCounts = append(bucketCounts, uint64(vBucketCount))
 			}
-
 		} else if k == common.AttributeStartTimeStatsd {
 		} else {
 			b.logger.Debug("skipping unrecognized histogram field", "field", k, "value", vi)
@@ -356,7 +360,6 @@ func (b *MetricsBatch) convertSummaryV1(measurement string, tags map[string]stri
 				valueAtQuantile.SetQuantile(quantile)
 				valueAtQuantile.SetValue(value)
 			}
-
 		} else if k == common.AttributeStartTimeStatsd {
 		} else {
 			b.logger.Debug("skipping unrecognized summary field", "field", k, "value", vi)
diff --git a/influx2otel/metrics_telegraf_prometheus_v1_test.go b/influx2otel/metrics_telegraf_prometheus_v1_test.go
index 7f48cae7..d8830766 100644
--- a/influx2otel/metrics_telegraf_prometheus_v1_test.go
+++ b/influx2otel/metrics_telegraf_prometheus_v1_test.go
@@ -121,6 +121,47 @@ func TestAddPoint_v1_untypedGauge(t *testing.T) {
 	assertMetricsEqual(t, expect, b.GetMetrics())
 }
 
+func TestAddPoint_v1_untyped(t *testing.T) {
+	c, err := influx2otel.NewLineProtocolToOtelMetrics(new(common.NoopLogger))
+	require.NoError(t, err)
+
+	b := c.NewBatch()
+	err = b.AddPoint("some_custom_metric",
+		map[string]string{
+			"container.name":       "42",
+			"otel.library.name":    "My Library",
+			"otel.library.version": "latest",
+		},
+		map[string]any{
+			"count":          int64(1),
+			"something_else": float64(2.3),
+		},
+		time.Unix(0, 1395066363000000123).UTC(),
+		common.InfluxMetricValueTypeUntyped)
+	require.NoError(t, err)
+
+	expect := pmetric.NewMetrics()
+	rm := expect.ResourceMetrics().AppendEmpty()
+	rm.Resource().Attributes().PutStr("container.name", "42")
+	isMetrics := rm.ScopeMetrics().AppendEmpty()
+	isMetrics.Scope().SetName("My Library")
+	isMetrics.Scope().SetVersion("latest")
+	m := isMetrics.Metrics().AppendEmpty()
+	m.SetName("some_custom_metric_count")
+	m.SetEmptyGauge()
+	dp := m.Gauge().DataPoints().AppendEmpty()
+	dp.SetTimestamp(pcommon.NewTimestampFromTime(time.Unix(0, 1395066363000000123)))
+	dp.SetIntValue(1)
+	m = isMetrics.Metrics().AppendEmpty()
+	m.SetName("some_custom_metric_something_else")
+	m.SetEmptyGauge()
+	dp = m.Gauge().DataPoints().AppendEmpty()
+	dp.SetTimestamp(pcommon.NewTimestampFromTime(time.Unix(0, 1395066363000000123)))
+	dp.SetDoubleValue(2.3)
+
+	assertMetricsEqual(t, expect, b.GetMetrics())
+}
+
 func TestAddPoint_v1_sum(t *testing.T) {
 	c, err := influx2otel.NewLineProtocolToOtelMetrics(new(common.NoopLogger))
 	require.NoError(t, err)