Skip to content

Commit

Permalink
[exporter/logicmonitorexporter] Fix leaking goroutines on shutdown (#…
Browse files Browse the repository at this point in the history
…31150)

**Description:** <Describe what has changed.>
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
The [logicmonitor
dependency](https://github.com/logicmonitor/lm-data-sdk-go) that handles
exporting data relies on the context being cancelled to shutdown its
running goroutines ([traces
reference](https://github.com/logicmonitor/lm-data-sdk-go/blob/93e0505a0dce33bfd874c67bdf85e3845126cc3d/pkg/ratelimiter/traces.go#L113),
[logs
reference](https://github.com/logicmonitor/lm-data-sdk-go/blob/93e0505a0dce33bfd874c67bdf85e3845126cc3d/pkg/ratelimiter/logs.go#L70)).
The exporter has now been updated to properly cancel the passed context
to fix leaking goroutines on shutdown. This change also enables `goleak`
to check for leaking goroutines in the logicmonitor exporter package.

**Link to tracking Issue:** <Issue number if applicable>
#30438

**Testing:** <Describe what testing was performed and which tests were
added.>
All existing tests are passing, as well as added `goleak` check.
  • Loading branch information
crobert-1 authored Feb 12, 2024
1 parent 384a94d commit 115fe04
Show file tree
Hide file tree
Showing 7 changed files with 68 additions and 1 deletion.
27 changes: 27 additions & 0 deletions .chloggen/goleak_logicmonitorexp.yaml
Original file line number Diff line number Diff line change
@@ -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: bug_fix

# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver)
component: logicmonitorexporter

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: Fix memory leak on shutdown

# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists.
issues: [31150]

# (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: []
5 changes: 4 additions & 1 deletion exporter/logicmonitorexporter/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ func createLogsExporter(ctx context.Context, set exporter.CreateSettings, cfg co
exporterhelper.WithStart(lmLogExp.start),
exporterhelper.WithQueue(c.QueueSettings),
exporterhelper.WithRetry(c.BackOffConfig),
exporterhelper.WithShutdown(lmLogExp.shutdown),
)
}

Expand All @@ -61,5 +62,7 @@ func createTracesExporter(ctx context.Context, set exporter.CreateSettings, cfg
exporterhelper.WithCapabilities(consumer.Capabilities{MutatesData: false}),
exporterhelper.WithStart(lmTraceExp.start),
exporterhelper.WithRetry(c.BackOffConfig),
exporterhelper.WithQueue(c.QueueSettings))
exporterhelper.WithQueue(c.QueueSettings),
exporterhelper.WithShutdown(lmTraceExp.shutdown),
)
}
10 changes: 10 additions & 0 deletions exporter/logicmonitorexporter/logs_exporter.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ type logExporter struct {
config *Config
sender *logs.Sender
settings component.TelemetrySettings
cancel context.CancelFunc
}

// Create new logicmonitor logs exporter
Expand All @@ -52,6 +53,7 @@ func (e *logExporter) start(ctx context.Context, host component.Host) error {

opts := buildLogIngestOpts(e.config, client)

ctx, e.cancel = context.WithCancel(ctx)
e.sender, err = logs.NewSender(ctx, e.settings.Logger, opts...)
if err != nil {
return err
Expand Down Expand Up @@ -95,6 +97,14 @@ func (e *logExporter) PushLogData(ctx context.Context, lg plog.Logs) error {
return e.sender.SendLogs(ctx, payload)
}

func (e *logExporter) shutdown(_ context.Context) error {
if e.cancel != nil {
e.cancel()
}

return nil
}

func buildLogIngestOpts(config *Config, client *http.Client) []lmsdklogs.Option {
authParams := utils.AuthParams{
AccessID: config.APIToken.AccessID,
Expand Down
1 change: 1 addition & 0 deletions exporter/logicmonitorexporter/logs_exporter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ func TestPushLogData(t *testing.T) {
exp := newLogsExporter(test.args.ctx, test.fields.config, set)

require.NoError(t, exp.start(test.args.ctx, componenttest.NewNopHost()))
defer func() { require.NoError(t, exp.shutdown(test.args.ctx)) }()
err := exp.PushLogData(test.args.ctx, test.args.lg)
assert.NoError(t, err)
})
Expand Down
14 changes: 14 additions & 0 deletions exporter/logicmonitorexporter/package_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// Copyright The OpenTelemetry Authors
// SPDX-License-Identifier: Apache-2.0

package logicmonitorexporter

import (
"testing"

"go.uber.org/goleak"
)

func TestMain(m *testing.M) {
goleak.VerifyTestMain(m)
}
11 changes: 11 additions & 0 deletions exporter/logicmonitorexporter/traces_exporter.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ type tracesExporter struct {
config *Config
sender *traces.Sender
settings component.TelemetrySettings
cancel context.CancelFunc
}

// newTracesExporter creates new Logicmonitor Traces Exporter.
Expand All @@ -43,6 +44,8 @@ func (e *tracesExporter) start(ctx context.Context, host component.Host) error {
AccessKey: string(e.config.APIToken.AccessKey),
BearerToken: string(e.config.Headers["Authorization"]),
}

ctx, e.cancel = context.WithCancel(ctx)
e.sender, err = traces.NewSender(ctx, e.config.Endpoint, client, authParams, e.settings.Logger)
if err != nil {
return err
Expand All @@ -53,3 +56,11 @@ func (e *tracesExporter) start(ctx context.Context, host component.Host) error {
func (e *tracesExporter) PushTraceData(ctx context.Context, td ptrace.Traces) error {
return e.sender.SendTraces(ctx, td)
}

func (e *tracesExporter) shutdown(_ context.Context) error {
if e.cancel != nil {
e.cancel()
}

return nil
}
1 change: 1 addition & 0 deletions exporter/logicmonitorexporter/traces_exporter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ func TestPushTraceData(t *testing.T) {
exp, err := f.CreateTracesExporter(ctx, params, config)
assert.NoError(t, err)
assert.NoError(t, exp.Start(ctx, componenttest.NewNopHost()))
defer func() { assert.NoError(t, exp.Shutdown(ctx)) }()

testTraces := ptrace.NewTraces()
generateTraces().CopyTo(testTraces)
Expand Down

0 comments on commit 115fe04

Please sign in to comment.