Skip to content

Signalfx exporter: remove misleading warnings #869

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Aug 30, 2020
Merged

Signalfx exporter: remove misleading warnings #869

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 8 additions & 29 deletions exporter/signalfxexporter/translation/converter.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
package translation

import (
"fmt"
"math"
"strconv"
"strings"
Expand Down Expand Up @@ -110,8 +109,6 @@ func (c *MetricsConverter) metricDataToSfxDataPoints(md consumerdata.MetricsData
sfxDataPoints []*sfxpb.DataPoint,
numDroppedTimeSeries int,
) {
var err error

// Labels from Node and Resource.
// TODO: Options to add lib, service name, etc as dimensions?
// Q.: what about resource type?
Expand Down Expand Up @@ -183,29 +180,15 @@ func (c *MetricsConverter) metricDataToSfxDataPoints(md consumerdata.MetricsData
metricDataPoints = append(metricDataPoints, sfxDataPoint)

case *metricspb.Point_DistributionValue:
metricDataPoints, err = appendDistributionValues(
metricDataPoints = appendDistributionValues(
metricDataPoints,
sfxDataPoint,
pv.DistributionValue)
if err != nil {
numDroppedTimeSeries++
c.logger.Warn(
"Timeseries for distribution metric dropped",
zap.Error(err),
zap.String("metric", sfxDataPoint.Metric))
}
case *metricspb.Point_SummaryValue:
metricDataPoints, err = appendSummaryValues(
metricDataPoints = appendSummaryValues(
metricDataPoints,
sfxDataPoint,
pv.SummaryValue)
if err != nil {
numDroppedTimeSeries++
c.logger.Warn(
"Timeseries for summary metric dropped",
zap.Error(err),
zap.String("metric", sfxDataPoint.Metric))
}
default:
numDroppedTimeSeries++
c.logger.Warn(
Expand Down Expand Up @@ -254,7 +237,7 @@ func appendDistributionValues(
sfxDataPoints []*sfxpb.DataPoint,
sfxBaseDataPoint *sfxpb.DataPoint,
distributionValue *metricspb.DistributionValue,
) ([]*sfxpb.DataPoint, error) {
) []*sfxpb.DataPoint {

// Translating distribution values per symmetrical recommendations to Prometheus:
// https://docs.signalfx.com/en/latest/integrations/agent/monitors/prometheus-exporter.html#overview
Expand All @@ -276,9 +259,7 @@ func appendDistributionValues(
metricName := sfxBaseDataPoint.Metric + "_bucket"
explicitBuckets := distributionValue.BucketOptions.GetExplicit()
if explicitBuckets == nil {
return sfxDataPoints, fmt.Errorf(
"unknown bucket options type for metric %q",
sfxBaseDataPoint.Metric)
return sfxDataPoints
}
bounds := explicitBuckets.Bounds
sfxBounds := make([]string, len(bounds)+1)
Expand Down Expand Up @@ -308,14 +289,14 @@ func appendDistributionValues(
sfxDataPoints = append(sfxDataPoints, &bucketDP)
}

return sfxDataPoints, nil
return sfxDataPoints
}

func appendSummaryValues(
sfxDataPoints []*sfxpb.DataPoint,
sfxBaseDataPoint *sfxpb.DataPoint,
summaryValue *metricspb.SummaryValue,
) ([]*sfxpb.DataPoint, error) {
) []*sfxpb.DataPoint {

// Translating summary values per symmetrical recommendations to Prometheus:
// https://docs.signalfx.com/en/latest/integrations/agent/monitors/prometheus-exporter.html#overview
Expand All @@ -335,9 +316,7 @@ func appendSummaryValues(
// and will include a dimension called quantile that specifies the quantile.
percentiles := summaryValue.GetSnapshot().GetPercentileValues()
if percentiles == nil {
return sfxDataPoints, fmt.Errorf(
"unknown percentiles values for summary metric %q",
sfxBaseDataPoint.Metric)
return sfxDataPoints
}
metricName := sfxBaseDataPoint.Metric + "_quantile"
for _, quantile := range percentiles {
Expand All @@ -362,7 +341,7 @@ func appendSummaryValues(
sfxDataPoints = append(sfxDataPoints, &quantileDP)
}

return sfxDataPoints, nil
return sfxDataPoints
}

func appendTotalAndSum(
Expand Down
125 changes: 56 additions & 69 deletions exporter/signalfxexporter/translation/converter_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,19 @@ func Test_MetricDataToSignalFxV2(t *testing.T) {
values,
metricstestutil.DistPt(tsUnix, distributionBounds, distributionCounts))

distributionValueNoBuckets := metricspb.DistributionValue{
Count: 2,
Sum: 10,
}
distributionNoBuckets := metricstestutil.Timeseries(
tsUnix,
values,
&metricspb.Point{
Timestamp: metricstestutil.Timestamp(tsUnix),
Value: &metricspb.Point_DistributionValue{DistributionValue: &distributionValueNoBuckets},
},
)

summaryTimeSeries := metricstestutil.Timeseries(
tsUnix,
values,
Expand All @@ -68,6 +81,19 @@ func Test_MetricDataToSignalFxV2(t *testing.T) {
[]float64{90, 95, 99, 99.9},
[]float64{100, 6, 4, 1}))

summaryValueNoQuantiles := metricspb.SummaryValue{
Sum: &wrapperspb.DoubleValue{Value: 111},
Count: &wrapperspb.Int64Value{Value: 11},
}
summaryNoQuantiles := metricstestutil.Timeseries(
tsUnix,
values,
&metricspb.Point{
Timestamp: metricstestutil.Timestamp(tsUnix),
Value: &metricspb.Point_SummaryValue{SummaryValue: &summaryValueNoQuantiles},
},
)

tests := []struct {
name string
metricsDataFn func() []consumerdata.MetricsData
Expand Down Expand Up @@ -173,6 +199,19 @@ func Test_MetricDataToSignalFxV2(t *testing.T) {
expectedFromDistribution("gauge_distrib", tsMSecs, keys, values, distributionTimeSeries),
expectedFromDistribution("cumulative_distrib", tsMSecs, keys, values, distributionTimeSeries)...),
},
{
name: "distribution_no_buckets",
metricsDataFn: func() []consumerdata.MetricsData {
return []consumerdata.MetricsData{
{
Metrics: []*metricspb.Metric{
metricstestutil.GaugeDist("invalid_distrib", keys, distributionNoBuckets),
},
},
}
},
wantSfxDataPoints: expectedFromDistribution("invalid_distrib", tsMSecs, keys, values, distributionNoBuckets),
},
{
name: "summary",
metricsDataFn: func() []consumerdata.MetricsData {
Expand All @@ -186,6 +225,19 @@ func Test_MetricDataToSignalFxV2(t *testing.T) {
},
wantSfxDataPoints: expectedFromSummary("summary", tsMSecs, keys, values, summaryTimeSeries),
},
{
name: "summary_no_quantiles",
metricsDataFn: func() []consumerdata.MetricsData {
return []consumerdata.MetricsData{
{
Metrics: []*metricspb.Metric{
metricstestutil.Summary("summary_no_quantiles", keys, summaryNoQuantiles),
},
},
}
},
wantSfxDataPoints: expectedFromSummary("summary_no_quantiles", tsMSecs, keys, values, summaryNoQuantiles),
},
}
c := NewMetricsConverter(logger, nil)
for _, tt := range tests {
Expand Down Expand Up @@ -343,6 +395,9 @@ func expectedFromDistribution(
distributionValue.Sum))

explicitBuckets := distributionValue.BucketOptions.GetExplicit()
if explicitBuckets == nil {
return dps
}
for i := 0; i < len(explicitBuckets.Bounds); i++ {
dps = append(dps,
int64SFxDataPoint(metricName+"_bucket", ts, &sfxMetricTypeCumulativeCounter,
Expand All @@ -367,9 +422,7 @@ func expectedFromSummary(
) []*sfxpb.DataPoint {
summaryValue := summaryTimeSeries.Points[0].GetSummaryValue()

// Two additional data points: one for count and one for sum.
const extraDataPoints = 2
dps := make([]*sfxpb.DataPoint, 0, len(summaryValue.Snapshot.PercentileValues)+extraDataPoints)
dps := []*sfxpb.DataPoint{}

dps = append(dps,
int64SFxDataPoint(metricName+"_count", ts, &sfxMetricTypeCumulativeCounter, keys, values,
Expand All @@ -389,72 +442,6 @@ func expectedFromSummary(
return dps
}

func Test_InvalidDistribution_NoExplicitBuckets(t *testing.T) {
logger := zap.NewNop()
unixSecs := int64(1574092046)
unixNSecs := int64(11 * time.Millisecond)
tsUnix := time.Unix(unixSecs, unixNSecs)
keys := []string{"k0", "k1"}
values := []string{"v0", "v1"}
buckets := make([]*metricspb.DistributionValue_Bucket, 2)

distrValue := &metricspb.DistributionValue{
BucketOptions: &metricspb.DistributionValue_BucketOptions{
Type: &metricspb.DistributionValue_BucketOptions_Explicit_{
Explicit: nil,
},
},
Count: 42,
Sum: 42,
Buckets: buckets,
}
point := &metricspb.Point{Timestamp: metricstestutil.Timestamp(tsUnix), Value: &metricspb.Point_DistributionValue{DistributionValue: distrValue}}
assert.Nil(t, point.GetDistributionValue().BucketOptions.GetExplicit())
metricData := []consumerdata.MetricsData{
{
Metrics: []*metricspb.Metric{
metricstestutil.GaugeDist("gauge_distrib", keys, metricstestutil.Timeseries(
tsUnix,
values,
point)),
},
},
}
c := NewMetricsConverter(logger, nil)
_, gotNumDroppedTimeSeries := c.MetricDataToSignalFxV2(metricData)
assert.Equal(t, 1, gotNumDroppedTimeSeries)
}

func Test_InvalidSummary_NoPercentileValues(t *testing.T) {
logger := zap.NewNop()
unixSecs := int64(1574092046)
unixNSecs := int64(11 * time.Millisecond)
tsUnix := time.Unix(unixSecs, unixNSecs)
keys := []string{"k0", "k1"}
values := []string{"v0", "v1"}

summaryValue := &metricspb.SummaryValue{
Count: &wrapperspb.Int64Value{Value: 42},
Sum: &wrapperspb.DoubleValue{Value: 42},
Snapshot: &metricspb.SummaryValue_Snapshot{},
}
point := &metricspb.Point{Timestamp: metricstestutil.Timestamp(tsUnix), Value: &metricspb.Point_SummaryValue{SummaryValue: summaryValue}}
assert.Nil(t, point.GetSummaryValue().GetSnapshot().GetPercentileValues())
metricData := []consumerdata.MetricsData{
{
Metrics: []*metricspb.Metric{
metricstestutil.Summary("summary", keys, metricstestutil.Timeseries(
tsUnix,
values,
point)),
},
},
}
c := NewMetricsConverter(logger, nil)
_, gotNumDroppedTimeSeries := c.MetricDataToSignalFxV2(metricData)
assert.Equal(t, 1, gotNumDroppedTimeSeries)
}

func Test_InvalidPoint_NoValue(t *testing.T) {
logger := zap.NewNop()
unixSecs := int64(1574092046)
Expand Down