Skip to content

Commit 1eeeca6

Browse files
pjanottiAkhigbeEromo
authored andcommitted
[receiver/opencensus] Do not report error message on clean shutdown (open-telemetry#36622)
#### Description The receiver is reporting error on a clean shutdown because it is not looking for the correct error. Besides logging a false-positive error message this can end up in a deadlock during shutdown due to open-telemetry/opentelemetry-collector#9824 #### Testing Added a test specific for that. In principle, this should be part of the generated tests, but, to do that it will be necessary to add support to the `Reporter` interface. #### Documentation Changelog.
1 parent 9554dee commit 1eeeca6

File tree

3 files changed

+67
-3
lines changed

3 files changed

+67
-3
lines changed
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: opencensusreceiver
8+
9+
# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
10+
note: Do not report error message when OpenCensus receiver is shutdown cleanly.
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: [36622]
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: [user]

receiver/opencensusreceiver/opencensus.go

+3-3
Original file line numberDiff line numberDiff line change
@@ -146,19 +146,19 @@ func (ocr *ocReceiver) Start(ctx context.Context, host component.Host) error {
146146
defer ocr.stopWG.Done()
147147
startWG.Done()
148148
// Check for cmux.ErrServerClosed, because during the shutdown this is not properly close before closing the cmux,
149-
if err := ocr.serverGRPC.Serve(grpcL); !errors.Is(err, grpc.ErrServerStopped) && !errors.Is(err, cmux.ErrServerClosed) && err != nil {
149+
if err := ocr.serverGRPC.Serve(grpcL); err != nil && !errors.Is(err, grpc.ErrServerStopped) && !errors.Is(err, cmux.ErrServerClosed) {
150150
componentstatus.ReportStatus(host, componentstatus.NewFatalErrorEvent(err))
151151
}
152152
}()
153153
go func() {
154154
startWG.Done()
155-
if err := ocr.serverHTTP.Serve(httpL); !errors.Is(err, http.ErrServerClosed) && !errors.Is(err, cmux.ErrServerClosed) && err != nil {
155+
if err := ocr.serverHTTP.Serve(httpL); err != nil && !errors.Is(err, http.ErrServerClosed) && !errors.Is(err, cmux.ErrServerClosed) {
156156
componentstatus.ReportStatus(host, componentstatus.NewFatalErrorEvent(err))
157157
}
158158
}()
159159
go func() {
160160
startWG.Done()
161-
if err := ocr.multiplexer.Serve(); !errors.Is(err, cmux.ErrServerClosed) && !errors.Is(err, cmux.ErrListenerClosed) && err != nil {
161+
if err := ocr.multiplexer.Serve(); err != nil && !errors.Is(err, net.ErrClosed) {
162162
componentstatus.ReportStatus(host, componentstatus.NewFatalErrorEvent(err))
163163
}
164164
}()

receiver/opencensusreceiver/opencensus_test.go

+37
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import (
2828
"github.com/stretchr/testify/assert"
2929
"github.com/stretchr/testify/require"
3030
"go.opentelemetry.io/collector/component"
31+
"go.opentelemetry.io/collector/component/componentstatus"
3132
"go.opentelemetry.io/collector/component/componenttest"
3233
"go.opentelemetry.io/collector/config/configgrpc"
3334
"go.opentelemetry.io/collector/config/confignet"
@@ -774,6 +775,28 @@ func TestInvalidTLSCredentials(t *testing.T) {
774775
assert.Nil(t, srv)
775776
}
776777

778+
func TestStartShutdownShouldNotReportError(t *testing.T) {
779+
addr := testutil.GetAvailableLocalAddress(t)
780+
cfg := Config{
781+
ServerConfig: configgrpc.ServerConfig{
782+
NetAddr: confignet.AddrConfig{
783+
Endpoint: addr,
784+
Transport: "tcp",
785+
},
786+
},
787+
}
788+
ocr := newOpenCensusReceiver(&cfg, new(consumertest.TracesSink), new(consumertest.MetricsSink), receivertest.NewNopSettings())
789+
require.NotNil(t, ocr)
790+
791+
nopHostReporter := &nopHost{
792+
reportFunc: func(event *componentstatus.Event) {
793+
assert.NoError(t, event.Err())
794+
},
795+
}
796+
require.NoError(t, ocr.Start(context.Background(), nopHostReporter))
797+
require.NoError(t, ocr.Shutdown(context.Background()))
798+
}
799+
777800
type errOrSinkConsumer struct {
778801
*consumertest.TracesSink
779802
*consumertest.MetricsSink
@@ -829,3 +852,17 @@ func (esc *errOrSinkConsumer) Reset() {
829852
esc.MetricsSink.Reset()
830853
}
831854
}
855+
856+
var _ componentstatus.Reporter = (*nopHost)(nil)
857+
858+
type nopHost struct {
859+
reportFunc func(event *componentstatus.Event)
860+
}
861+
862+
func (nh *nopHost) GetExtensions() map[component.ID]component.Component {
863+
return nil
864+
}
865+
866+
func (nh *nopHost) Report(event *componentstatus.Event) {
867+
nh.reportFunc(event)
868+
}

0 commit comments

Comments
 (0)