Skip to content

Commit 36084a9

Browse files
authored
[receiver/jmx] Fix memory leak on shutdown (open-telemetry#32289)
**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 receiver was starting a goroutine that would run without being stopped during shutdown. This changes the goroutine to be stopped during shutdown. `goleak` is also added as a part of this change. **Link to tracking Issue:** <Issue number if applicable> open-telemetry#30438 **Testing:** <Describe what testing was performed and which tests were added.> All existing tests are passing, as well as added `goleak` check.
1 parent d752888 commit 36084a9

File tree

4 files changed

+60
-4
lines changed

4 files changed

+60
-4
lines changed

.chloggen/goleak_jmxrec.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: bug_fix
5+
6+
# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver)
7+
component: jmxreceiver
8+
9+
# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
10+
note: Fix memory leak during component shutdown
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: [32289]
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: []

receiver/jmxreceiver/go.mod

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ require (
1919
go.opentelemetry.io/collector/receiver/otlpreceiver v0.97.1-0.20240409140257-792fac1b62d4
2020
go.opentelemetry.io/otel/metric v1.24.0
2121
go.opentelemetry.io/otel/trace v1.24.0
22+
go.uber.org/goleak v1.3.0
2223
go.uber.org/zap v1.27.0
2324
)
2425

receiver/jmxreceiver/package_test.go

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
// Copyright The OpenTelemetry Authors
2+
// SPDX-License-Identifier: Apache-2.0
3+
4+
package jmxreceiver
5+
6+
import (
7+
"testing"
8+
9+
"go.uber.org/goleak"
10+
)
11+
12+
func TestMain(m *testing.M) {
13+
goleak.VerifyTestMain(m)
14+
}

receiver/jmxreceiver/receiver.go

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ type jmxMetricReceiver struct {
3636
otlpReceiver receiver.Metrics
3737
nextConsumer consumer.Metrics
3838
configFile string
39+
cancel context.CancelFunc
3940
}
4041

4142
func newJMXMetricReceiver(
@@ -54,6 +55,8 @@ func newJMXMetricReceiver(
5455
func (jmx *jmxMetricReceiver) Start(ctx context.Context, host component.Host) error {
5556
jmx.logger.Debug("starting JMX Receiver")
5657

58+
ctx, jmx.cancel = context.WithCancel(ctx)
59+
5760
var err error
5861
jmx.otlpReceiver, err = jmx.buildOTLPReceiver()
5962
if err != nil {
@@ -98,13 +101,19 @@ func (jmx *jmxMetricReceiver) Start(ctx context.Context, host component.Host) er
98101
return err
99102
}
100103
go func() {
101-
for range jmx.subprocess.Stdout { // nolint
102-
// ensure stdout/stderr buffer is read from.
103-
// these messages are already debug logged when captured.
104+
for {
105+
select {
106+
case <-ctx.Done():
107+
return
108+
case <-jmx.subprocess.Stdout:
109+
// ensure stdout/stderr buffer is read from.
110+
// these messages are already debug logged when captured.
111+
continue
112+
}
104113
}
105114
}()
106115

107-
return jmx.subprocess.Start(context.Background())
116+
return jmx.subprocess.Start(ctx)
108117
}
109118

110119
func (jmx *jmxMetricReceiver) Shutdown(ctx context.Context) error {
@@ -114,6 +123,11 @@ func (jmx *jmxMetricReceiver) Shutdown(ctx context.Context) error {
114123
jmx.logger.Debug("Shutting down JMX Receiver")
115124
subprocessErr := jmx.subprocess.Shutdown(ctx)
116125
otlpErr := jmx.otlpReceiver.Shutdown(ctx)
126+
127+
if jmx.cancel != nil {
128+
jmx.cancel()
129+
}
130+
117131
removeErr := os.Remove(jmx.configFile)
118132
if subprocessErr != nil {
119133
return subprocessErr

0 commit comments

Comments
 (0)