From e473ffd33b4e06dbb72d267df2607fa32771949c Mon Sep 17 00:00:00 2001 From: Tomasz Rojek Date: Thu, 21 Oct 2021 03:22:30 +0200 Subject: [PATCH 1/9] Add setStatus operation for spanprocessor --- processor/spanprocessor/README.md | 18 ++++++++++++++++-- processor/spanprocessor/config.go | 13 +++++++++++++ processor/spanprocessor/factory.go | 5 +++-- processor/spanprocessor/span.go | 8 ++++++++ processor/spanprocessor/span_test.go | 9 +++++++++ processor/spanprocessor/testdata/config.yaml | 6 ++++++ 6 files changed, 55 insertions(+), 4 deletions(-) diff --git a/processor/spanprocessor/README.md b/processor/spanprocessor/README.md index 6954d29c7feb..29025e93507f 100644 --- a/processor/spanprocessor/README.md +++ b/processor/spanprocessor/README.md @@ -2,14 +2,15 @@ Supported pipeline types: traces -The span processor modifies the span name based on its attributes or extract span attributes from the span name. Please refer to -[config.go](./config.go) for the config spec. +The span processor modifies the span name based on its attributes or extract span attributes from the span name. It also allows +to change span status. Please refer to [config.go](./config.go) for the config spec. It optionally supports the ability to [include/exclude spans](../README.md#includeexclude-spans). The following actions are supported: - `name`: Modify the name of attributes within a span +- `status`: Modify the status of the span ### Name a span @@ -96,5 +97,18 @@ span/to_attributes: - ^\/api\/v1\/document\/(?P.*)\/update$ ``` +### Set status for span + +Example: + +```yaml +# Set status allows to set specific status for a given span. +span/set_status: + status: + code: 2 + description: "some error description" +``` + + Refer to [config.yaml](./testdata/config.yaml) for detailed examples on using the processor. diff --git a/processor/spanprocessor/config.go b/processor/spanprocessor/config.go index d6f386576a8e..9a8ffadbcaa7 100644 --- a/processor/spanprocessor/config.go +++ b/processor/spanprocessor/config.go @@ -16,6 +16,7 @@ package spanprocessor import ( "go.opentelemetry.io/collector/config" + "go.opentelemetry.io/otel/codes" "github.com/open-telemetry/opentelemetry-collector-contrib/internal/coreinternal/processor/filterconfig" ) @@ -35,6 +36,10 @@ type Config struct { // Note: The field name is `Rename` to avoid collision with the Name() method // from config.NamedEntity Rename Name `mapstructure:"name"` + + // SetStatus specifies status which should be set for this span. Please check: + // https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/api.md#set-status + SetStatus *Status `mapstructure:"status"` } // Name specifies the attributes to use to re-name a span. @@ -80,6 +85,14 @@ type ToAttributes struct { BreakAfterMatch bool `mapstructure:"break_after_match"` } +type Status struct { + // Code is one of three values + Code codes.Code `mapstructure:"code"` + + // Optional description + Description string `mapstructure:"description"` +} + var _ config.Processor = (*Config)(nil) // Validate checks if the processor configuration is valid diff --git a/processor/spanprocessor/factory.go b/processor/spanprocessor/factory.go index 2abbfd6a32ae..3ab0e1f45fcd 100644 --- a/processor/spanprocessor/factory.go +++ b/processor/spanprocessor/factory.go @@ -35,7 +35,7 @@ var processorCapabilities = consumer.Capabilities{MutatesData: true} // is not specified. // TODO https://github.com/open-telemetry/opentelemetry-collector/issues/215 // Move this to the error package that allows for span name and field to be specified. -var errMissingRequiredField = errors.New("error creating \"span\" processor: either \"from_attributes\" or \"to_attributes\" must be specified in \"name:\"") +var errMissingRequiredField = errors.New("error creating \"span\" processor: either \"from_attributes\" or \"to_attributes\" must be specified in \"name:\" or \"setStatus\" must be specified") // NewFactory returns a new factory for the Span processor. func NewFactory() component.ProcessorFactory { @@ -62,7 +62,8 @@ func createTracesProcessor( // processor to be valid. If not set and not enforced, the processor would do no work. oCfg := cfg.(*Config) if len(oCfg.Rename.FromAttributes) == 0 && - (oCfg.Rename.ToAttributes == nil || len(oCfg.Rename.ToAttributes.Rules) == 0) { + (oCfg.Rename.ToAttributes == nil || len(oCfg.Rename.ToAttributes.Rules) == 0) && + oCfg.SetStatus == nil { return nil, errMissingRequiredField } diff --git a/processor/spanprocessor/span.go b/processor/spanprocessor/span.go index 7cc71619b44e..a5c643883209 100644 --- a/processor/spanprocessor/span.go +++ b/processor/spanprocessor/span.go @@ -97,6 +97,7 @@ func (sp *spanProcessor) processTraces(_ context.Context, td pdata.Traces) (pdat } sp.processFromAttributes(s) sp.processToAttributes(s) + sp.processUpdateStatus(s) } } } @@ -220,3 +221,10 @@ func (sp *spanProcessor) processToAttributes(span pdata.Span) { } } } + +func (sp *spanProcessor) processUpdateStatus(span pdata.Span) { + if sp.config.SetStatus != nil { + span.Status().SetCode(pdata.StatusCode(sp.config.SetStatus.Code)) + span.Status().SetMessage(sp.config.SetStatus.Description) + } +} diff --git a/processor/spanprocessor/span_test.go b/processor/spanprocessor/span_test.go index db850843d7ff..8227346dfe74 100644 --- a/processor/spanprocessor/span_test.go +++ b/processor/spanprocessor/span_test.go @@ -593,3 +593,12 @@ func TestSpanProcessor_skipSpan(t *testing.T) { runIndividualTestCase(t, tc, tp) } } + +func TestSpanProcessor_setStatusCode(t *testing.T) { + factory := NewFactory() + cfg := factory.CreateDefaultConfig() + oCfg := cfg.(*Config) + oCfg.SetStatus.Code = 2 + oCfg.SetStatus.Description = "someStatus" + // TODO: Work in progress +} diff --git a/processor/spanprocessor/testdata/config.yaml b/processor/spanprocessor/testdata/config.yaml index 59884cd2d3dd..9813e7868bf9 100644 --- a/processor/spanprocessor/testdata/config.yaml +++ b/processor/spanprocessor/testdata/config.yaml @@ -84,6 +84,12 @@ processors: rules: - "(?P.*?)$" + # This example changes status of a span to error and sets description + span/setstatus: + status: + code: 2 + description: "some error description" + exporters: nop: From 2ff92e69e61fee88929c16a1fc534ec831263281 Mon Sep 17 00:00:00 2001 From: Tomasz Rojek Date: Sat, 23 Oct 2021 04:41:13 +0200 Subject: [PATCH 2/9] Change code to have text format instead of numeric and add another example --- processor/spanprocessor/README.md | 6 +++-- processor/spanprocessor/config.go | 6 ++--- processor/spanprocessor/config_test.go | 25 ++++++++++++++++++++ processor/spanprocessor/factory.go | 6 +++++ processor/spanprocessor/span.go | 13 +++++++--- processor/spanprocessor/testdata/config.yaml | 19 +++++++++++---- 6 files changed, 63 insertions(+), 12 deletions(-) diff --git a/processor/spanprocessor/README.md b/processor/spanprocessor/README.md index 29025e93507f..3f0fe5e987c8 100644 --- a/processor/spanprocessor/README.md +++ b/processor/spanprocessor/README.md @@ -102,10 +102,12 @@ span/to_attributes: Example: ```yaml -# Set status allows to set specific status for a given span. +# Set status allows to set specific status for a given span. Possible values are +# Ok and Error as per +# https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/api.md#set-status span/set_status: status: - code: 2 + code: Error description: "some error description" ``` diff --git a/processor/spanprocessor/config.go b/processor/spanprocessor/config.go index 9a8ffadbcaa7..883a7f7e039f 100644 --- a/processor/spanprocessor/config.go +++ b/processor/spanprocessor/config.go @@ -16,7 +16,6 @@ package spanprocessor import ( "go.opentelemetry.io/collector/config" - "go.opentelemetry.io/otel/codes" "github.com/open-telemetry/opentelemetry-collector-contrib/internal/coreinternal/processor/filterconfig" ) @@ -86,8 +85,9 @@ type ToAttributes struct { } type Status struct { - // Code is one of three values - Code codes.Code `mapstructure:"code"` + // Code is one of two values "Ok" or "Err". Please check: + // https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/api.md#set-status + Code string `mapstructure:"code"` // Optional description Description string `mapstructure:"description"` diff --git a/processor/spanprocessor/config_test.go b/processor/spanprocessor/config_test.go index 17e678f7d395..57ecfdf8ddcc 100644 --- a/processor/spanprocessor/config_test.go +++ b/processor/spanprocessor/config_test.go @@ -87,6 +87,31 @@ func TestLoadConfig(t *testing.T) { }, }, }) + + // Set name + p4 := cfg.Processors[config.NewComponentIDWithName("span", "set_status_err")] + assert.Equal(t, p4, &Config{ + ProcessorSettings: config.NewProcessorSettings(config.NewComponentIDWithName("span", "set_status_err")), + SetStatus: &Status{ + Code: "Err", + Description: "some additional error description", + }, + }) + + p5 := cfg.Processors[config.NewComponentIDWithName("span", "set_status_ok")] + assert.Equal(t, p5, &Config{ + ProcessorSettings: config.NewProcessorSettings(config.NewComponentIDWithName("span", "set_status_ok")), + MatchConfig: filterconfig.MatchConfig{ + Include: &filterconfig.MatchProperties{ + Attributes: []filterconfig.Attribute{ + {Key: "http.status_code", Value: 400}, + }, + }, + }, + SetStatus: &Status{ + Code: "Ok", + }, + }) } func createMatchConfig(matchType filterset.MatchType) *filterset.Config { diff --git a/processor/spanprocessor/factory.go b/processor/spanprocessor/factory.go index 3ab0e1f45fcd..10cfec858a96 100644 --- a/processor/spanprocessor/factory.go +++ b/processor/spanprocessor/factory.go @@ -36,6 +36,7 @@ var processorCapabilities = consumer.Capabilities{MutatesData: true} // TODO https://github.com/open-telemetry/opentelemetry-collector/issues/215 // Move this to the error package that allows for span name and field to be specified. var errMissingRequiredField = errors.New("error creating \"span\" processor: either \"from_attributes\" or \"to_attributes\" must be specified in \"name:\" or \"setStatus\" must be specified") +var errIncorrectStatusCode = errors.New("error creating \"span\" processor: \"setStatus\" must have specified \"code\" as \"Ok\" or \"Err\"") // NewFactory returns a new factory for the Span processor. func NewFactory() component.ProcessorFactory { @@ -67,6 +68,11 @@ func createTracesProcessor( return nil, errMissingRequiredField } + if oCfg.SetStatus != nil && + (oCfg.SetStatus.Code != "Ok" && oCfg.SetStatus.Code != "Err") { + return nil, errIncorrectStatusCode + } + sp, err := newSpanProcessor(*oCfg) if err != nil { return nil, err diff --git a/processor/spanprocessor/span.go b/processor/spanprocessor/span.go index a5c643883209..744ba14ae97a 100644 --- a/processor/spanprocessor/span.go +++ b/processor/spanprocessor/span.go @@ -223,8 +223,15 @@ func (sp *spanProcessor) processToAttributes(span pdata.Span) { } func (sp *spanProcessor) processUpdateStatus(span pdata.Span) { - if sp.config.SetStatus != nil { - span.Status().SetCode(pdata.StatusCode(sp.config.SetStatus.Code)) - span.Status().SetMessage(sp.config.SetStatus.Description) + cfg := sp.config.SetStatus + if cfg != nil { + if cfg.Code == "Ok" { + span.Status().SetCode(pdata.StatusCodeOk) + } else if cfg.Code == "Err" { + span.Status().SetCode(pdata.StatusCodeError) + } + if len(cfg.Description) > 0 { + span.Status().SetMessage(cfg.Description) + } } } diff --git a/processor/spanprocessor/testdata/config.yaml b/processor/spanprocessor/testdata/config.yaml index 9813e7868bf9..5b42abaf0837 100644 --- a/processor/spanprocessor/testdata/config.yaml +++ b/processor/spanprocessor/testdata/config.yaml @@ -84,11 +84,22 @@ processors: rules: - "(?P.*?)$" - # This example changes status of a span to error and sets description - span/setstatus: + # This example changes status of a span to error and sets description. + # Possible values for code are: "Ok" and "Err" with an additional descrption. + span/set_status_err: status: - code: 2 - description: "some error description" + code: "Err" + description: "some additional error description" + + # However you may want to set status conditionally. Example below sets + # status to success only when attribute http.status_code is equal to 400 + span/set_status_ok: + include: + attributes: + - Key: http.status_code + Value: 400 + status: + code: "Ok" exporters: nop: From 1ee3319322f84b3c5a59a74131942f2c7cdfe0db Mon Sep 17 00:00:00 2001 From: Tomasz Rojek Date: Mon, 25 Oct 2021 15:27:50 +0200 Subject: [PATCH 3/9] Add "Unset" value and tests --- processor/spanprocessor/config.go | 2 +- processor/spanprocessor/factory.go | 4 +- processor/spanprocessor/span.go | 2 + processor/spanprocessor/span_test.go | 78 ++++++++++++++++++++++++++-- 4 files changed, 80 insertions(+), 6 deletions(-) diff --git a/processor/spanprocessor/config.go b/processor/spanprocessor/config.go index 883a7f7e039f..b55cfe6d0dbb 100644 --- a/processor/spanprocessor/config.go +++ b/processor/spanprocessor/config.go @@ -85,7 +85,7 @@ type ToAttributes struct { } type Status struct { - // Code is one of two values "Ok" or "Err". Please check: + // Code is one of three values "Ok" or "Err" or "Unset". Please check: // https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/api.md#set-status Code string `mapstructure:"code"` diff --git a/processor/spanprocessor/factory.go b/processor/spanprocessor/factory.go index 10cfec858a96..6d70ca4c7095 100644 --- a/processor/spanprocessor/factory.go +++ b/processor/spanprocessor/factory.go @@ -36,7 +36,7 @@ var processorCapabilities = consumer.Capabilities{MutatesData: true} // TODO https://github.com/open-telemetry/opentelemetry-collector/issues/215 // Move this to the error package that allows for span name and field to be specified. var errMissingRequiredField = errors.New("error creating \"span\" processor: either \"from_attributes\" or \"to_attributes\" must be specified in \"name:\" or \"setStatus\" must be specified") -var errIncorrectStatusCode = errors.New("error creating \"span\" processor: \"setStatus\" must have specified \"code\" as \"Ok\" or \"Err\"") +var errIncorrectStatusCode = errors.New("error creating \"span\" processor: \"setStatus\" must have specified \"code\" as \"Ok\" or \"Err\" or \"Unset\"") // NewFactory returns a new factory for the Span processor. func NewFactory() component.ProcessorFactory { @@ -69,7 +69,7 @@ func createTracesProcessor( } if oCfg.SetStatus != nil && - (oCfg.SetStatus.Code != "Ok" && oCfg.SetStatus.Code != "Err") { + (oCfg.SetStatus.Code != "Ok" && oCfg.SetStatus.Code != "Err" && oCfg.SetStatus.Code != "Unset") { return nil, errIncorrectStatusCode } diff --git a/processor/spanprocessor/span.go b/processor/spanprocessor/span.go index 744ba14ae97a..fd677394154b 100644 --- a/processor/spanprocessor/span.go +++ b/processor/spanprocessor/span.go @@ -229,6 +229,8 @@ func (sp *spanProcessor) processUpdateStatus(span pdata.Span) { span.Status().SetCode(pdata.StatusCodeOk) } else if cfg.Code == "Err" { span.Status().SetCode(pdata.StatusCodeError) + } else if cfg.Code == "Unset" { + span.Status().SetCode(pdata.StatusCodeUnset) } if len(cfg.Description) > 0 { span.Status().SetMessage(cfg.Description) diff --git a/processor/spanprocessor/span_test.go b/processor/spanprocessor/span_test.go index 8227346dfe74..08c8ef012de0 100644 --- a/processor/spanprocessor/span_test.go +++ b/processor/spanprocessor/span_test.go @@ -594,11 +594,83 @@ func TestSpanProcessor_skipSpan(t *testing.T) { } } +func generateTraceDatSetStatus(code pdata.StatusCode, description string, attrs map[string]pdata.AttributeValue) pdata.Traces { + td := pdata.NewTraces() + rs := td.ResourceSpans().AppendEmpty() + span := rs.InstrumentationLibrarySpans().AppendEmpty().Spans().AppendEmpty() + span.Status().SetCode(code) + span.Status().SetMessage(description) + span.Attributes().InitFromMap(attrs).Sort() + return td +} + func TestSpanProcessor_setStatusCode(t *testing.T) { factory := NewFactory() cfg := factory.CreateDefaultConfig() oCfg := cfg.(*Config) - oCfg.SetStatus.Code = 2 - oCfg.SetStatus.Description = "someStatus" - // TODO: Work in progress + oCfg.SetStatus = &Status{ + Code: "Err", + Description: "Set custom error message", + } + tp, err := factory.CreateTracesProcessor(context.Background(), componenttest.NewNopProcessorCreateSettings(), oCfg, consumertest.NewNop()) + require.Nil(t, err) + require.NotNil(t, tp) + + td := generateTraceDatSetStatus(pdata.StatusCodeUnset, "foobar", nil) + td.InternalRep() + + assert.NoError(t, tp.ConsumeTraces(context.Background(), td)) + + assert.EqualValues(t, generateTraceDatSetStatus(pdata.StatusCodeError, "Set custom error message", nil), td) +} + +func TestSpanProcessor_setStatusCodeConditionally(t *testing.T) { + factory := NewFactory() + cfg := factory.CreateDefaultConfig() + oCfg := cfg.(*Config) + oCfg.SetStatus = &Status{ + Code: "Err", + Description: "custom error message", + } + // This test numer two include rule for applying rule only for status code 400 + oCfg.Include = &filterconfig.MatchProperties{ + Attributes: []filterconfig.Attribute{ + {Key: "http.status_code", Value: 400}, + }, + } + tp, err := factory.CreateTracesProcessor(context.Background(), componenttest.NewNopProcessorCreateSettings(), oCfg, consumertest.NewNop()) + require.Nil(t, err) + require.NotNil(t, tp) + + testCases := []struct { + inputAttributes map[string]pdata.AttributeValue + inputStatusCode pdata.StatusCode + outputStatusCode pdata.StatusCode + outputStatusDescription string + }{ + { + // without attribiutes - should not apply rule and leave status code as it is + inputStatusCode: pdata.StatusCodeOk, + outputStatusCode: pdata.StatusCodeOk, + }, + { + inputAttributes: map[string]pdata.AttributeValue{ + "http.status_code": pdata.NewAttributeValueInt(400), + }, + inputStatusCode: pdata.StatusCodeOk, + outputStatusCode: pdata.StatusCodeError, + outputStatusDescription: "custom error message", + }, + } + + for _, tc := range testCases { + t.Run("set-status-test", func(t *testing.T) { + td := generateTraceDatSetStatus(tc.inputStatusCode, "", tc.inputAttributes) + td.InternalRep() + + assert.NoError(t, tp.ConsumeTraces(context.Background(), td)) + + assert.EqualValues(t, generateTraceDatSetStatus(tc.outputStatusCode, tc.outputStatusDescription, tc.inputAttributes), td) + }) + } } From 7163857455d0fe17b2029e330fe78dff4c1376a6 Mon Sep 17 00:00:00 2001 From: Tomasz Rojek Date: Mon, 25 Oct 2021 15:55:00 +0200 Subject: [PATCH 4/9] Update README for Unset value --- processor/spanprocessor/README.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/processor/spanprocessor/README.md b/processor/spanprocessor/README.md index 3f0fe5e987c8..95e9cee75486 100644 --- a/processor/spanprocessor/README.md +++ b/processor/spanprocessor/README.md @@ -103,8 +103,9 @@ Example: ```yaml # Set status allows to set specific status for a given span. Possible values are -# Ok and Error as per +# Ok, Error and Unset as per # https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/api.md#set-status +# additionaly description allows to set human-readable message span/set_status: status: code: Error From e2f5d7b890605044f872893859a9e842f45b0d6b Mon Sep 17 00:00:00 2001 From: Tomasz Rojek Date: Mon, 25 Oct 2021 22:09:20 +0200 Subject: [PATCH 5/9] Apply suggestions from code review Co-authored-by: Pablo Baeyens --- processor/spanprocessor/README.md | 2 +- processor/spanprocessor/config.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/processor/spanprocessor/README.md b/processor/spanprocessor/README.md index 95e9cee75486..59877d05b9e2 100644 --- a/processor/spanprocessor/README.md +++ b/processor/spanprocessor/README.md @@ -105,7 +105,7 @@ Example: # Set status allows to set specific status for a given span. Possible values are # Ok, Error and Unset as per # https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/api.md#set-status -# additionaly description allows to set human-readable message +# The description field allows to set a human-readable message for errors. span/set_status: status: code: Error diff --git a/processor/spanprocessor/config.go b/processor/spanprocessor/config.go index b55cfe6d0dbb..80768047891e 100644 --- a/processor/spanprocessor/config.go +++ b/processor/spanprocessor/config.go @@ -89,7 +89,7 @@ type Status struct { // https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/api.md#set-status Code string `mapstructure:"code"` - // Optional description + // Description is an optional field documenting Error statuses. Description string `mapstructure:"description"` } From 8480c55602083680c765d3eaf93060eda153f0ca Mon Sep 17 00:00:00 2001 From: Tomasz Rojek Date: Mon, 25 Oct 2021 23:51:21 +0200 Subject: [PATCH 6/9] From review: Move statuses names to constants and fix "Err" => "Error" --- processor/spanprocessor/README.md | 7 +++++++ processor/spanprocessor/config_test.go | 2 +- processor/spanprocessor/factory.go | 20 ++++++++++++++++---- processor/spanprocessor/span.go | 6 +++--- processor/spanprocessor/span_test.go | 14 +++++++------- processor/spanprocessor/testdata/config.yaml | 2 +- 6 files changed, 35 insertions(+), 16 deletions(-) diff --git a/processor/spanprocessor/README.md b/processor/spanprocessor/README.md index 59877d05b9e2..de2aa8986242 100644 --- a/processor/spanprocessor/README.md +++ b/processor/spanprocessor/README.md @@ -99,6 +99,13 @@ span/to_attributes: ### Set status for span +The following setting is required: + +- `code`: Represents span status. One of the following values "Unset", "Error", "Ok". + +The following setting is allowed only for code "Error": +- `description` + Example: ```yaml diff --git a/processor/spanprocessor/config_test.go b/processor/spanprocessor/config_test.go index 57ecfdf8ddcc..8bfb387e5881 100644 --- a/processor/spanprocessor/config_test.go +++ b/processor/spanprocessor/config_test.go @@ -93,7 +93,7 @@ func TestLoadConfig(t *testing.T) { assert.Equal(t, p4, &Config{ ProcessorSettings: config.NewProcessorSettings(config.NewComponentIDWithName("span", "set_status_err")), SetStatus: &Status{ - Code: "Err", + Code: "Error", Description: "some additional error description", }, }) diff --git a/processor/spanprocessor/factory.go b/processor/spanprocessor/factory.go index 6d70ca4c7095..397881e2a185 100644 --- a/processor/spanprocessor/factory.go +++ b/processor/spanprocessor/factory.go @@ -29,6 +29,13 @@ const ( typeStr = "span" ) +const ( + // status represents span status + statusCodeUnset = "Unset" + statusCodeError = "Error" + statusCodeOk = "Ok" +) + var processorCapabilities = consumer.Capabilities{MutatesData: true} // errMissingRequiredField is returned when a required field in the config @@ -36,7 +43,8 @@ var processorCapabilities = consumer.Capabilities{MutatesData: true} // TODO https://github.com/open-telemetry/opentelemetry-collector/issues/215 // Move this to the error package that allows for span name and field to be specified. var errMissingRequiredField = errors.New("error creating \"span\" processor: either \"from_attributes\" or \"to_attributes\" must be specified in \"name:\" or \"setStatus\" must be specified") -var errIncorrectStatusCode = errors.New("error creating \"span\" processor: \"setStatus\" must have specified \"code\" as \"Ok\" or \"Err\" or \"Unset\"") +var errIncorrectStatusCode = errors.New("error creating \"span\" processor: \"status\" must have specified \"code\" as \"Ok\" or \"Error\" or \"Unset\"") +var errIncorrectStatusDescription = errors.New("error creating \"span\" processor: \"description\" can be specified only for \"code\" \"Error\"") // NewFactory returns a new factory for the Span processor. func NewFactory() component.ProcessorFactory { @@ -68,9 +76,13 @@ func createTracesProcessor( return nil, errMissingRequiredField } - if oCfg.SetStatus != nil && - (oCfg.SetStatus.Code != "Ok" && oCfg.SetStatus.Code != "Err" && oCfg.SetStatus.Code != "Unset") { - return nil, errIncorrectStatusCode + if oCfg.SetStatus != nil { + if oCfg.SetStatus.Code != statusCodeUnset && oCfg.SetStatus.Code != statusCodeError && oCfg.SetStatus.Code != statusCodeOk { + return nil, errIncorrectStatusCode + } + if len(oCfg.SetStatus.Description) > 0 && oCfg.SetStatus.Code != statusCodeError { + return nil, errIncorrectStatusDescription + } } sp, err := newSpanProcessor(*oCfg) diff --git a/processor/spanprocessor/span.go b/processor/spanprocessor/span.go index fd677394154b..6a5a9eff8a19 100644 --- a/processor/spanprocessor/span.go +++ b/processor/spanprocessor/span.go @@ -225,11 +225,11 @@ func (sp *spanProcessor) processToAttributes(span pdata.Span) { func (sp *spanProcessor) processUpdateStatus(span pdata.Span) { cfg := sp.config.SetStatus if cfg != nil { - if cfg.Code == "Ok" { + if cfg.Code == statusCodeOk { span.Status().SetCode(pdata.StatusCodeOk) - } else if cfg.Code == "Err" { + } else if cfg.Code == statusCodeError { span.Status().SetCode(pdata.StatusCodeError) - } else if cfg.Code == "Unset" { + } else if cfg.Code == statusCodeUnset { span.Status().SetCode(pdata.StatusCodeUnset) } if len(cfg.Description) > 0 { diff --git a/processor/spanprocessor/span_test.go b/processor/spanprocessor/span_test.go index 08c8ef012de0..72f4f1cda2b5 100644 --- a/processor/spanprocessor/span_test.go +++ b/processor/spanprocessor/span_test.go @@ -594,7 +594,7 @@ func TestSpanProcessor_skipSpan(t *testing.T) { } } -func generateTraceDatSetStatus(code pdata.StatusCode, description string, attrs map[string]pdata.AttributeValue) pdata.Traces { +func generateTraceDataSetStatus(code pdata.StatusCode, description string, attrs map[string]pdata.AttributeValue) pdata.Traces { td := pdata.NewTraces() rs := td.ResourceSpans().AppendEmpty() span := rs.InstrumentationLibrarySpans().AppendEmpty().Spans().AppendEmpty() @@ -609,19 +609,19 @@ func TestSpanProcessor_setStatusCode(t *testing.T) { cfg := factory.CreateDefaultConfig() oCfg := cfg.(*Config) oCfg.SetStatus = &Status{ - Code: "Err", + Code: "Error", Description: "Set custom error message", } tp, err := factory.CreateTracesProcessor(context.Background(), componenttest.NewNopProcessorCreateSettings(), oCfg, consumertest.NewNop()) require.Nil(t, err) require.NotNil(t, tp) - td := generateTraceDatSetStatus(pdata.StatusCodeUnset, "foobar", nil) + td := generateTraceDataSetStatus(pdata.StatusCodeUnset, "foobar", nil) td.InternalRep() assert.NoError(t, tp.ConsumeTraces(context.Background(), td)) - assert.EqualValues(t, generateTraceDatSetStatus(pdata.StatusCodeError, "Set custom error message", nil), td) + assert.EqualValues(t, generateTraceDataSetStatus(pdata.StatusCodeError, "Set custom error message", nil), td) } func TestSpanProcessor_setStatusCodeConditionally(t *testing.T) { @@ -629,7 +629,7 @@ func TestSpanProcessor_setStatusCodeConditionally(t *testing.T) { cfg := factory.CreateDefaultConfig() oCfg := cfg.(*Config) oCfg.SetStatus = &Status{ - Code: "Err", + Code: "Error", Description: "custom error message", } // This test numer two include rule for applying rule only for status code 400 @@ -665,12 +665,12 @@ func TestSpanProcessor_setStatusCodeConditionally(t *testing.T) { for _, tc := range testCases { t.Run("set-status-test", func(t *testing.T) { - td := generateTraceDatSetStatus(tc.inputStatusCode, "", tc.inputAttributes) + td := generateTraceDataSetStatus(tc.inputStatusCode, "", tc.inputAttributes) td.InternalRep() assert.NoError(t, tp.ConsumeTraces(context.Background(), td)) - assert.EqualValues(t, generateTraceDatSetStatus(tc.outputStatusCode, tc.outputStatusDescription, tc.inputAttributes), td) + assert.EqualValues(t, generateTraceDataSetStatus(tc.outputStatusCode, tc.outputStatusDescription, tc.inputAttributes), td) }) } } diff --git a/processor/spanprocessor/testdata/config.yaml b/processor/spanprocessor/testdata/config.yaml index 5b42abaf0837..52a8067a6135 100644 --- a/processor/spanprocessor/testdata/config.yaml +++ b/processor/spanprocessor/testdata/config.yaml @@ -88,7 +88,7 @@ processors: # Possible values for code are: "Ok" and "Err" with an additional descrption. span/set_status_err: status: - code: "Err" + code: "Error" description: "some additional error description" # However you may want to set status conditionally. Example below sets From 8fbf6eb1ae227f0f0c402b0aa3c61271248bab92 Mon Sep 17 00:00:00 2001 From: Tomasz Rojek Date: Tue, 26 Oct 2021 16:52:53 +0200 Subject: [PATCH 7/9] Code review: fixing style --- processor/spanprocessor/factory.go | 8 +++++--- processor/spanprocessor/testdata/config.yaml | 2 +- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/processor/spanprocessor/factory.go b/processor/spanprocessor/factory.go index 397881e2a185..7c616da02850 100644 --- a/processor/spanprocessor/factory.go +++ b/processor/spanprocessor/factory.go @@ -42,9 +42,11 @@ var processorCapabilities = consumer.Capabilities{MutatesData: true} // is not specified. // TODO https://github.com/open-telemetry/opentelemetry-collector/issues/215 // Move this to the error package that allows for span name and field to be specified. -var errMissingRequiredField = errors.New("error creating \"span\" processor: either \"from_attributes\" or \"to_attributes\" must be specified in \"name:\" or \"setStatus\" must be specified") -var errIncorrectStatusCode = errors.New("error creating \"span\" processor: \"status\" must have specified \"code\" as \"Ok\" or \"Error\" or \"Unset\"") -var errIncorrectStatusDescription = errors.New("error creating \"span\" processor: \"description\" can be specified only for \"code\" \"Error\"") +var ( + errMissingRequiredField = errors.New("error creating \"span\" processor: either \"from_attributes\" or \"to_attributes\" must be specified in \"name:\" or \"setStatus\" must be specified") + errIncorrectStatusCode = errors.New("error creating \"span\" processor: \"status\" must have specified \"code\" as \"Ok\" or \"Error\" or \"Unset\"") + errIncorrectStatusDescription = errors.New("error creating \"span\" processor: \"description\" can be specified only for \"code\" \"Error\"") +) // NewFactory returns a new factory for the Span processor. func NewFactory() component.ProcessorFactory { diff --git a/processor/spanprocessor/testdata/config.yaml b/processor/spanprocessor/testdata/config.yaml index 52a8067a6135..ebd69d1cdfc1 100644 --- a/processor/spanprocessor/testdata/config.yaml +++ b/processor/spanprocessor/testdata/config.yaml @@ -85,7 +85,7 @@ processors: - "(?P.*?)$" # This example changes status of a span to error and sets description. - # Possible values for code are: "Ok" and "Err" with an additional descrption. + # Possible values for code are: "Ok", "Error" or "Unset" with an additional descrption. span/set_status_err: status: code: "Error" From 19a74bf0066eeec5a6e41745d706481b6bbb66f3 Mon Sep 17 00:00:00 2001 From: Tomasz Rojek Date: Thu, 28 Oct 2021 00:57:03 +0200 Subject: [PATCH 8/9] One more "Err" and fixing comment(s) as suggested from review --- processor/spanprocessor/config.go | 5 ++--- processor/spanprocessor/testdata/config.yaml | 3 ++- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/processor/spanprocessor/config.go b/processor/spanprocessor/config.go index 80768047891e..ce47c51dc83a 100644 --- a/processor/spanprocessor/config.go +++ b/processor/spanprocessor/config.go @@ -36,8 +36,7 @@ type Config struct { // from config.NamedEntity Rename Name `mapstructure:"name"` - // SetStatus specifies status which should be set for this span. Please check: - // https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/api.md#set-status + // SetStatus specifies status which should be set for this span. SetStatus *Status `mapstructure:"status"` } @@ -85,7 +84,7 @@ type ToAttributes struct { } type Status struct { - // Code is one of three values "Ok" or "Err" or "Unset". Please check: + // Code is one of three values "Ok" or "Error" or "Unset". Please check: // https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/api.md#set-status Code string `mapstructure:"code"` diff --git a/processor/spanprocessor/testdata/config.yaml b/processor/spanprocessor/testdata/config.yaml index ebd69d1cdfc1..1c900520d32e 100644 --- a/processor/spanprocessor/testdata/config.yaml +++ b/processor/spanprocessor/testdata/config.yaml @@ -85,7 +85,8 @@ processors: - "(?P.*?)$" # This example changes status of a span to error and sets description. - # Possible values for code are: "Ok", "Error" or "Unset" with an additional descrption. + # Possible values for code are: "Ok", "Error" or "Unset". + # Description is an optional field used for documenting Error statuses. span/set_status_err: status: code: "Error" From 7dc824b83c061039ca65d83251c12dc31f710865 Mon Sep 17 00:00:00 2001 From: Tomasz Rojek Date: Thu, 28 Oct 2021 14:26:56 +0200 Subject: [PATCH 9/9] Clear status description for cases different than error --- processor/spanprocessor/span.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/processor/spanprocessor/span.go b/processor/spanprocessor/span.go index 6a5a9eff8a19..950d8147366c 100644 --- a/processor/spanprocessor/span.go +++ b/processor/spanprocessor/span.go @@ -227,13 +227,13 @@ func (sp *spanProcessor) processUpdateStatus(span pdata.Span) { if cfg != nil { if cfg.Code == statusCodeOk { span.Status().SetCode(pdata.StatusCodeOk) + span.Status().SetMessage("") } else if cfg.Code == statusCodeError { span.Status().SetCode(pdata.StatusCodeError) + span.Status().SetMessage(cfg.Description) } else if cfg.Code == statusCodeUnset { span.Status().SetCode(pdata.StatusCodeUnset) - } - if len(cfg.Description) > 0 { - span.Status().SetMessage(cfg.Description) + span.Status().SetMessage("") } } }