diff --git a/exporter/signalfxexporter/dimensions/dimclient.go b/exporter/signalfxexporter/dimensions/dimclient.go index 114d66a4137b..47880fdac7c9 100644 --- a/exporter/signalfxexporter/dimensions/dimclient.go +++ b/exporter/signalfxexporter/dimensions/dimclient.go @@ -72,7 +72,7 @@ type DimensionClient struct { TotalSuccessfulUpdates int64 logUpdates bool logger *zap.Logger - metricTranslator *translation.MetricTranslator + metricsConverter translation.MetricsConverter } type queuedDimension struct { @@ -87,12 +87,11 @@ type DimensionClientOptions struct { Logger *zap.Logger SendDelay int PropertiesMaxBuffered int - MetricTranslator *translation.MetricTranslator + MetricsConverter translation.MetricsConverter } // NewDimensionClient returns a new client func NewDimensionClient(ctx context.Context, options DimensionClientOptions) *DimensionClient { - client := &http.Client{ Timeout: 10 * time.Second, Transport: &http.Transport{ @@ -122,7 +121,7 @@ func NewDimensionClient(ctx context.Context, options DimensionClientOptions) *Di now: time.Now, logger: options.Logger, logUpdates: options.LogUpdates, - metricTranslator: options.MetricTranslator, + metricsConverter: options.MetricsConverter, } } diff --git a/exporter/signalfxexporter/dimensions/metadata.go b/exporter/signalfxexporter/dimensions/metadata.go index b437b1fdadb0..b5119a64b29e 100644 --- a/exporter/signalfxexporter/dimensions/metadata.go +++ b/exporter/signalfxexporter/dimensions/metadata.go @@ -32,25 +32,17 @@ type MetadataUpdateClient interface { func getDimensionUpdateFromMetadata( metadata metadata.MetadataUpdate, - metricTranslator *translation.MetricTranslator, -) *DimensionUpdate { + metricsConverter translation.MetricsConverter) *DimensionUpdate { properties, tags := getPropertiesAndTags(metadata) return &DimensionUpdate{ - Name: translateDimension(metricTranslator, metadata.ResourceIDKey), + Name: metricsConverter.ConvertDimension(metadata.ResourceIDKey), Value: string(metadata.ResourceID), Properties: properties, Tags: tags, } } -func translateDimension(metricTranslator *translation.MetricTranslator, dim string) string { - if metricTranslator != nil { - return metricTranslator.TranslateDimension(dim) - } - return dim -} - const ( oTelK8sServicePrefix = "k8s.service." sfxK8sServicePrefix = "kubernetes_service_" @@ -116,7 +108,7 @@ func getPropertiesAndTags(kmu metadata.MetadataUpdate) (map[string]*string, map[ func (dc *DimensionClient) PushMetadata(metadata []*metadata.MetadataUpdate) error { var errs []error for _, m := range metadata { - dimensionUpdate := getDimensionUpdateFromMetadata(*m, dc.metricTranslator) + dimensionUpdate := getDimensionUpdateFromMetadata(*m, dc.metricsConverter) if dimensionUpdate.Name == "" || dimensionUpdate.Value == "" { atomic.AddInt64(&dc.TotalInvalidDimensions, int64(1)) diff --git a/exporter/signalfxexporter/dimensions/metadata_test.go b/exporter/signalfxexporter/dimensions/metadata_test.go index 25bbf98ad4bd..aaa0be1de46b 100644 --- a/exporter/signalfxexporter/dimensions/metadata_test.go +++ b/exporter/signalfxexporter/dimensions/metadata_test.go @@ -18,6 +18,9 @@ import ( "reflect" "testing" + "github.com/stretchr/testify/require" + "go.uber.org/zap" + "github.com/open-telemetry/opentelemetry-collector-contrib/exporter/signalfxexporter/translation" metadata "github.com/open-telemetry/opentelemetry-collector-contrib/pkg/experimentalmetricmetadata" ) @@ -205,7 +208,16 @@ func TestGetDimensionUpdateFromMetadata(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got := getDimensionUpdateFromMetadata(tt.args.metadata, tt.args.metricTranslator) + + converter, err := translation.NewMetricsConverter( + zap.NewNop(), + tt.args.metricTranslator, + nil, + nil, + "-_.", + ) + require.NoError(t, err) + got := getDimensionUpdateFromMetadata(tt.args.metadata, *converter) if !reflect.DeepEqual(*got, *tt.want) { t.Errorf("getDimensionUpdateFromMetadata() = %v, want %v", *got, *tt.want) } diff --git a/exporter/signalfxexporter/exporter.go b/exporter/signalfxexporter/exporter.go index 2f5a1c23f562..515945ae7307 100644 --- a/exporter/signalfxexporter/exporter.go +++ b/exporter/signalfxexporter/exporter.go @@ -125,7 +125,7 @@ func newSignalFxExporter( // buffer a fixed number of updates. Might also be a good candidate // to make configurable. PropertiesMaxBuffered: 10000, - MetricTranslator: options.metricTranslator, + MetricsConverter: *converter, }) dimClient.Start() diff --git a/exporter/signalfxexporter/exporter_test.go b/exporter/signalfxexporter/exporter_test.go index e863c4c5a242..2053475b575b 100644 --- a/exporter/signalfxexporter/exporter_test.go +++ b/exporter/signalfxexporter/exporter_test.go @@ -25,6 +25,7 @@ import ( "net/url" "sort" "strconv" + "strings" "sync" "testing" "time" @@ -755,6 +756,15 @@ func generateLargeEventBatch() pdata.Logs { } func TestConsumeMetadata(t *testing.T) { + cfg := createDefaultConfig().(*Config) + converter, err := translation.NewMetricsConverter( + zap.NewNop(), + nil, + cfg.ExcludeMetrics, + cfg.IncludeMetrics, + cfg.NonAlphanumericDimensionChars, + ) + require.NoError(t, err) type args struct { metadata []*metadata.MetadataUpdate } @@ -762,13 +772,15 @@ func TestConsumeMetadata(t *testing.T) { payLoad map[string]interface{} } tests := []struct { - name string - fields fields - args args + name string + fields fields + args args + expectedDimensionKey string + expectedDimensionValue string }{ { - "Test property updates", - fields{ + name: "Test property updates", + fields: fields{ map[string]interface{}{ "customProperties": map[string]interface{}{ "prop.erty1": "val1", @@ -780,7 +792,7 @@ func TestConsumeMetadata(t *testing.T) { "tagsToRemove": nil, }, }, - args{ + args: args{ []*metadata.MetadataUpdate{ { ResourceIDKey: "key", @@ -800,10 +812,12 @@ func TestConsumeMetadata(t *testing.T) { }, }, }, + expectedDimensionKey: "key", + expectedDimensionValue: "id", }, { - "Test tag updates", - fields{ + name: "Test tag updates", + fields: fields{ map[string]interface{}{ "customProperties": map[string]interface{}{}, "tags": []interface{}{ @@ -814,7 +828,7 @@ func TestConsumeMetadata(t *testing.T) { }, }, }, - args{ + args: args{ []*metadata.MetadataUpdate{ { ResourceIDKey: "key", @@ -831,10 +845,12 @@ func TestConsumeMetadata(t *testing.T) { }, }, }, + expectedDimensionKey: "key", + expectedDimensionValue: "id", }, { - "Test quick successive updates", - fields{ + name: "Test quick successive updates", + fields: fields{ map[string]interface{}{ "customProperties": map[string]interface{}{ "property1": nil, @@ -849,7 +865,7 @@ func TestConsumeMetadata(t *testing.T) { }, }, }, - args{ + args: args{ []*metadata.MetadataUpdate{ { ResourceIDKey: "key", @@ -898,6 +914,45 @@ func TestConsumeMetadata(t *testing.T) { }, }, }, + expectedDimensionKey: "key", + expectedDimensionValue: "id", + }, + { + name: "Test updates on dimensions with nonalphanumeric characters (other than the default allow list)", + fields: fields{ + map[string]interface{}{ + "customProperties": map[string]interface{}{ + "prop.erty1": "val1", + "property2": nil, + "prop.erty3": "val33", + "property4": nil, + }, + "tags": nil, + "tagsToRemove": nil, + }, + }, + args: args{ + []*metadata.MetadataUpdate{ + { + ResourceIDKey: "k!e=y", + ResourceID: "id", + MetadataDelta: metadata.MetadataDelta{ + MetadataToAdd: map[string]string{ + "prop.erty1": "val1", + }, + MetadataToRemove: map[string]string{ + "property2": "val2", + }, + MetadataToUpdate: map[string]string{ + "prop.erty3": "val33", + "property4": "", + }, + }, + }, + }, + }, + expectedDimensionKey: "k_e_y", + expectedDimensionValue: "id", }, } for _, tt := range tests { @@ -911,6 +966,11 @@ func TestConsumeMetadata(t *testing.T) { b, err := ioutil.ReadAll(r.Body) assert.NoError(t, err) + // Test metadata updates are sent onto the right dimensions. + dimPair := strings.Split(r.RequestURI, "/")[3:5] + assert.Equal(t, tt.expectedDimensionKey, dimPair[0]) + assert.Equal(t, tt.expectedDimensionValue, dimPair[1]) + p := map[string]interface{}{ "customProperties": map[string]*string{}, "tags": []string{}, @@ -939,6 +999,7 @@ func TestConsumeMetadata(t *testing.T) { Logger: logger, SendDelay: 1, PropertiesMaxBuffered: 10, + MetricsConverter: *converter, }) dimClient.Start() diff --git a/exporter/signalfxexporter/translation/converter.go b/exporter/signalfxexporter/translation/converter.go index 2b6341912f47..abe924e07b2e 100644 --- a/exporter/signalfxexporter/translation/converter.go +++ b/exporter/signalfxexporter/translation/converter.go @@ -411,3 +411,11 @@ func timestampToSignalFx(ts pdata.Timestamp) int64 { // Convert nanosecs to millisecs. return int64(ts) / 1e6 } + +func (c *MetricsConverter) ConvertDimension(dim string) string { + res := dim + if c.metricTranslator != nil { + res = c.metricTranslator.translateDimension(dim) + } + return filterKeyChars(res, c.nonAlphanumericDimChars) +} diff --git a/exporter/signalfxexporter/translation/converter_test.go b/exporter/signalfxexporter/translation/converter_test.go index c479a3c3dce1..10877e28835e 100644 --- a/exporter/signalfxexporter/translation/converter_test.go +++ b/exporter/signalfxexporter/translation/converter_test.go @@ -912,3 +912,61 @@ func TestNewMetricsConverter(t *testing.T) { }) } } + +func TestMetricsConverter_ConvertDimension(t *testing.T) { + type fields struct { + metricTranslator *MetricTranslator + nonAlphanumericDimChars string + } + type args struct { + dim string + } + tests := []struct { + name string + fields fields + args args + want string + }{ + { + name: "No translations", + fields: fields{ + metricTranslator: nil, + nonAlphanumericDimChars: "_-", + }, + args: args{ + dim: "d.i.m", + }, + want: "d_i_m", + }, + { + name: "With translations", + fields: fields{ + metricTranslator: func() *MetricTranslator { + t, _ := NewMetricTranslator([]Rule{ + { + Action: ActionRenameDimensionKeys, + Mapping: map[string]string{ + "d.i.m": "di.m", + }, + }, + }, 0) + return t + }(), + nonAlphanumericDimChars: "_-", + }, + args: args{ + dim: "d.i.m", + }, + want: "di_m", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + c, err := NewMetricsConverter(zap.NewNop(), tt.fields.metricTranslator, nil, nil, tt.fields.nonAlphanumericDimChars) + require.NoError(t, err) + if got := c.ConvertDimension(tt.args.dim); got != tt.want { + t.Errorf("ConvertDimension() = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/exporter/signalfxexporter/translation/translator.go b/exporter/signalfxexporter/translation/translator.go index a99b291ae28e..3648de667b78 100644 --- a/exporter/signalfxexporter/translation/translator.go +++ b/exporter/signalfxexporter/translation/translator.go @@ -588,7 +588,7 @@ func ptToFloatVal(pt *sfxpb.DataPoint) *float64 { return &f } -func (mp *MetricTranslator) TranslateDimension(orig string) string { +func (mp *MetricTranslator) translateDimension(orig string) string { if translated, ok := mp.dimensionsMap[orig]; ok { return translated } diff --git a/exporter/signalfxexporter/translation/translator_test.go b/exporter/signalfxexporter/translation/translator_test.go index 57533a2e5ffb..c901598ebe4a 100644 --- a/exporter/signalfxexporter/translation/translator_test.go +++ b/exporter/signalfxexporter/translation/translator_test.go @@ -1923,14 +1923,14 @@ func TestTestTranslateDimension(t *testing.T) { }, 1) require.NoError(t, err) - assert.Equal(t, "new_dimension", mt.TranslateDimension("old_dimension")) - assert.Equal(t, "new.dimension", mt.TranslateDimension("old.dimension")) - assert.Equal(t, "another_dimension", mt.TranslateDimension("another_dimension")) + assert.Equal(t, "new_dimension", mt.translateDimension("old_dimension")) + assert.Equal(t, "new.dimension", mt.translateDimension("old.dimension")) + assert.Equal(t, "another_dimension", mt.translateDimension("another_dimension")) // Test no rename_dimension_keys translation rule mt, err = NewMetricTranslator([]Rule{}, 1) require.NoError(t, err) - assert.Equal(t, "old_dimension", mt.TranslateDimension("old_dimension")) + assert.Equal(t, "old_dimension", mt.translateDimension("old_dimension")) } func TestNewCalculateNewMetricErrors(t *testing.T) {