Skip to content

Commit 030d9ad

Browse files
bogdandrutuAkshayS198
authored andcommitted
[stanza] Remove unnecessary slice allocation to track errors (even nil) (open-telemetry#39367)
The most inefficient usages are in ProcessBatch and ProcessBatchWith, where we allocate a slice of errors even if return value is nil. Changed all usages to make sure we don't repeat that mistake and establish a healthier pattern. Signed-off-by: Bogdan Drutu <[email protected]>
1 parent 007d9ea commit 030d9ad

File tree

14 files changed

+93
-68
lines changed

14 files changed

+93
-68
lines changed

.chloggen/stanza-error.yaml

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
# Use this changelog template to create an entry for release notes.
2+
3+
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
4+
change_type: enhancement
5+
6+
# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver)
7+
component: pkg/stanza
8+
9+
# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
10+
note: Remove unnecessary slice allocation to track errors (even nil)
11+
12+
# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists.
13+
issues: [39367]
14+
15+
# (Optional) One or more lines of additional information to render under the primary note.
16+
# These lines will be padded with 2 spaces and then inserted directly into the document.
17+
# Use pipe (|) for multiline entries.
18+
subtext:
19+
20+
# If your change doesn't affect end users or the exported elements of any package,
21+
# you should instead start your pull request title with [chore] or use the "Skip Changelog" label.
22+
# Optional: The change log or logs in which this entry should be included.
23+
# e.g. '[user]' or '[user, api]'
24+
# Include 'user' if the change is relevant to end users.
25+
# Include 'api' if there is a change to a library API.
26+
# Default: '[user]'
27+
change_logs: []

pkg/stanza/fileconsumer/internal/checkpoint/checkpoint.go

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"fmt"
1212

1313
"go.opentelemetry.io/collector/extension/xextension/storage"
14+
"go.uber.org/multierr"
1415

1516
"github.com/open-telemetry/opentelemetry-collector-contrib/pkg/stanza/fileconsumer/internal/reader"
1617
"github.com/open-telemetry/opentelemetry-collector-contrib/pkg/stanza/operator"
@@ -32,19 +33,19 @@ func SaveKey(ctx context.Context, persister operator.Persister, rmds []*reader.M
3233
return fmt.Errorf("encode num files: %w", err)
3334
}
3435

35-
var errs []error
36+
var errs error
3637
// Encode each known file
3738
for _, rmd := range rmds {
3839
if err := enc.Encode(rmd); err != nil {
39-
errs = append(errs, fmt.Errorf("encode metadata: %w", err))
40+
errs = multierr.Append(errs, fmt.Errorf("encode metadata: %w", err))
4041
}
4142
}
4243
ops = append(ops, storage.SetOperation(key, buf.Bytes()))
4344
if err := persister.Batch(ctx, ops...); err != nil {
44-
errs = append(errs, fmt.Errorf("persist known files: %w", err))
45+
errs = multierr.Append(errs, fmt.Errorf("persist known files: %w", err))
4546
}
4647

47-
return errors.Join(errs...)
48+
return errs
4849
}
4950

5051
// Load loads the most recent set of files to the database
@@ -71,7 +72,7 @@ func LoadKey(ctx context.Context, persister operator.Persister, key string) ([]*
7172
}
7273

7374
// Decode each of the known files
74-
var errs []error
75+
var errs error
7576
rmds := make([]*reader.Metadata, 0, knownFileCount)
7677
for i := 0; i < knownFileCount; i++ {
7778
rmd := new(reader.Metadata)
@@ -92,13 +93,13 @@ func LoadKey(ctx context.Context, persister operator.Persister, key string) ([]*
9293
}
9394
delete(rmd.FileAttributes, "HeaderAttributes")
9495
default:
95-
errs = append(errs, errors.New("migrate header attributes: unexpected format"))
96+
errs = multierr.Append(errs, errors.New("migrate header attributes: unexpected format"))
9697
}
9798
}
9899

99100
// This reader won't be used for anything other than metadata reference, so just wrap the metadata
100101
rmds = append(rmds, rmd)
101102
}
102103

103-
return rmds, errors.Join(errs...)
104+
return rmds, errs
104105
}

pkg/stanza/fileconsumer/matcher/internal/finder/finder.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,12 @@
44
package finder // import "github.com/open-telemetry/opentelemetry-collector-contrib/pkg/stanza/fileconsumer/matcher/internal/finder"
55

