Skip to content

Commit

Permalink
Drop datapoints that have more than 36 dimensions
Browse files Browse the repository at this point in the history
Signed-off-by: Dani Louca <dlouca@splunk.com>
  • Loading branch information
dloucasfx committed Oct 3, 2022
1 parent d31a5c3 commit ee650b9
Show file tree
Hide file tree
Showing 3 changed files with 85 additions and 5 deletions.
16 changes: 16 additions & 0 deletions .chloggen/signalfx-exporter.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
change_type: enhancement

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

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: Drop datapoints that have more than 36 dimensions and log a message when agent is set to debug

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

# (Optional) One or more lines of additional information to render under the primary note.
# These lines will be padded with 2 spaces and then inserted directly into the document.
# Use pipe (|) for multiline entries.
subtext: Additionally, the metric and dimension validation failures are now logged in Debug instead of Warn
25 changes: 20 additions & 5 deletions exporter/signalfxexporter/internal/translation/converter.go
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,7 @@ const (
maxMetricNameLength = 256
maxDimensionNameLength = 128
maxDimensionValueLength = 256
maxNumberOfDimensions = 36
)

var (
Expand All @@ -183,6 +184,8 @@ var (
"dimension name longer than %d characters", maxDimensionNameLength)
invalidDimensionValueReason = fmt.Sprintf(
"dimension value longer than %d characters", maxDimensionValueLength)
invalidNumberOfDimensions = fmt.Sprintf(
"number of dimensions is larger than %d", maxNumberOfDimensions)
)

