Skip to content

Commit

Permalink
[signalfxexporter]: remove no longer necessary feature gate for prom …
Browse files Browse the repository at this point in the history 
…compatible metrics (#12671)

Signed-off-by: Bogdan Drutu <[email protected]>
  • Loading branch information
@bogdandrutu
bogdandrutu authored Jul 29, 2022
1 parent c364eb9 commit 29f4773
Show file tree
Hide file tree
Showing 5 changed files with 38 additions and 136 deletions.
15 changes: 1 addition & 14 deletions exporter/signalfxexporter/internal/translation/converter.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import (
sfxpb "github.com/signalfx/com_signalfx_metrics_protobuf/model"
"go.opentelemetry.io/collector/pdata/pcommon"
"go.opentelemetry.io/collector/pdata/pmetric"
"go.opentelemetry.io/collector/service/featuregate"
"go.uber.org/zap"
"go.uber.org/zap/zapcore"

Expand All @@ -42,18 +41,6 @@ var (
sfxMetricTypeCounter = sfxpb.MetricType_COUNTER
)

const prometheusCompatible = "exporter.signalfxexporter.PrometheusCompatible"

var prometheusCompatibleGate = featuregate.Gate{
ID: prometheusCompatible,
Enabled: true,
Description: "Controls if conversion should follow prometheus compatibility for histograms and summaries.",
}

func init() {
featuregate.GetRegistry().MustRegister(prometheusCompatibleGate)
}

// MetricsConverter converts MetricsData to sfxpb DataPoints. It holds an optional
// MetricTranslator to translate SFx metrics using translation rules.
type MetricsConverter struct {
Expand Down Expand Up @@ -82,7 +69,7 @@ func NewMetricsConverter(
metricTranslator: t,
filterSet: fs,
datapointValidator: newDatapointValidator(logger, nonAlphanumericDimChars),
translator: &signalfx.FromTranslator{PrometheusCompatible: featuregate.GetRegistry().IsEnabled(prometheusCompatible)},
translator: &signalfx.FromTranslator{},
}, nil
}

Expand Down
35 changes: 9 additions & 26 deletions pkg/translator/signalfx/from_metrics.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,22 +36,15 @@ var (
)

const (
// upper bound dimension key for histogram buckets.
bucketDimensionKey = "upper_bound"

// prometheus compatible dimension key for histogram buckets.
prometheusBucketDimensionKey = "le"
bucketDimensionKey = "le"

// quantile dimension key for summary quantiles.
quantileDimensionKey = "quantile"
)

// FromTranslator converts from pdata to SignalFx proto data model.
type FromTranslator struct {
// PrometheusCompatible controls if conversion should follow prometheus compatibility for histograms and summaries.
// If false it emits old signalfx smart agent format.
PrometheusCompatible bool
}
type FromTranslator struct{}

// FromMetrics converts pmetric.Metrics to SignalFx proto data points.
func (ft *FromTranslator) FromMetrics(md pmetric.Metrics) ([]*sfxpb.DataPoint, error) {
Expand Down Expand Up @@ -86,9 +79,9 @@ func (ft *FromTranslator) FromMetric(m pmetric.Metric, extraDimensions []*sfxpb.
case pmetric.MetricDataTypeSum:
dps = convertNumberDataPoints(m.Sum().DataPoints(), m.Name(), mt, extraDimensions)
case pmetric.MetricDataTypeHistogram:
dps = convertHistogram(m.Histogram().DataPoints(), m.Name(), mt, extraDimensions, ft.PrometheusCompatible)
dps = convertHistogram(m.Histogram().DataPoints(), m.Name(), mt, extraDimensions)
case pmetric.MetricDataTypeSummary:
dps = convertSummaryDataPoints(m.Summary().DataPoints(), m.Name(), extraDimensions, ft.PrometheusCompatible)
dps = convertSummaryDataPoints(m.Summary().DataPoints(), m.Name(), extraDimensions)
}

return dps
Expand Down Expand Up @@ -137,7 +130,7 @@ func convertNumberDataPoints(in pmetric.NumberDataPointSlice, name string, mt *s
return dps.out
}

func convertHistogram(in pmetric.HistogramDataPointSlice, name string, mt *sfxpb.MetricType, extraDims []*sfxpb.Dimension, promCompatible bool) []*sfxpb.DataPoint {
func convertHistogram(in pmetric.HistogramDataPointSlice, name string, mt *sfxpb.MetricType, extraDims []*sfxpb.Dimension) []*sfxpb.DataPoint {
var numDPs int
for i := 0; i < in.Len(); i++ {
numDPs += 2 + in.At(i).BucketCounts().Len()
Expand All @@ -153,10 +146,7 @@ func convertHistogram(in pmetric.HistogramDataPointSlice, name string, mt *sfxpb
count := int64(histDP.Count())
countDP.Value.IntValue = &count

sumName := name
if promCompatible {
sumName = name + "_sum"
}
sumName := name + "_sum"
sumDP := dps.appendPoint(sumName, mt, ts, dims)
sum := histDP.Sum()
sumDP.Value.DoubleValue = &sum
Expand All @@ -171,10 +161,6 @@ func convertHistogram(in pmetric.HistogramDataPointSlice, name string, mt *sfxpb
}

bucketMetricName := name + "_bucket"
bdKey := bucketDimensionKey
if promCompatible {
bdKey = prometheusBucketDimensionKey
}
var val uint64
for j := 0; j < counts.Len(); j++ {
val += counts.At(j)
Expand All @@ -185,7 +171,7 @@ func convertHistogram(in pmetric.HistogramDataPointSlice, name string, mt *sfxpb
cloneDim := make([]*sfxpb.Dimension, len(dims)+1)
copy(cloneDim, dims)
cloneDim[len(dims)] = &sfxpb.Dimension{
Key: bdKey,
Key: bucketDimensionKey,
Value: bound,
}
dp := dps.appendPoint(bucketMetricName, mt, ts, cloneDim)
Expand All @@ -197,7 +183,7 @@ func convertHistogram(in pmetric.HistogramDataPointSlice, name string, mt *sfxpb
return dps.out
}

func convertSummaryDataPoints(in pmetric.SummaryDataPointSlice, name string, extraDims []*sfxpb.Dimension, promCompatible bool) []*sfxpb.DataPoint {
func convertSummaryDataPoints(in pmetric.SummaryDataPointSlice, name string, extraDims []*sfxpb.Dimension) []*sfxpb.DataPoint {
var numDPs int
for i := 0; i < in.Len(); i++ {
numDPs += 2 + in.At(i).QuantileValues().Len()
Expand All @@ -214,10 +200,7 @@ func convertSummaryDataPoints(in pmetric.SummaryDataPointSlice, name string, ext
c := int64(inDp.Count())
countDP.Value.IntValue = &c

sumName := name
if promCompatible {
sumName = name + "_sum"
}
sumName := name + "_sum"
sumDP := dps.appendPoint(sumName, &sfxMetricTypeCumulativeCounter, ts, dims)
sum := inDp.Sum()
sumDP.Value.DoubleValue = &sum
Expand Down
102 changes: 6 additions & 96 deletions pkg/translator/signalfx/from_metrics_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,6 @@ func Test_FromMetrics(t *testing.T) {

tests := []struct {
name string
promCompatible bool
metricsFn func() pmetric.Metrics
wantSfxDataPoints []*sfxpb.DataPoint
}{
Expand Down Expand Up @@ -268,7 +267,7 @@ func Test_FromMetrics(t *testing.T) {
},
wantSfxDataPoints: []*sfxpb.DataPoint{
int64SFxDataPoint("histogram_count", &sfxMetricTypeCumulativeCounter, labelMap, 16),
doubleSFxDataPoint("histogram", &sfxMetricTypeCumulativeCounter, labelMap, 100.0),
doubleSFxDataPoint("histogram_sum", &sfxMetricTypeCumulativeCounter, labelMap, 100.0),
int64SFxDataPoint("histogram_bucket", &sfxMetricTypeCumulativeCounter,
maps.MergeStringMaps(map[string]string{bucketDimensionKey: "1"}, labelMap), 4),
int64SFxDataPoint("histogram_bucket", &sfxMetricTypeCumulativeCounter,
Expand All @@ -279,32 +278,6 @@ func Test_FromMetrics(t *testing.T) {
maps.MergeStringMaps(map[string]string{bucketDimensionKey: "+Inf"}, labelMap), 16),
},
},
{
name: "prometheus_histogram",
promCompatible: true,
metricsFn: func() pmetric.Metrics {
out := pmetric.NewMetrics()
ilm := out.ResourceMetrics().AppendEmpty().ScopeMetrics().AppendEmpty()
m := ilm.Metrics().AppendEmpty()
m.SetName("prometheus_histogram")
m.SetDataType(pmetric.MetricDataTypeHistogram)
m.Histogram().SetAggregationTemporality(pmetric.MetricAggregationTemporalityCumulative)
initHistDP(m.Histogram().DataPoints().AppendEmpty())
return out
},
wantSfxDataPoints: []*sfxpb.DataPoint{
int64SFxDataPoint("prometheus_histogram_count", &sfxMetricTypeCumulativeCounter, labelMap, 16),
doubleSFxDataPoint("prometheus_histogram_sum", &sfxMetricTypeCumulativeCounter, labelMap, 100.0),
int64SFxDataPoint("prometheus_histogram_bucket", &sfxMetricTypeCumulativeCounter,
maps.MergeStringMaps(map[string]string{prometheusBucketDimensionKey: "1"}, labelMap), 4),
int64SFxDataPoint("prometheus_histogram_bucket", &sfxMetricTypeCumulativeCounter,
maps.MergeStringMaps(map[string]string{prometheusBucketDimensionKey: "2"}, labelMap), 6),
int64SFxDataPoint("prometheus_histogram_bucket", &sfxMetricTypeCumulativeCounter,
maps.MergeStringMaps(map[string]string{prometheusBucketDimensionKey: "4"}, labelMap), 9),
int64SFxDataPoint("prometheus_histogram_bucket", &sfxMetricTypeCumulativeCounter,
maps.MergeStringMaps(map[string]string{prometheusBucketDimensionKey: "+Inf"}, labelMap), 16),
},
},
{
name: "delta_histogram",
metricsFn: func() pmetric.Metrics {
Expand All @@ -319,7 +292,7 @@ func Test_FromMetrics(t *testing.T) {
},
wantSfxDataPoints: []*sfxpb.DataPoint{
int64SFxDataPoint("delta_histogram_count", &sfxMetricTypeCounter, labelMap, 16),
doubleSFxDataPoint("delta_histogram", &sfxMetricTypeCounter, labelMap, 100.0),
doubleSFxDataPoint("delta_histogram_sum", &sfxMetricTypeCounter, labelMap, 100.0),
int64SFxDataPoint("delta_histogram_bucket", &sfxMetricTypeCounter,
maps.MergeStringMaps(map[string]string{bucketDimensionKey: "1"}, labelMap), 4),
int64SFxDataPoint("delta_histogram_bucket", &sfxMetricTypeCounter,
Expand All @@ -330,32 +303,6 @@ func Test_FromMetrics(t *testing.T) {
maps.MergeStringMaps(map[string]string{bucketDimensionKey: "+Inf"}, labelMap), 16),
},
},
{
name: "delta_prometheus_histogram",
promCompatible: true,
metricsFn: func() pmetric.Metrics {
out := pmetric.NewMetrics()
ilm := out.ResourceMetrics().AppendEmpty().ScopeMetrics().AppendEmpty()
m := ilm.Metrics().AppendEmpty()
m.SetName("delta_prometheus_histogram")
m.SetDataType(pmetric.MetricDataTypeHistogram)
m.Histogram().SetAggregationTemporality(pmetric.MetricAggregationTemporalityDelta)
initHistDP(m.Histogram().DataPoints().AppendEmpty())
return out
},
wantSfxDataPoints: []*sfxpb.DataPoint{
int64SFxDataPoint("delta_prometheus_histogram_count", &sfxMetricTypeCounter, labelMap, 16),
doubleSFxDataPoint("delta_prometheus_histogram_sum", &sfxMetricTypeCounter, labelMap, 100.0),
int64SFxDataPoint("delta_prometheus_histogram_bucket", &sfxMetricTypeCounter,
maps.MergeStringMaps(map[string]string{prometheusBucketDimensionKey: "1"}, labelMap), 4),
int64SFxDataPoint("delta_prometheus_histogram_bucket", &sfxMetricTypeCounter,
maps.MergeStringMaps(map[string]string{prometheusBucketDimensionKey: "2"}, labelMap), 6),
int64SFxDataPoint("delta_prometheus_histogram_bucket", &sfxMetricTypeCounter,
maps.MergeStringMaps(map[string]string{prometheusBucketDimensionKey: "4"}, labelMap), 9),
int64SFxDataPoint("delta_prometheus_histogram_bucket", &sfxMetricTypeCounter,
maps.MergeStringMaps(map[string]string{prometheusBucketDimensionKey: "+Inf"}, labelMap), 16),
},
},
{
name: "distribution_no_buckets",
metricsFn: func() pmetric.Metrics {
Expand All @@ -373,7 +320,7 @@ func Test_FromMetrics(t *testing.T) {
},
wantSfxDataPoints: []*sfxpb.DataPoint{
int64SFxDataPoint("no_bucket_histo_count", &sfxMetricTypeCumulativeCounter, labelMap, 2),
doubleSFxDataPoint("no_bucket_histo", &sfxMetricTypeCumulativeCounter, labelMap, 10),
doubleSFxDataPoint("no_bucket_histo_sum", &sfxMetricTypeCumulativeCounter, labelMap, 10),
},
},
{
Expand All @@ -399,7 +346,7 @@ func Test_FromMetrics(t *testing.T) {
},
wantSfxDataPoints: []*sfxpb.DataPoint{
int64SFxDataPoint("summary_count", &sfxMetricTypeCumulativeCounter, labelMap, 111),
doubleSFxDataPoint("summary", &sfxMetricTypeCumulativeCounter, labelMap, 123.4),
doubleSFxDataPoint("summary_sum", &sfxMetricTypeCumulativeCounter, labelMap, 123.4),
doubleSFxDataPoint("summary_quantile", &sfxMetricTypeGauge,
maps.MergeStringMaps(map[string]string{quantileDimensionKey: "0.25"}, labelMap), 0),
doubleSFxDataPoint("summary_quantile", &sfxMetricTypeGauge,
Expand All @@ -410,41 +357,6 @@ func Test_FromMetrics(t *testing.T) {
maps.MergeStringMaps(map[string]string{quantileDimensionKey: "1"}, labelMap), 3),
},
},
{
name: "prometheus_summaries",
promCompatible: true,
metricsFn: func() pmetric.Metrics {
out := pmetric.NewMetrics()
ilm := out.ResourceMetrics().AppendEmpty().ScopeMetrics().AppendEmpty()
m := ilm.Metrics().AppendEmpty()
m.SetName("prometheus_summary")
m.SetDataType(pmetric.MetricDataTypeSummary)
dp := m.Summary().DataPoints().AppendEmpty()
dp.SetTimestamp(ts)
dp.SetSum(123.4)
dp.SetCount(111)
qvs := dp.QuantileValues()
for i := 0; i < 4; i++ {
qv := qvs.AppendEmpty()
qv.SetQuantile(0.25 * float64(i+1))
qv.SetValue(float64(i))
}
attrMap.CopyTo(dp.Attributes())
return out
},
wantSfxDataPoints: []*sfxpb.DataPoint{
int64SFxDataPoint("prometheus_summary_count", &sfxMetricTypeCumulativeCounter, labelMap, 111),
doubleSFxDataPoint("prometheus_summary_sum", &sfxMetricTypeCumulativeCounter, labelMap, 123.4),
doubleSFxDataPoint("prometheus_summary_quantile", &sfxMetricTypeGauge,
maps.MergeStringMaps(map[string]string{quantileDimensionKey: "0.25"}, labelMap), 0),
doubleSFxDataPoint("prometheus_summary_quantile", &sfxMetricTypeGauge,
maps.MergeStringMaps(map[string]string{quantileDimensionKey: "0.5"}, labelMap), 1),
doubleSFxDataPoint("prometheus_summary_quantile", &sfxMetricTypeGauge,
maps.MergeStringMaps(map[string]string{quantileDimensionKey: "0.75"}, labelMap), 2),
doubleSFxDataPoint("prometheus_summary_quantile", &sfxMetricTypeGauge,
maps.MergeStringMaps(map[string]string{quantileDimensionKey: "1"}, labelMap), 3),
},
},
{
name: "empty_summary",
metricsFn: func() pmetric.Metrics {
Expand All @@ -462,15 +374,13 @@ func Test_FromMetrics(t *testing.T) {
},
wantSfxDataPoints: []*sfxpb.DataPoint{
int64SFxDataPoint("empty_summary_count", &sfxMetricTypeCumulativeCounter, labelMap, 11),
doubleSFxDataPoint("empty_summary", &sfxMetricTypeCumulativeCounter, labelMap, 12.3),
doubleSFxDataPoint("empty_summary_sum", &sfxMetricTypeCumulativeCounter, labelMap, 12.3),
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
from := &FromTranslator{
PrometheusCompatible: tt.promCompatible,
}
from := &FromTranslator{}
gotSfxDataPoints, err := from.FromMetrics(tt.metricsFn())
require.NoError(t, err)
// Sort SFx dimensions since they are built from maps and the order
Expand Down
11 changes: 11 additions & 0 deletions unreleased/remove-prom-compatible-argument.yaml
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
change_type: breaking

# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver)
component: pkg/translator/signalfx

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: "Remove no longer necessary prom compatible configuration from `FromTranslator`"

# One or more tracking issues related to the change
issues: [12671]
11 changes: 11 additions & 0 deletions unreleased/remove-prom-compatible-gate.yaml
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
change_type: breaking

# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver)
component: signalfxexporter

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: "Remove no longer necessary feature gate for prom compatible metrics"

# One or more tracking issues related to the change
issues: [12671]

0 comments on commit 29f4773

Please sign in to comment.