66
import (
7-
"errors"
87
"fmt"
98
"maps"
109
"slices"
1110

1211
"github.com/bmatcuk/doublestar/v4"
12+
"go.uber.org/multierr"
1313
)
1414

1515
func Validate(globs []string) error {
@@ -30,7 +30,7 @@ func FindFiles(includes []string, excludes []string) ([]string, error) {
3030
for _, include := range includes {
3131
matches, err := doublestar.FilepathGlob(include, doublestar.WithFilesOnly(), doublestar.WithFailOnIOErrors())
3232
if err != nil {
33-
errs = errors.Join(errs, fmt.Errorf("find files with '%s' pattern: %w", include, err))
33+
errs = multierr.Append(errs, fmt.Errorf("find files with '%s' pattern: %w", include, err))
3434
// the same pattern could cause an IO error due to one file or directory,
3535
// but also could still find files without `doublestar.WithFailOnIOErrors()`.
3636
matches, _ = doublestar.FilepathGlob(include, doublestar.WithFilesOnly())

pkg/stanza/fileconsumer/matcher/matcher.go

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"time"
1111

1212
"go.opentelemetry.io/collector/featuregate"
13+
"go.uber.org/multierr"
1314

1415
"github.com/open-telemetry/opentelemetry-collector-contrib/pkg/stanza/fileconsumer/matcher/internal/filter"
1516
"github.com/open-telemetry/opentelemetry-collector-contrib/pkg/stanza/fileconsumer/matcher/internal/finder"
@@ -173,11 +174,9 @@ type Matcher struct {
173174
func (m Matcher) MatchFiles() ([]string, error) {
174175
var errs error
175176
files, err := finder.FindFiles(m.include, m.exclude)
176-
if err != nil {
177-
errs = errors.Join(errs, err)
178-
}
177+
errs = multierr.Append(errs, err)
179178
if len(files) == 0 {
180-
return files, errors.Join(errors.New("no files match the configured criteria"), errs)
179+
return files, multierr.Append(errors.New("no files match the configured criteria"), errs)
181180
}
182181
if len(m.filterOpts) == 0 {
183182
return files, errs
@@ -200,7 +199,7 @@ func (m Matcher) MatchFiles() ([]string, error) {
200199
for _, groupedFiles := range groups {
201200
groupResult, err := filter.Filter(groupedFiles, m.regex, m.filterOpts...)
202201
if len(groupResult) == 0 {
203-
return groupResult, errors.Join(err, errs)
202+
return groupResult, multierr.Append(err, errs)
204203
}
205204
result = append(result, groupResult...)
206205
}

pkg/stanza/operator/helper/transformer.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010

1111
"github.com/expr-lang/expr/vm"
1212
"go.opentelemetry.io/collector/component"
13+
"go.uber.org/multierr"
1314
"go.uber.org/zap"
1415
"go.uber.org/zap/zapcore"
1516

@@ -78,11 +79,11 @@ func (t *TransformerOperator) CanProcess() bool {
7879
}
7980

8081
func (t *TransformerOperator) ProcessBatchWith(ctx context.Context, entries []*entry.Entry, process ProcessFunction) error {
81-
var errs []error
82+
var errs error
8283
for i := range entries {
83-
errs = append(errs, process(ctx, entries[i]))
84+
errs = multierr.Append(errs, process(ctx, entries[i]))
8485
}
85-
return errors.Join(errs...)
86+
return errs
8687
}
8788

8889
// ProcessWith will process an entry with a transform function.

pkg/stanza/operator/input/file/input.go

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,9 @@ package file // import "github.com/open-telemetry/opentelemetry-collector-contri
55

66
import (
77
"context"
8-
"errors"
98
"fmt"
109

10+
"go.uber.org/multierr"
1111
"go.uber.org/zap"
1212

1313
"github.com/open-telemetry/opentelemetry-collector-contrib/pkg/stanza/entry"
@@ -40,22 +40,22 @@ func (i *Input) Stop() error {
4040
}
4141

4242
func (i *Input) emitBatch(ctx context.Context, tokens [][]byte, attributes map[string]any, lastRecordNumber int64) error {
43-
entries, conversionError := i.convertTokens(tokens, attributes, lastRecordNumber)
44-
if conversionError != nil {
45-
conversionError = fmt.Errorf("convert tokens: %w", conversionError)
43+
var errs error
44+
entries, err := i.convertTokens(tokens, attributes, lastRecordNumber)
45+
if err != nil {
46+
errs = multierr.Append(errs, fmt.Errorf("convert tokens: %w", err))
4647
}
4748

48-
consumeError := i.WriteBatch(ctx, entries)
49-
if consumeError != nil {
50-
consumeError = fmt.Errorf("consume entries: %w", consumeError)
49+
if err = i.WriteBatch(ctx, entries); err != nil {
50+
errs = multierr.Append(errs, fmt.Errorf("consume entries: %w", err))
5151
}
5252

53-
return errors.Join(conversionError, consumeError)
53+
return errs
5454
}
5555

5656
func (i *Input) convertTokens(tokens [][]byte, attributes map[string]any, lastRecordNumber int64) ([]*entry.Entry, error) {
5757
entries := make([]*entry.Entry, 0, len(tokens))
58-
var errs []error
58+
var errs error
5959

6060
for tokenIndex, token := range tokens {
6161
if len(token) == 0 {
@@ -64,7 +64,7 @@ func (i *Input) convertTokens(tokens [][]byte, attributes map[string]any, lastRe
6464

6565
ent, err := i.NewEntry(i.toBody(token))
6666
if err != nil {
67-
errs = append(errs, fmt.Errorf("create entry: %w", err))
67+
errs = multierr.Append(errs, fmt.Errorf("create entry: %w", err))
6868
continue
6969
}
7070

@@ -82,5 +82,5 @@ func (i *Input) convertTokens(tokens [][]byte, attributes map[string]any, lastRe
8282

8383
entries = append(entries, ent)
8484
}
85-
return entries, errors.Join(errs...)
85+
return entries, errs
8686
}

pkg/stanza/operator/input/windows/input.go

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import (
1313
"time"
1414

1515
"go.opentelemetry.io/collector/component"
16+
"go.uber.org/multierr"
1617
"go.uber.org/zap"
1718
"golang.org/x/sys/windows"
1819

@@ -165,18 +166,18 @@ func (i *Input) Stop() error {
165166

166167
var errs error
167168
if err := i.subscription.Close(); err != nil {
168-
errs = errors.Join(errs, fmt.Errorf("failed to close subscription: %w", err))
169+
errs = multierr.Append(errs, fmt.Errorf("failed to close subscription: %w", err))
169170
}
170171

171172
if err := i.bookmark.Close(); err != nil {
172-
errs = errors.Join(errs, fmt.Errorf("failed to close bookmark: %w", err))
173+
errs = multierr.Append(errs, fmt.Errorf("failed to close bookmark: %w", err))
173174
}
174175

175176
if err := i.publisherCache.evictAll(); err != nil {
176-
errs = errors.Join(errs, fmt.Errorf("failed to close publishers: %w", err))
177+
errs = multierr.Append(errs, fmt.Errorf("failed to close publishers: %w", err))
177178
}
178179

179-
return errors.Join(errs, i.stopRemoteSession())
180+
return multierr.Append(errs, i.stopRemoteSession())
180181
}
181182

182183
// readOnInterval will read events with respect to the polling interval until it reaches the end of the channel.
@@ -272,7 +273,7 @@ func (i *Input) renderDeepAndSend(ctx context.Context, event Event, publisher Pu
272273
if err == nil {
273274
return i.sendEvent(ctx, deepEvent)
274275
}
275-
return errors.Join(
276+
return multierr.Append(
276277
fmt.Errorf("render deep event: %w", err),
277278
i.renderSimpleAndSend(ctx, event),
278279
)
@@ -297,7 +298,7 @@ func (i *Input) processEventWithRenderingInfo(ctx context.Context, event Event)
297298

298299
publisher, err := i.publisherCache.get(providerName)
299300
if err != nil {
300-
return errors.Join(
301+
return multierr.Append(
301302
fmt.Errorf("open event source for provider %q: %w", providerName, err),
302303
i.renderSimpleAndSend(ctx, event),
303304
)

pkg/stanza/operator/input/windows/publishercache.go

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
package windows // import "github.com/open-telemetry/opentelemetry-collector-contrib/pkg/stanza/operator/input/windows"
77

88
import (
9-
"errors"
9+
"go.uber.org/multierr"
1010
)
1111

1212
type publisherCache struct {
@@ -43,9 +43,7 @@ func (c *publisherCache) evictAll() error {
4343
var errs error
4444
for _, publisher := range c.cache {
4545
if publisher.Valid() {
46-
if err := publisher.Close(); err != nil {
47-
errs = errors.Join(errs, err)
48-
}
46+
errs = multierr.Append(errs, publisher.Close())
4947
}
5048
}
5149

pkg/stanza/operator/output/file/output.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,11 @@ package file // import "github.com/open-telemetry/opentelemetry-collector-contri
66
import (
77
"context"
88
"encoding/json"
9-
"errors"
109
"os"
1110
"sync"
1211
"text/template"
1312

13+
"go.uber.org/multierr"
1414
"go.uber.org/zap"
1515

1616
"github.com/open-telemetry/opentelemetry-collector-contrib/pkg/stanza/entry"
@@ -54,11 +54,11 @@ func (o *Output) Stop() error {
5454
}
5555

5656
func (o *Output) ProcessBatch(ctx context.Context, entries []*entry.Entry) error {
57-
var errs []error
57+
var errs error
5858
for i := range entries {
59-
errs = append(errs, o.Process(ctx, entries[i]))
59+
errs = multierr.Append(errs, o.Process(ctx, entries[i]))
6060
}
61-
return errors.Join(errs...)
61+
return errs
6262
}
6363

6464
// Process will write an entry to the output file.

pkg/stanza/operator/output/stdout/output.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,9 @@ package stdout // import "github.com/open-telemetry/opentelemetry-collector-cont
66
import (
77
"context"
88
"encoding/json"
9-
"errors"
109
"sync"
1110

11+
"go.uber.org/multierr"
1212
"go.uber.org/zap"
1313

1414
"github.com/open-telemetry/opentelemetry-collector-contrib/pkg/stanza/entry"
@@ -23,11 +23,11 @@ type Output struct {
2323
}
2424

2525
func (o *Output) ProcessBatch(ctx context.Context, entries []*entry.Entry) error {
26-
var errs []error
26+
var errs error
2727
for i := range entries {
28-
errs = append(errs, o.Process(ctx, entries[i]))
28+
errs = multierr.Append(errs, o.Process(ctx, entries[i]))
2929
}
30-
return errors.Join(errs...)
30+
return errs
3131
}
3232

3333
// Process will log entries received.

pkg/stanza/operator/parser/container/parser.go

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import (
1313
"time"
1414

1515
"github.com/goccy/go-json"
16+
"go.uber.org/multierr"
1617
"go.uber.org/zap"
1718

1819
"github.com/open-telemetry/opentelemetry-collector-contrib/internal/coreinternal/timeutils"
@@ -155,21 +156,19 @@ func (p *Parser) Stop() error {
155156
// nothing is started return
156157
return nil
157158
}
158-
var stopErrs []error
159-
err := p.recombineParser.Stop()
160-
if err != nil {
161-
stopErrs = append(stopErrs, fmt.Errorf("unable to stop the internal recombine operator: %w", err))
159+
var errs error
160+
if err := p.recombineParser.Stop(); err != nil {
161+
errs = multierr.Append(errs, fmt.Errorf("unable to stop the internal recombine operator: %w", err))
162162
}
163163
// the recombineParser will call the Process of the criLogEmitter synchronously so the entries will be first
164164
// written to the channel before the Stop of the recombineParser returns. Then since the criLogEmitter handles
165165
// the entries synchronously it is safe to call its Stop.
166166
// After criLogEmitter is stopped the crioConsumer will consume the remaining messages and return.
167-
err = p.criLogEmitter.Stop()
168-
if err != nil {
169-
stopErrs = append(stopErrs, fmt.Errorf("unable to stop the internal LogEmitter: %w", err))
167+
if err := p.criLogEmitter.Stop(); err != nil {
168+
errs = multierr.Append(errs, fmt.Errorf("unable to stop the internal LogEmitter: %w", err))
170169
}
171170
p.criConsumers.Wait()
172-
return errors.Join(stopErrs...)
171+
return errs
173172
}
174173

175174
// detectFormat will detect the container log format

0 commit comments

Comments
 (0)