From 0d9b1b0815cbcbed3d7522cbcecc3e7d08776d6b Mon Sep 17 00:00:00 2001 From: Brandon Johnson Date: Thu, 14 Mar 2024 21:16:10 -0400 Subject: [PATCH 1/3] [pkg/ottl]: Add ParseXML converter (#31487) **Description:** * Adds a ParseXML converter function that can be used to parse an XML document to a pcommon.Map value **Link to tracking Issue:** Closes #31133 **Testing:** Unit tests Manually tested parsing XML logs **Documentation:** Added documentation for the ParseXML function to the ottl_funcs README. --------- Co-authored-by: Evan Bradley <11745660+evan-bradley@users.noreply.github.com> --- .chloggen/feat_ottl_xml-parse-function.yaml | 13 + pkg/ottl/e2e/e2e_test.go | 16 + pkg/ottl/ottlfuncs/README.md | 73 +++++ pkg/ottl/ottlfuncs/func_parse_xml.go | 134 +++++++++ pkg/ottl/ottlfuncs/func_parse_xml_test.go | 309 ++++++++++++++++++++ pkg/ottl/ottlfuncs/functions.go | 1 + 6 files changed, 546 insertions(+) create mode 100755 .chloggen/feat_ottl_xml-parse-function.yaml create mode 100644 pkg/ottl/ottlfuncs/func_parse_xml.go create mode 100644 pkg/ottl/ottlfuncs/func_parse_xml_test.go diff --git a/.chloggen/feat_ottl_xml-parse-function.yaml b/.chloggen/feat_ottl_xml-parse-function.yaml new file mode 100755 index 0000000000000..710eedae3f487 --- /dev/null +++ b/.chloggen/feat_ottl_xml-parse-function.yaml @@ -0,0 +1,13 @@ +# Use this changelog template to create an entry for release notes. + +# 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: pkg/ottl + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: Add `ParseXML` function for parsing XML from a target string. + +# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists. +issues: [31133] diff --git a/pkg/ottl/e2e/e2e_test.go b/pkg/ottl/e2e/e2e_test.go index f850af9b0aa36..315a00e8c28b8 100644 --- a/pkg/ottl/e2e/e2e_test.go +++ b/pkg/ottl/e2e/e2e_test.go @@ -485,6 +485,22 @@ func Test_e2e_converters(t *testing.T) { m.PutStr("k2", "v2__!__v2") }, }, + { + statement: `set(attributes["test"], ParseXML("This is a log message!"))`, + want: func(tCtx ottllog.TransformContext) { + log := tCtx.GetLogRecord().Attributes().PutEmptyMap("test") + log.PutStr("tag", "Log") + + attrs := log.PutEmptyMap("attributes") + attrs.PutStr("id", "1") + + logChildren := log.PutEmptySlice("children") + + message := logChildren.AppendEmpty().SetEmptyMap() + message.PutStr("tag", "Message") + message.PutStr("content", "This is a log message!") + }, + }, { statement: `set(attributes["test"], Seconds(Duration("1m")))`, want: func(tCtx ottllog.TransformContext) { diff --git a/pkg/ottl/ottlfuncs/README.md b/pkg/ottl/ottlfuncs/README.md index a87b6562f57bf..94712ca3074db 100644 --- a/pkg/ottl/ottlfuncs/README.md +++ b/pkg/ottl/ottlfuncs/README.md @@ -403,6 +403,7 @@ Available Converters: - [ParseCSV](#parsecsv) - [ParseJSON](#parsejson) - [ParseKeyValue](#parsekeyvalue) +- [ParseXML](#parsexml) - [Seconds](#seconds) - [SHA1](#sha1) - [SHA256](#sha256) @@ -913,6 +914,78 @@ Examples: - `ParseKeyValue(attributes["pairs"])` +### ParseXML + +`ParseXML(target)` + +The `ParseXML` Converter returns a `pcommon.Map` struct that is the result of parsing the target string as an XML document. + +`target` is a Getter that returns a string. This string should be in XML format. +If `target` is not a string, nil, or cannot be parsed as XML, `ParseXML` will return an error. + +Unmarshalling XML is done using the following rules: +1. All character data for an XML element is trimmed, joined, and placed into the `content` field. +2. The tag for an XML element is trimmed, and placed into the `tag` field. +3. The attributes for an XML element is placed as a `pcommon.Map` into the `attribute` field. +4. Processing instructions, directives, and comments are ignored and not represented in the resultant map. +5. All child elements are parsed as above, and placed in a `pcommon.Slice`, which is then placed into the `children` field. + +For example, the following XML document: +```xml + + + + 00001 + Joe + joe.smith@example.com + + User fired alert A + +``` + +will be parsed as: +```json +{ + "tag": "Log", + "children": [ + { + "tag": "User", + "children": [ + { + "tag": "ID", + "content": "00001" + }, + { + "tag": "Name", + "content": "Joe", + "attributes": { + "type": "first" + } + }, + { + "tag": "Email", + "content": "joe.smith@example.com" + } + ] + }, + { + "tag": "Text", + "content": "User fired alert A" + } + ] +} +``` + +Examples: + +- `ParseXML(body)` + +- `ParseXML(attributes["xml"])` + +- `ParseXML("")` + + + ### Seconds `Seconds(value)` diff --git a/pkg/ottl/ottlfuncs/func_parse_xml.go b/pkg/ottl/ottlfuncs/func_parse_xml.go new file mode 100644 index 0000000000000..42dac93307dfb --- /dev/null +++ b/pkg/ottl/ottlfuncs/func_parse_xml.go @@ -0,0 +1,134 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +package ottlfuncs // import "github.com/open-telemetry/opentelemetry-collector-contrib/pkg/ottl/ottlfuncs" + +import ( + "bytes" + "context" + "encoding/xml" + "errors" + "fmt" + "strings" + + "go.opentelemetry.io/collector/pdata/pcommon" + + "github.com/open-telemetry/opentelemetry-collector-contrib/pkg/ottl" +) + +type ParseXMLArguments[K any] struct { + Target ottl.StringGetter[K] +} + +func NewParseXMLFactory[K any]() ottl.Factory[K] { + return ottl.NewFactory("ParseXML", &ParseXMLArguments[K]{}, createParseXMLFunction[K]) +} + +func createParseXMLFunction[K any](_ ottl.FunctionContext, oArgs ottl.Arguments) (ottl.ExprFunc[K], error) { + args, ok := oArgs.(*ParseXMLArguments[K]) + + if !ok { + return nil, fmt.Errorf("ParseXMLFactory args must be of type *ParseXMLArguments[K]") + } + + return parseXML(args.Target), nil +} + +// parseXML returns a `pcommon.Map` struct that is a result of parsing the target string as XML +func parseXML[K any](target ottl.StringGetter[K]) ottl.ExprFunc[K] { + return func(ctx context.Context, tCtx K) (any, error) { + targetVal, err := target.Get(ctx, tCtx) + if err != nil { + return nil, err + } + + parsedXML := xmlElement{} + + decoder := xml.NewDecoder(strings.NewReader(targetVal)) + err = decoder.Decode(&parsedXML) + if err != nil { + return nil, fmt.Errorf("unmarshal xml: %w", err) + } + + if decoder.InputOffset() != int64(len(targetVal)) { + return nil, errors.New("trailing bytes after parsing xml") + } + + parsedMap := pcommon.NewMap() + parsedXML.intoMap(parsedMap) + + return parsedMap, nil + } +} + +type xmlElement struct { + tag string + attributes []xml.Attr + text string + children []xmlElement +} + +// UnmarshalXML implements xml.Unmarshaler for xmlElement +func (a *xmlElement) UnmarshalXML(d *xml.Decoder, start xml.StartElement) error { + a.tag = start.Name.Local + a.attributes = start.Attr + + for { + tok, err := d.Token() + if err != nil { + return fmt.Errorf("decode next token: %w", err) + } + + switch t := tok.(type) { + case xml.StartElement: + child := xmlElement{} + err := d.DecodeElement(&child, &t) + if err != nil { + return err + } + + a.children = append(a.children, child) + case xml.EndElement: + // End element means we've reached the end of parsing + return nil + case xml.CharData: + // Strip leading/trailing spaces to ignore newlines and + // indentation in formatted XML + a.text += string(bytes.TrimSpace([]byte(t))) + case xml.Comment: // ignore comments + case xml.ProcInst: // ignore processing instructions + case xml.Directive: // ignore directives + default: + return fmt.Errorf("unexpected token type %T", t) + } + } +} + +// intoMap converts and adds the xmlElement into the provided pcommon.Map. +func (a xmlElement) intoMap(m pcommon.Map) { + m.EnsureCapacity(4) + + m.PutStr("tag", a.tag) + + if a.text != "" { + m.PutStr("content", a.text) + } + + if len(a.attributes) > 0 { + attrs := m.PutEmptyMap("attributes") + attrs.EnsureCapacity(len(a.attributes)) + + for _, attr := range a.attributes { + attrs.PutStr(attr.Name.Local, attr.Value) + } + } + + if len(a.children) > 0 { + children := m.PutEmptySlice("children") + children.EnsureCapacity(len(a.children)) + + for _, child := range a.children { + child.intoMap(children.AppendEmpty().SetEmptyMap()) + } + } +} diff --git a/pkg/ottl/ottlfuncs/func_parse_xml_test.go b/pkg/ottl/ottlfuncs/func_parse_xml_test.go new file mode 100644 index 0000000000000..8c348d3a6e762 --- /dev/null +++ b/pkg/ottl/ottlfuncs/func_parse_xml_test.go @@ -0,0 +1,309 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +package ottlfuncs + +import ( + "context" + "fmt" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.opentelemetry.io/collector/pdata/pcommon" + + "github.com/open-telemetry/opentelemetry-collector-contrib/pkg/ottl" +) + +func Test_ParseXML(t *testing.T) { + tests := []struct { + name string + oArgs ottl.Arguments + want map[string]any + createError string + parseError string + }{ + { + name: "Text values in nested elements", + oArgs: &ParseXMLArguments[any]{ + Target: ottl.StandardStringGetter[any]{ + Getter: func(_ context.Context, _ any) (any, error) { + return "00001Joejoe.smith@example.comUser did a thing", nil + }, + }, + }, + want: map[string]any{ + "tag": "Log", + "children": []any{ + map[string]any{ + "tag": "User", + "children": []any{ + map[string]any{ + "tag": "ID", + "content": "00001", + }, + map[string]any{ + "tag": "Name", + "content": "Joe", + }, + map[string]any{ + "tag": "Email", + "content": "joe.smith@example.com", + }, + }, + }, + map[string]any{ + "tag": "Text", + "content": "User did a thing", + }, + }, + }, + }, + { + name: "Formatted example", + oArgs: &ParseXMLArguments[any]{ + Target: ottl.StandardStringGetter[any]{ + Getter: func(_ context.Context, _ any) (any, error) { + return ` + + + 00001 + Joe + joe.smith@example.com + + User did a thing + `, nil + }, + }, + }, + want: map[string]any{ + "tag": "Log", + "children": []any{ + map[string]any{ + "tag": "User", + "children": []any{ + map[string]any{ + "tag": "ID", + "content": "00001", + }, + map[string]any{ + "tag": "Name", + "content": "Joe", + }, + map[string]any{ + "tag": "Email", + "content": "joe.smith@example.com", + }, + }, + }, + map[string]any{ + "tag": "Text", + "content": "User did a thing", + }, + }, + }, + }, + { + name: "Multiple tags with the same name", + oArgs: &ParseXMLArguments[any]{ + Target: ottl.StandardStringGetter[any]{ + Getter: func(_ context.Context, _ any) (any, error) { + return `This record has a collision`, nil + }, + }, + }, + want: map[string]any{ + "tag": "Log", + "content": "This record has a collision", + "children": []any{ + map[string]any{ + "tag": "User", + "attributes": map[string]any{ + "id": "0001", + }, + }, + map[string]any{ + "tag": "User", + "attributes": map[string]any{ + "id": "0002", + }, + }, + }, + }, + }, + { + name: "Multiple lines of content", + oArgs: &ParseXMLArguments[any]{ + Target: ottl.StandardStringGetter[any]{ + Getter: func(_ context.Context, _ any) (any, error) { + return ` + This record has multiple lines of + + text content + `, nil + }, + }, + }, + want: map[string]any{ + "tag": "Log", + "content": "This record has multiple lines oftext content", + "children": []any{ + map[string]any{ + "tag": "User", + "attributes": map[string]any{ + "id": "0001", + }, + }, + }, + }, + }, + { + name: "Attribute only element", + oArgs: &ParseXMLArguments[any]{ + Target: ottl.StandardStringGetter[any]{ + Getter: func(_ context.Context, _ any) (any, error) { + return ``, nil + }, + }, + }, + want: map[string]any{ + "tag": "HostInfo", + "attributes": map[string]any{ + "hostname": "example.com", + "zone": "east-1", + "cloudprovider": "aws", + }, + }, + }, + { + name: "Ignores XML declaration", + oArgs: &ParseXMLArguments[any]{ + Target: ottl.StandardStringGetter[any]{ + Getter: func(_ context.Context, _ any) (any, error) { + return `Log content`, nil + }, + }, + }, + want: map[string]any{ + "tag": "Log", + "content": "Log content", + }, + }, + { + name: "Ignores comments", + oArgs: &ParseXMLArguments[any]{ + Target: ottl.StandardStringGetter[any]{ + Getter: func(_ context.Context, _ any) (any, error) { + return `This has a comment `, nil + }, + }, + }, + want: map[string]any{ + "tag": "Log", + "content": "This has a comment", + }, + }, + { + name: "Ignores processing instructions", + oArgs: &ParseXMLArguments[any]{ + Target: ottl.StandardStringGetter[any]{ + Getter: func(_ context.Context, _ any) (any, error) { + return `Log content`, nil + }, + }, + }, + want: map[string]any{ + "tag": "Log", + "content": "Log content", + }, + }, + { + name: "Ignores directives", + oArgs: &ParseXMLArguments[any]{ + Target: ottl.StandardStringGetter[any]{ + Getter: func(_ context.Context, _ any) (any, error) { + return `Log content`, nil + }, + }, + }, + want: map[string]any{ + "tag": "Log", + "content": "Log content", + }, + }, + { + name: "Missing closing element", + oArgs: &ParseXMLArguments[any]{ + Target: ottl.StandardStringGetter[any]{ + Getter: func(_ context.Context, _ any) (any, error) { + return ``, nil + }, + }, + }, + parseError: "unmarshal xml: decode next token: XML syntax error on line 1: unexpected EOF", + }, + { + name: "Missing nested closing element", + oArgs: &ParseXMLArguments[any]{ + Target: ottl.StandardStringGetter[any]{ + Getter: func(_ context.Context, _ any) (any, error) { + return ``, nil + }, + }, + }, + parseError: "unmarshal xml: decode next token: XML syntax error on line 1: element closed by ", + }, + { + name: "Multiple XML elements in payload (trailing bytes)", + oArgs: &ParseXMLArguments[any]{ + Target: ottl.StandardStringGetter[any]{ + Getter: func(_ context.Context, _ any) (any, error) { + return ``, nil + }, + }, + }, + parseError: "trailing bytes after parsing xml", + }, + { + name: "Error getting target", + oArgs: &ParseXMLArguments[any]{ + Target: ottl.StandardStringGetter[any]{ + Getter: func(_ context.Context, _ any) (any, error) { + return "", fmt.Errorf("failed to get string") + }, + }, + }, + parseError: "error getting value in ottl.StandardStringGetter[interface {}]: failed to get string", + }, + { + name: "Invalid arguments", + oArgs: nil, + createError: "ParseXMLFactory args must be of type *ParseXMLArguments[K]", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + exprFunc, err := createParseXMLFunction[any](ottl.FunctionContext{}, tt.oArgs) + if tt.createError != "" { + require.ErrorContains(t, err, tt.createError) + return + } + + require.NoError(t, err) + + result, err := exprFunc(context.Background(), nil) + if tt.parseError != "" { + require.ErrorContains(t, err, tt.parseError) + return + } + + assert.NoError(t, err) + + resultMap, ok := result.(pcommon.Map) + require.True(t, ok) + + require.Equal(t, tt.want, resultMap.AsRaw()) + }) + } +} diff --git a/pkg/ottl/ottlfuncs/functions.go b/pkg/ottl/ottlfuncs/functions.go index 1f419a746e42a..9bb33ff3230f0 100644 --- a/pkg/ottl/ottlfuncs/functions.go +++ b/pkg/ottl/ottlfuncs/functions.go @@ -61,6 +61,7 @@ func converters[K any]() []ottl.Factory[K] { NewParseCSVFactory[K](), NewParseJSONFactory[K](), NewParseKeyValueFactory[K](), + NewParseXMLFactory[K](), NewSecondsFactory[K](), NewSHA1Factory[K](), NewSHA256Factory[K](), From 8a29fa4548279cb414c594b9a95615134830979c Mon Sep 17 00:00:00 2001 From: Yang Song Date: Fri, 15 Mar 2024 07:37:07 -0400 Subject: [PATCH 2/3] [connector/datadog] Remove feature gate connector.datadogconnector.performance (#31770) This restores https://github.com/open-telemetry/opentelemetry-collector-contrib/pull/31638. https://github.com/open-telemetry/opentelemetry-collector-contrib/pull/31638 was reverted initially because we thought it led to https://github.com/open-telemetry/opentelemetry-collector-contrib/issues/31663, but it turns out https://github.com/open-telemetry/opentelemetry-collector-contrib/issues/31663 is still happening without https://github.com/open-telemetry/opentelemetry-collector-contrib/pull/31638 so it is probably a different issue. It should be safe to roll forward https://github.com/open-telemetry/opentelemetry-collector-contrib/pull/31638. --------- Co-authored-by: Pablo Baeyens --- .chloggen/dd-feature-gate-removal.yaml | 27 ++++++++++++++++++++++++++ internal/datadog/agent.go | 8 -------- internal/datadog/go.mod | 2 -- internal/datadog/go.sum | 4 ---- 4 files changed, 27 insertions(+), 14 deletions(-) create mode 100644 .chloggen/dd-feature-gate-removal.yaml diff --git a/.chloggen/dd-feature-gate-removal.yaml b/.chloggen/dd-feature-gate-removal.yaml new file mode 100644 index 0000000000000..76f4ee7cb8cf2 --- /dev/null +++ b/.chloggen/dd-feature-gate-removal.yaml @@ -0,0 +1,27 @@ +# Use this changelog template to create an entry for release notes. + +# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' +change_type: breaking + +# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver) +component: datadogconnector + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: Remove feature gate `connector.datadogconnector.performance` + +# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists. +issues: [31638] + +# (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: + +# If your change doesn't affect end users or the exported elements of any package, +# you should instead start your pull request title with [chore] or use the "Skip Changelog" label. +# Optional: The change log or logs in which this entry should be included. +# e.g. '[user]' or '[user, api]' +# Include 'user' if the change is relevant to end users. +# Include 'api' if there is a change to a library API. +# Default: '[user]' +change_logs: [] diff --git a/internal/datadog/agent.go b/internal/datadog/agent.go index fef56516823da..7bba14cad2cfd 100644 --- a/internal/datadog/agent.go +++ b/internal/datadog/agent.go @@ -19,7 +19,6 @@ import ( "github.com/DataDog/datadog-agent/pkg/trace/timing" "github.com/DataDog/datadog-go/v5/statsd" "github.com/DataDog/opentelemetry-mapping-go/pkg/otlp/metrics" - "go.opentelemetry.io/collector/featuregate" "go.opentelemetry.io/collector/pdata/ptrace" ) @@ -38,13 +37,6 @@ type TraceAgent struct { exit chan struct{} } -var _ = featuregate.GlobalRegistry().MustRegister( - "connector.datadogconnector.performance", - featuregate.StageStable, - featuregate.WithRegisterDescription("Datadog Connector will use optimized code"), - featuregate.WithRegisterToVersion("0.97.0"), -) - // newAgent creates a new unstarted traceagent using the given context. Call Start to start the traceagent. // The out channel will receive outoing stats payloads resulting from spans ingested using the Ingest method. func NewAgent(ctx context.Context, out chan *pb.StatsPayload, metricsClient statsd.ClientInterface, timingReporter timing.Reporter) *TraceAgent { diff --git a/internal/datadog/go.mod b/internal/datadog/go.mod index e0e11586dc964..25e2bbba8c070 100644 --- a/internal/datadog/go.mod +++ b/internal/datadog/go.mod @@ -10,7 +10,6 @@ require ( github.com/DataDog/opentelemetry-mapping-go/pkg/otlp/metrics v0.13.4 github.com/stretchr/testify v1.9.0 go.opentelemetry.io/collector/component v0.96.1-0.20240306115632-b2693620eff6 - go.opentelemetry.io/collector/featuregate v1.3.1-0.20240306115632-b2693620eff6 go.opentelemetry.io/collector/pdata v1.3.1-0.20240306115632-b2693620eff6 go.opentelemetry.io/otel v1.24.0 go.opentelemetry.io/otel/metric v1.24.0 @@ -46,7 +45,6 @@ require ( github.com/golang/mock v1.6.0 // indirect github.com/golang/protobuf v1.5.3 // indirect github.com/google/uuid v1.6.0 // indirect - github.com/hashicorp/go-version v1.6.0 // indirect github.com/json-iterator/go v1.1.12 // indirect github.com/karrick/godirwalk v1.17.0 // indirect github.com/knadh/koanf/maps v0.1.1 // indirect diff --git a/internal/datadog/go.sum b/internal/datadog/go.sum index 985bf1f7b9078..bc964448c0a40 100644 --- a/internal/datadog/go.sum +++ b/internal/datadog/go.sum @@ -85,8 +85,6 @@ github.com/google/gofuzz v1.2.0 h1:xRy4A+RhZaiKjJ1bPfwQ8sedCA+YS2YcCHW6ec7JMi0= github.com/google/gofuzz v1.2.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg= github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0= github.com/google/uuid v1.6.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= -github.com/hashicorp/go-version v1.6.0 h1:feTTfFNnjP967rlCxM/I9g701jU+RN74YKx2mOkIeek= -github.com/hashicorp/go-version v1.6.0/go.mod h1:fltr4n8CU8Ke44wwGCBoEymUuxUHl09ZGVZPK5anwXA= github.com/json-iterator/go v1.1.12 h1:PV8peI4a0ysnczrg+LtxykD8LfKY9ML6u2jnxaEnrnM= github.com/json-iterator/go v1.1.12/go.mod h1:e30LSqwooZae/UwlEbR2852Gd8hjQvJoHmT4TnhNGBo= github.com/karrick/godirwalk v1.17.0 h1:b4kY7nqDdioR/6qnbHQyDvmA17u5G1cZ6J+CZXwSWoI= @@ -192,8 +190,6 @@ go.opentelemetry.io/collector/config/configtelemetry v0.96.1-0.20240306115632-b2 go.opentelemetry.io/collector/config/configtelemetry v0.96.1-0.20240306115632-b2693620eff6/go.mod h1:YV5PaOdtnU1xRomPcYqoHmyCr48tnaAREeGO96EZw8o= go.opentelemetry.io/collector/confmap v0.96.1-0.20240306115632-b2693620eff6 h1:nsskiJcF5iL6gYZ/jjkeIShdQZMbJFwOnR5BqctrAiI= go.opentelemetry.io/collector/confmap v0.96.1-0.20240306115632-b2693620eff6/go.mod h1:AnJmZcZoOLuykSXGiAf3shi11ZZk5ei4tZd9dDTTpWE= -go.opentelemetry.io/collector/featuregate v1.3.1-0.20240306115632-b2693620eff6 h1:WPX5pMQgNPvjLrtQ+XoBBsbyhy1m1JtYc1B/rIFhCnQ= -go.opentelemetry.io/collector/featuregate v1.3.1-0.20240306115632-b2693620eff6/go.mod h1:w7nUODKxEi3FLf1HslCiE6YWtMtOOrMnSwsDam8Mg9w= go.opentelemetry.io/collector/pdata v1.3.1-0.20240306115632-b2693620eff6 h1:YV+WmEZfM0wv4pUtj5uJsLgC1lHgGp8WKhzyNphyX9Y= go.opentelemetry.io/collector/pdata v1.3.1-0.20240306115632-b2693620eff6/go.mod h1:0Ttp4wQinhV5oJTd9MjyvUegmZBO9O0nrlh/+EDLw+Q= go.opentelemetry.io/collector/semconv v0.96.1-0.20240306115632-b2693620eff6 h1:wLnFcJSimh5/axOIYGssu09e/9m4oy8dkj9nfFS3QP8= From 3351c9dcb10a12851019bab09048a1c4bb70b5bd Mon Sep 17 00:00:00 2001 From: Janik K <10290002+led0nk@users.noreply.github.com> Date: Fri, 15 Mar 2024 14:23:34 +0100 Subject: [PATCH 3/3] [internal/sqlquery] use errors.Join instead of multierr (#31768) **Link to tracking Issue:** - #25121 --- internal/sqlquery/config.go | 37 ++++++++++++++--------------- internal/sqlquery/db_client.go | 8 +++---- internal/sqlquery/db_client_test.go | 14 +++++++---- internal/sqlquery/scraper.go | 7 +++--- 4 files changed, 34 insertions(+), 32 deletions(-) diff --git a/internal/sqlquery/config.go b/internal/sqlquery/config.go index d45fbf4a99609..ceb2b4556cfdf 100644 --- a/internal/sqlquery/config.go +++ b/internal/sqlquery/config.go @@ -9,7 +9,6 @@ import ( "go.opentelemetry.io/collector/component" "go.opentelemetry.io/collector/receiver/scraperhelper" - "go.uber.org/multierr" ) type Config struct { @@ -48,24 +47,24 @@ type Query struct { } func (q Query) Validate() error { - var errs error + var errs []error if q.SQL == "" { - errs = multierr.Append(errs, errors.New("'query.sql' cannot be empty")) + errs = append(errs, errors.New("'query.sql' cannot be empty")) } if len(q.Logs) == 0 && len(q.Metrics) == 0 { - errs = multierr.Append(errs, errors.New("at least one of 'query.logs' and 'query.metrics' must not be empty")) + errs = append(errs, errors.New("at least one of 'query.logs' and 'query.metrics' must not be empty")) } for _, logs := range q.Logs { if err := logs.Validate(); err != nil { - errs = multierr.Append(errs, err) + errs = append(errs, err) } } for _, metric := range q.Metrics { if err := metric.Validate(); err != nil { - errs = multierr.Append(errs, err) + errs = append(errs, err) } } - return errs + return errors.Join(errs...) } type LogsCfg struct { @@ -73,11 +72,11 @@ type LogsCfg struct { } func (config LogsCfg) Validate() error { - var errs error + var errs []error if config.BodyColumn == "" { - errs = multierr.Append(errs, errors.New("'body_column' must not be empty")) + errs = append(errs, errors.New("'body_column' must not be empty")) } - return errs + return errors.Join(errs...) } type MetricCfg struct { @@ -96,29 +95,29 @@ type MetricCfg struct { } func (c MetricCfg) Validate() error { - var errs error + var errs []error if c.MetricName == "" { - errs = multierr.Append(errs, errors.New("'metric_name' cannot be empty")) + errs = append(errs, errors.New("'metric_name' cannot be empty")) } if c.ValueColumn == "" { - errs = multierr.Append(errs, errors.New("'value_column' cannot be empty")) + errs = append(errs, errors.New("'value_column' cannot be empty")) } if err := c.ValueType.Validate(); err != nil { - errs = multierr.Append(errs, err) + errs = append(errs, err) } if err := c.DataType.Validate(); err != nil { - errs = multierr.Append(errs, err) + errs = append(errs, err) } if err := c.Aggregation.Validate(); err != nil { - errs = multierr.Append(errs, err) + errs = append(errs, err) } if c.DataType == MetricTypeGauge && c.Aggregation != "" { - errs = multierr.Append(errs, fmt.Errorf("aggregation=%s but data_type=%s does not support aggregation", c.Aggregation, c.DataType)) + errs = append(errs, fmt.Errorf("aggregation=%s but data_type=%s does not support aggregation", c.Aggregation, c.DataType)) } if errs != nil && c.MetricName != "" { - errs = multierr.Append(fmt.Errorf("invalid metric config with metric_name '%s'", c.MetricName), errs) + errs = append(errs, fmt.Errorf("invalid metric config with metric_name '%s'", c.MetricName)) } - return errs + return errors.Join(errs...) } type MetricType string diff --git a/internal/sqlquery/db_client.go b/internal/sqlquery/db_client.go index 9f7d00908f0f8..e702a6b829236 100644 --- a/internal/sqlquery/db_client.go +++ b/internal/sqlquery/db_client.go @@ -5,6 +5,7 @@ package sqlquery // import "github.com/open-telemetry/opentelemetry-collector-co import ( "context" + "errors" // register Db drivers _ "github.com/SAP/go-hdb/driver" @@ -14,7 +15,6 @@ import ( _ "github.com/microsoft/go-mssqldb/integratedauth/krb5" _ "github.com/sijms/go-ora/v2" _ "github.com/snowflakedb/gosnowflake" - "go.uber.org/multierr" "go.uber.org/zap" ) @@ -52,7 +52,7 @@ func (cl DbSQLClient) QueryRows(ctx context.Context, args ...any) ([]StringMap, return nil, err } scanner := newRowScanner(colTypes) - var warnings error + var warnings []error for sqlRows.Next() { err = scanner.scan(sqlRows) if err != nil { @@ -60,11 +60,11 @@ func (cl DbSQLClient) QueryRows(ctx context.Context, args ...any) ([]StringMap, } sm, scanErr := scanner.toStringMap() if scanErr != nil { - warnings = multierr.Append(warnings, scanErr) + warnings = append(warnings, scanErr) } out = append(out, sm) } - return out, warnings + return out, errors.Join(warnings...) } func (cl DbSQLClient) prepareQueryFields(sql string, args []any) []zap.Field { diff --git a/internal/sqlquery/db_client_test.go b/internal/sqlquery/db_client_test.go index a40ebd02e31f1..4546886b0ec0e 100644 --- a/internal/sqlquery/db_client_test.go +++ b/internal/sqlquery/db_client_test.go @@ -11,7 +11,6 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "go.uber.org/multierr" "go.uber.org/zap" ) @@ -90,11 +89,16 @@ func TestDBSQLClient_Nulls_MultiRow(t *testing.T) { } rows, err := cl.QueryRows(context.Background()) assert.Error(t, err) - errs := multierr.Errors(err) - for _, err := range errs { - assert.True(t, errors.Is(err, errNullValueWarning)) + + var e interface{ Unwrap() []error } + if errors.As(err, &e) { + uw := e.Unwrap() + assert.Len(t, uw, 2) + + for _, err := range uw { + assert.True(t, errors.Is(err, errNullValueWarning)) + } } - assert.Len(t, errs, 2) assert.Len(t, rows, 2) assert.EqualValues(t, map[string]string{ "col_0": "42", diff --git a/internal/sqlquery/scraper.go b/internal/sqlquery/scraper.go index e43edf4aea340..84be8370911ef 100644 --- a/internal/sqlquery/scraper.go +++ b/internal/sqlquery/scraper.go @@ -15,7 +15,6 @@ import ( "go.opentelemetry.io/collector/pdata/pmetric" "go.opentelemetry.io/collector/receiver/scrapererror" "go.opentelemetry.io/collector/receiver/scraperhelper" - "go.uber.org/multierr" "go.uber.org/zap" ) @@ -84,17 +83,17 @@ func (s *Scraper) Scrape(ctx context.Context) (pmetric.Metrics, error) { sms := rm.ScopeMetrics() sm := sms.AppendEmpty() ms := sm.Metrics() - var errs error + var errs []error for _, metricCfg := range s.Query.Metrics { for i, row := range rows { if err = rowToMetric(row, metricCfg, ms.AppendEmpty(), s.StartTime, ts, s.ScrapeCfg); err != nil { err = fmt.Errorf("row %d: %w", i, err) - errs = multierr.Append(errs, err) + errs = append(errs, err) } } } if errs != nil { - return out, scrapererror.NewPartialScrapeError(errs, len(multierr.Errors(errs))) + return out, scrapererror.NewPartialScrapeError(errors.Join(errs...), len(errs)) } return out, nil }