From b2bb369b9ff47e17d52231a95f9bbeca4afe0beb Mon Sep 17 00:00:00 2001 From: Antoine Toulme Date: Fri, 2 Feb 2024 21:32:48 -0800 Subject: [PATCH] Remove functions of output related to sending traces (#4241) --- .../pkg/monitors/types/output.go | 5 - internal/signalfx-agent/pkg/neotest/output.go | 15 -- pkg/receiver/smartagentreceiver/output.go | 44 +----- .../smartagentreceiver/output_test.go | 11 -- .../smartagentreceiver/output_traces_test.go | 146 ------------------ 5 files changed, 1 insertion(+), 220 deletions(-) diff --git a/internal/signalfx-agent/pkg/monitors/types/output.go b/internal/signalfx-agent/pkg/monitors/types/output.go index 6756a059eb..2d897ac7a8 100644 --- a/internal/signalfx-agent/pkg/monitors/types/output.go +++ b/internal/signalfx-agent/pkg/monitors/types/output.go @@ -18,11 +18,6 @@ type Output interface { SendSpans(spans ...*trace.Span) SendDimensionUpdate(dim *Dimension) AddExtraDimension(key string, value string) - RemoveExtraDimension(key string) - AddExtraSpanTag(key string, value string) - RemoveExtraSpanTag(key string) - AddDefaultSpanTag(key string, value string) - RemoveDefaultSpanTag(key string) } // FilteringOutput is Output enhanced with additional filtering mechanisms. diff --git a/internal/signalfx-agent/pkg/neotest/output.go b/internal/signalfx-agent/pkg/neotest/output.go index 405f4292c8..bfbdb80398 100644 --- a/internal/signalfx-agent/pkg/neotest/output.go +++ b/internal/signalfx-agent/pkg/neotest/output.go @@ -67,21 +67,6 @@ func (to *TestOutput) SendDimensionUpdate(dims *types.Dimension) { // AddExtraDimension is a noop here func (to *TestOutput) AddExtraDimension(key, value string) {} -// RemoveExtraDimension is a noop here -func (to *TestOutput) RemoveExtraDimension(key string) {} - -// AddExtraSpanTag is a noop here -func (to *TestOutput) AddExtraSpanTag(key, value string) {} - -// RemoveExtraSpanTag is a noop here -func (to *TestOutput) RemoveExtraSpanTag(key string) {} - -// AddDefaultSpanTag is a noop here -func (to *TestOutput) AddDefaultSpanTag(key, value string) {} - -// RemoveDefaultSpanTag is a noop here -func (to *TestOutput) RemoveDefaultSpanTag(key string) {} - // FlushDatapoints returns all of the datapoints injected into the channel so // far. func (to *TestOutput) FlushDatapoints() []*datapoint.Datapoint { diff --git a/pkg/receiver/smartagentreceiver/output.go b/pkg/receiver/smartagentreceiver/output.go index 2013b10dbe..c476070e4f 100644 --- a/pkg/receiver/smartagentreceiver/output.go +++ b/pkg/receiver/smartagentreceiver/output.go @@ -44,8 +44,6 @@ type output struct { nextLogsConsumer consumer.Logs nextTracesConsumer consumer.Traces extraDimensions map[string]string - extraSpanTags map[string]string - defaultSpanTags map[string]string logger *zap.Logger reporter *receiverhelper.ObsReport translator converter.Translator @@ -79,8 +77,6 @@ func newOutput( logger: params.Logger, translator: converter.NewTranslator(params.Logger), extraDimensions: map[string]string{}, - extraSpanTags: map[string]string{}, - defaultSpanTags: map[string]string{}, monitorFiltering: filtering, reporter: obsReceiver, }, nil @@ -190,8 +186,6 @@ func (out *output) Copy() types.Output { out.logger.Debug("Copying out", zap.Any("out", out)) cp := *out cp.extraDimensions = utils.CloneStringMap(out.extraDimensions) - cp.extraSpanTags = utils.CloneStringMap(out.extraSpanTags) - cp.defaultSpanTags = utils.CloneStringMap(out.defaultSpanTags) return &cp } @@ -210,7 +204,7 @@ func (out *output) SendDatapoints(datapoints ...*datapoint.Datapoint) { metrics, err := out.translator.ToMetrics(datapoints) if err != nil { - out.logger.Error("error converting SFx datapoints to ptrace.Traces", zap.Error(err)) + out.logger.Error("error converting SFx datapoints to pmetric.Metrics", zap.Error(err)) } numPoints := metrics.DataPointCount() @@ -239,21 +233,6 @@ func (out *output) SendSpans(spans ...*trace.Span) { return } - for _, span := range spans { - if span.Tags == nil { - span.Tags = map[string]string{} - } - - for name, value := range out.defaultSpanTags { - // If the tags are already set, don't override - if _, ok := span.Tags[name]; !ok { - span.Tags[name] = value - } - } - - span.Tags = utils.MergeStringMaps(span.Tags, out.extraSpanTags) - } - traces, err := out.translator.ToTraces(spans) if err != nil { out.logger.Error("error converting SFx spans to ptrace.Traces", zap.Error(err)) @@ -284,27 +263,6 @@ func (out *output) AddExtraDimension(key, value string) { out.extraDimensions[key] = value } -func (out *output) RemoveExtraDimension(key string) { - out.logger.Debug("Removing extra dimension", zap.String("key", key)) - delete(out.extraDimensions, key) -} - -func (out *output) AddExtraSpanTag(key, value string) { - out.extraSpanTags[key] = value -} - -func (out *output) RemoveExtraSpanTag(key string) { - delete(out.extraSpanTags, key) -} - -func (out *output) AddDefaultSpanTag(key, value string) { - out.defaultSpanTags[key] = value -} - -func (out *output) RemoveDefaultSpanTag(key string) { - delete(out.defaultSpanTags, key) -} - func (out *output) filterDatapoints(datapoints []*datapoint.Datapoint) []*datapoint.Datapoint { filteredDatapoints := make([]*datapoint.Datapoint, 0, len(datapoints)) for _, dp := range datapoints { diff --git a/pkg/receiver/smartagentreceiver/output_test.go b/pkg/receiver/smartagentreceiver/output_test.go index 81cc85fd77..316dd5d184 100644 --- a/pkg/receiver/smartagentreceiver/output_test.go +++ b/pkg/receiver/smartagentreceiver/output_test.go @@ -59,11 +59,6 @@ func TestOutput(t *testing.T) { output.SendSpans() output.SendDimensionUpdate(new(types.Dimension)) output.AddExtraDimension("", "") - output.RemoveExtraDimension("") - output.AddExtraSpanTag("", "") - output.RemoveExtraSpanTag("") - output.AddDefaultSpanTag("", "") - output.RemoveDefaultSpanTag("") } func TestHasEnabledMetric(t *testing.T) { @@ -137,8 +132,6 @@ func TestExtraDimensions(t *testing.T) { require.NoError(t, err) assert.Empty(t, o.extraDimensions) - o.RemoveExtraDimension("not_a_known_dimension_name") - o.AddExtraDimension("a_dimension_name", "a_value") assert.Equal(t, "a_value", o.extraDimensions["a_dimension_name"]) @@ -146,10 +139,6 @@ func TestExtraDimensions(t *testing.T) { require.True(t, ok) assert.Equal(t, "a_value", cp.extraDimensions["a_dimension_name"]) - cp.RemoveExtraDimension("a_dimension_name") - assert.Empty(t, cp.extraDimensions["a_dimension_name"]) - assert.Equal(t, "a_value", o.extraDimensions["a_dimension_name"]) - cp.AddExtraDimension("another_dimension_name", "another_dimension_value") assert.Equal(t, "another_dimension_value", cp.extraDimensions["another_dimension_name"]) assert.Empty(t, o.extraDimensions["another_dimension_name"]) diff --git a/pkg/receiver/smartagentreceiver/output_traces_test.go b/pkg/receiver/smartagentreceiver/output_traces_test.go index d3dcd9b862..ebef5956e3 100644 --- a/pkg/receiver/smartagentreceiver/output_traces_test.go +++ b/pkg/receiver/smartagentreceiver/output_traces_test.go @@ -15,7 +15,6 @@ package smartagentreceiver import ( - "encoding/hex" "fmt" "testing" @@ -37,151 +36,6 @@ func TestSendSpansWithoutNextTracesConsumer(t *testing.T) { output.SendSpans(&trace.Span{TraceID: "12345678", ID: "23456789"}) // doesn't panic } -func TestExtraSpanTags(t *testing.T) { - o, err := newOutput( - Config{}, fakeMonitorFiltering(), consumertest.NewNop(), consumertest.NewNop(), - consumertest.NewNop(), componenttest.NewNopHost(), newReceiverCreateSettings(""), - ) - require.NoError(t, err) - assert.Empty(t, o.extraSpanTags) - - o.RemoveExtraSpanTag("not_a_known_tag") - - o.AddExtraSpanTag("a_tag", "a_value") - assert.Equal(t, "a_value", o.extraSpanTags["a_tag"]) - - cp, ok := o.Copy().(*output) - require.True(t, ok) - assert.Equal(t, "a_value", cp.extraSpanTags["a_tag"]) - - cp.RemoveExtraSpanTag("a_tag") - assert.Empty(t, cp.extraSpanTags["a_tag"]) - assert.Equal(t, "a_value", o.extraSpanTags["a_tag"]) - - cp.AddExtraSpanTag("another_tag", "another_tag_value") - assert.Equal(t, "another_tag_value", cp.extraSpanTags["another_tag"]) - assert.Empty(t, o.extraSpanTags["another_tag"]) -} - -func TestDefaultSpanTags(t *testing.T) { - o, err := newOutput( - Config{}, fakeMonitorFiltering(), consumertest.NewNop(), consumertest.NewNop(), - consumertest.NewNop(), componenttest.NewNopHost(), newReceiverCreateSettings(""), - ) - require.NoError(t, err) - assert.Empty(t, o.defaultSpanTags) - - o.RemoveDefaultSpanTag("not_a_known_tag") - - o.AddDefaultSpanTag("a_tag", "a_value") - assert.Equal(t, "a_value", o.defaultSpanTags["a_tag"]) - - cp, ok := o.Copy().(*output) - require.True(t, ok) - assert.Equal(t, "a_value", cp.defaultSpanTags["a_tag"]) - - cp.RemoveDefaultSpanTag("a_tag") - assert.Empty(t, cp.defaultSpanTags["a_tag"]) - assert.Equal(t, "a_value", o.defaultSpanTags["a_tag"]) - - cp.AddDefaultSpanTag("another_tag", "another_tag_value") - assert.Equal(t, "another_tag_value", cp.defaultSpanTags["another_tag"]) - assert.Empty(t, o.defaultSpanTags["another_tag"]) -} - -func TestSendSpansWithDefaultAndExtraSpanTags(t *testing.T) { - tracesSink := consumertest.TracesSink{} - output, err := newOutput( - Config{}, fakeMonitorFiltering(), consumertest.NewNop(), consumertest.NewNop(), - &tracesSink, componenttest.NewNopHost(), newReceiverCreateSettings(""), - ) - require.NoError(t, err) - output.AddExtraSpanTag("will_be_overridden", "added extra value (want)") - output.AddDefaultSpanTag("wont_be_overridden", "added default value") - - sfxSpan0 := trace.Span{ - TraceID: "12345678", - ID: "23456789", - Tags: map[string]string{ - "will_be_overridden": "span-provided value (don't want)", - "some_tag": "some_value", - }, - } - - sfxSpan1 := trace.Span{ - TraceID: "34567890", - ID: "45678901", - Tags: map[string]string{ - "wont_be_overridden": "span-provided value (want)", - }, - } - - sfxSpan2 := trace.Span{ - TraceID: "56789012", - ID: "67890123", - } - - // core zipkin pdata translation reverses span order - output.SendSpans(&sfxSpan2, &sfxSpan1, &sfxSpan0) - - received := tracesSink.AllTraces() - require.Equal(t, 1, len(received)) - trace := received[0] - assert.Equal(t, 3, trace.SpanCount()) - resourceSpans := trace.ResourceSpans() - require.Equal(t, 1, resourceSpans.Len()) - illSpansSlice := resourceSpans.At(0).ScopeSpans() - require.Equal(t, 1, illSpansSlice.Len()) - illSpans := illSpansSlice.At(0).Spans() - require.Equal(t, 3, illSpans.Len()) - - span0 := illSpans.At(0) - tid := span0.TraceID() - require.Equal(t, "00000000000000000000000012345678", hex.EncodeToString(tid[:])) - sid := span0.SpanID() - require.Equal(t, "0000000023456789", hex.EncodeToString(sid[:])) - - extraValue, containsExtraValue := span0.Attributes().Get("will_be_overridden") - require.True(t, containsExtraValue) - assert.Equal(t, "added extra value (want)", extraValue.Str()) - - defaultValue, containsDefaultValue := span0.Attributes().Get("wont_be_overridden") - require.True(t, containsDefaultValue) - assert.Equal(t, "added default value", defaultValue.Str()) - - value, containsValue := span0.Attributes().Get("some_tag") - require.True(t, containsValue) - assert.Equal(t, "some_value", value.Str()) - - span1 := illSpans.At(1) - tid = span1.TraceID() - require.Equal(t, "00000000000000000000000034567890", hex.EncodeToString(tid[:])) - sid = span1.SpanID() - require.Equal(t, "0000000045678901", hex.EncodeToString(sid[:])) - - extraValue, containsExtraValue = span1.Attributes().Get("will_be_overridden") - require.True(t, containsExtraValue) - assert.Equal(t, "added extra value (want)", extraValue.Str()) - - defaultValue, containsDefaultValue = span1.Attributes().Get("wont_be_overridden") - require.True(t, containsDefaultValue) - assert.Equal(t, "span-provided value (want)", defaultValue.Str()) - - span2 := illSpans.At(2) - tid = span2.TraceID() - require.Equal(t, "00000000000000000000000056789012", hex.EncodeToString(tid[:])) - sid = span2.SpanID() - require.Equal(t, "0000000067890123", hex.EncodeToString(sid[:])) - - extraValue, containsExtraValue = span2.Attributes().Get("will_be_overridden") - require.True(t, containsExtraValue) - assert.Equal(t, "added extra value (want)", extraValue.Str()) - - defaultValue, containsDefaultValue = span2.Attributes().Get("wont_be_overridden") - require.True(t, containsDefaultValue) - assert.Equal(t, "added default value", defaultValue.Str()) -} - func TestSendSpansWithConsumerError(t *testing.T) { obs, logs := observer.New(zap.DebugLevel) logger := zap.New(obs)