type datapointValidator struct {
Expand All @@ -196,11 +199,11 @@ func newDatapointValidator(logger *zap.Logger, nonAlphanumericDimChars string) *

// sanitizeDataPoints sanitizes datapoints prior to dispatching them to the backend.
// Datapoints that do not conform to the requirements are removed. This method drops
// datapoints with metric name greater than 256 characters.
// datapoints with metric name greater than 256 characters and number of dimensions greater than 36.
func (dpv *datapointValidator) sanitizeDataPoints(dps []*sfxpb.DataPoint) []*sfxpb.DataPoint {
resultDatapointsLen := 0
for dpIndex, dp := range dps {
if dpv.isValidMetricName(dp.Metric) {
if dpv.isValidMetricName(dp.Metric) && dpv.isValidNumberOfDimension(dp) {
dp.Dimensions = dpv.sanitizeDimensions(dp.Dimensions)
if resultDatapointsLen < dpIndex {
dps[resultDatapointsLen] = dp
Expand Down Expand Up @@ -234,7 +237,7 @@ func (dpv *datapointValidator) sanitizeDimensions(dimensions []*sfxpb.Dimension)

func (dpv *datapointValidator) isValidMetricName(name string) bool {
if len(name) > maxMetricNameLength {
dpv.logger.Warn("dropping datapoint",
dpv.logger.Debug("dropping datapoint",
zap.String("reason", invalidMetricNameReason),
zap.String("metric_name", name),
zap.Int("metric_name_length", len(name)),
Expand All @@ -244,13 +247,25 @@ func (dpv *datapointValidator) isValidMetricName(name string) bool {
return true
}

func (dpv *datapointValidator) isValidNumberOfDimension(dp *sfxpb.DataPoint) bool {
if len(dp.Dimensions) > maxNumberOfDimensions {
dpv.logger.Debug("dropping datapoint",
zap.String("reason", invalidNumberOfDimensions),
zap.String("datapoint", DatapointToString(dp)),
zap.Int("number_of_dimensions", len(dp.Dimensions)),
)
return false
}
return true
}

func (dpv *datapointValidator) isValidDimension(dimension *sfxpb.Dimension) bool {
return dpv.isValidDimensionName(dimension.Key) && dpv.isValidDimensionValue(dimension.Value, dimension.Key)
}

func (dpv *datapointValidator) isValidDimensionName(name string) bool {
if len(name) > maxDimensionNameLength {
dpv.logger.Warn("dropping dimension",
dpv.logger.Debug("dropping dimension",
zap.String("reason", invalidDimensionNameReason),
zap.String("dimension_name", name),
zap.Int("dimension_name_length", len(name)),
Expand All @@ -262,7 +277,7 @@ func (dpv *datapointValidator) isValidDimensionName(name string) bool {

func (dpv *datapointValidator) isValidDimensionValue(value, name string) bool {
if len(value) > maxDimensionValueLength {
dpv.logger.Warn("dropping dimension",
dpv.logger.Debug("dropping dimension",
zap.String("dimension_name", name),
zap.String("reason", invalidDimensionValueReason),
zap.String("dimension_value", value),
Expand Down
49 changes: 49 additions & 0 deletions exporter/signalfxexporter/internal/translation/converter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ import (
"go.opentelemetry.io/collector/pdata/pmetric"
conventions "go.opentelemetry.io/collector/semconv/v1.6.1"
"go.uber.org/zap"
"go.uber.org/zap/zapcore"
"go.uber.org/zap/zaptest/observer"

"github.com/open-telemetry/opentelemetry-collector-contrib/exporter/signalfxexporter/internal/translation/dpfilters"
"github.com/open-telemetry/opentelemetry-collector-contrib/internal/common/maps"
Expand Down Expand Up @@ -726,6 +728,53 @@ func TestDimensionKeyCharsWithPeriod(t *testing.T) {

}

func TestInvalidNumberOfDimensions(t *testing.T) {
observedZapCore, observedLogs := observer.New(zap.DebugLevel)
logger := zap.New(observedZapCore)

md := pmetric.NewMetrics()
m := md.ResourceMetrics().AppendEmpty().ScopeMetrics().AppendEmpty().Metrics().AppendEmpty()
m.SetName("valid")
dp := m.SetEmptyGauge().DataPoints().AppendEmpty()
dp.SetIntValue(123)
for i := 0; i < 10; i++ {
dp.Attributes().PutString(fmt.Sprint("dim_key_", i), fmt.Sprint("dim_val_", i))
}
c, err := NewMetricsConverter(logger, nil, nil, nil, "_-.")
require.NoError(t, err)
assert.EqualValues(t, 1, len(c.MetricsToSignalFxV2(md)))
// No log message should be printed
require.Equal(t, 0, observedLogs.Len())

mdInvalid := pmetric.NewMetrics()
mInvalid := mdInvalid.ResourceMetrics().AppendEmpty().ScopeMetrics().AppendEmpty().Metrics().AppendEmpty()
m.SetName("invalid")
dpInvalid := mInvalid.SetEmptyGauge().DataPoints().AppendEmpty()
dp.SetIntValue(123)

// SFX datapoint is used for log validation
gaugeType := sfxpb.MetricType_GAUGE
dpSFX := &sfxpb.DataPoint{
MetricType: &gaugeType,
Dimensions: make([]*sfxpb.Dimension, 0, 37),
}
for i := 0; i < 37; i++ {
dpInvalid.Attributes().PutString(fmt.Sprint("dim_key_", i), fmt.Sprint("dim_val_", i))
dpSFX.Dimensions = append(dpSFX.Dimensions, &sfxpb.Dimension{
Key: fmt.Sprint("dim_key_", i),
Value: fmt.Sprint("dim_val_", i),
})
}
assert.EqualValues(t, 0, len(c.MetricsToSignalFxV2(mdInvalid)))
require.Equal(t, 1, observedLogs.Len())
assert.Equal(t, "dropping datapoint", observedLogs.All()[0].Message)
assert.ElementsMatch(t, []zap.Field{
{Type: zapcore.StringType, Key: "reason", String: invalidNumberOfDimensions},
{Type: zapcore.StringType, Key: "datapoint", String: DatapointToString(dpSFX)},
{Type: zapcore.Int64Type, Key: "number_of_dimensions", Integer: 37},
}, observedLogs.All()[0].Context)
}

func sortDimensions(points []*sfxpb.DataPoint) {
for _, point := range points {
if point.Dimensions == nil {
Expand Down

0 comments on commit ee650b9

Please sign in to comment.