Skip to content

Commit

Permalink
[chore] pprof extension ReportFatalError -> ReportStatus
Browse files Browse the repository at this point in the history
Remove use of deprecated host.ReportFatalError

Linked to #30501

Fixes #30584

Signed-off-by: Alex Boten <aboten@lightstep.com>
  • Loading branch information
Alex Boten committed Jan 17, 2024
1 parent 8f61b01 commit 54e4531
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 21 deletions.
2 changes: 1 addition & 1 deletion extension/pprofextension/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,5 +42,5 @@ func createExtension(_ context.Context, set extension.CreateSettings, cfg compon
return nil, errors.New("\"endpoint\" is required when using the \"pprof\" extension")
}

return newServer(*config, set.Logger), nil
return newServer(*config, set.TelemetrySettings), nil
}
22 changes: 11 additions & 11 deletions extension/pprofextension/pprofextension.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,14 @@ import (
var running = &atomic.Bool{}

type pprofExtension struct {
config Config
logger *zap.Logger
file *os.File
server http.Server
stopCh chan struct{}
config Config
file *os.File
server http.Server
stopCh chan struct{}
telemetrySettings component.TelemetrySettings
}

func (p *pprofExtension) Start(_ context.Context, host component.Host) error {
func (p *pprofExtension) Start(_ context.Context, _ component.Host) error {
// The runtime settings are global to the application, so while in principle it
// is possible to have more than one instance, running multiple will mean that
// the settings of the last started instance will prevail. In order to avoid
Expand Down Expand Up @@ -57,7 +57,7 @@ func (p *pprofExtension) Start(_ context.Context, host component.Host) error {
runtime.SetBlockProfileRate(p.config.BlockProfileFraction)
runtime.SetMutexProfileFraction(p.config.MutexProfileFraction)

p.logger.Info("Starting net/http/pprof server", zap.Any("config", p.config))
p.telemetrySettings.Logger.Info("Starting net/http/pprof server", zap.Any("config", p.config))
p.stopCh = make(chan struct{})
go func() {
defer func() {
Expand All @@ -67,7 +67,7 @@ func (p *pprofExtension) Start(_ context.Context, host component.Host) error {

// The listener ownership goes to the server.
if errHTTP := p.server.Serve(ln); !errors.Is(errHTTP, http.ErrServerClosed) && errHTTP != nil {
host.ReportFatalError(errHTTP)
p.telemetrySettings.ReportStatus(component.NewFatalErrorEvent(errHTTP))
}
}()

Expand Down Expand Up @@ -97,9 +97,9 @@ func (p *pprofExtension) Shutdown(context.Context) error {
return err
}

func newServer(config Config, logger *zap.Logger) *pprofExtension {
func newServer(config Config, params component.TelemetrySettings) *pprofExtension {
return &pprofExtension{
config: config,
logger: logger,
config: config,
telemetrySettings: params,
}
}
28 changes: 19 additions & 9 deletions extension/pprofextension/pprofextension_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@ import (
"testing"

"github.com/stretchr/testify/require"
"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/component/componenttest"
"go.opentelemetry.io/collector/config/confignet"
"go.uber.org/zap"

"github.com/open-telemetry/opentelemetry-collector-contrib/internal/common/testutil"
)
Expand All @@ -27,8 +27,10 @@ func TestPerformanceProfilerExtensionUsage(t *testing.T) {
BlockProfileFraction: 3,
MutexProfileFraction: 5,
}
tt, err := componenttest.SetupTelemetry(component.NewID("TestPprofExtension"))
require.NoError(t, err, "SetupTelemetry should succeed")

pprofExt := newServer(config, zap.NewNop())
pprofExt := newServer(config, tt.TelemetrySettings())
require.NotNil(t, pprofExt)

require.NoError(t, pprofExt.Start(context.Background(), componenttest.NewNopHost()))
Expand Down Expand Up @@ -59,7 +61,9 @@ func TestPerformanceProfilerExtensionPortAlreadyInUse(t *testing.T) {
Endpoint: endpoint,
},
}
pprofExt := newServer(config, zap.NewNop())
tt, err := componenttest.SetupTelemetry(component.NewID("TestPprofExtension"))
require.NoError(t, err, "SetupTelemetry should succeed")
pprofExt := newServer(config, tt.TelemetrySettings())
require.NotNil(t, pprofExt)

require.Error(t, pprofExt.Start(context.Background(), componenttest.NewNopHost()))
Expand All @@ -72,7 +76,9 @@ func TestPerformanceProfilerMultipleStarts(t *testing.T) {
},
}

pprofExt := newServer(config, zap.NewNop())
tt, err := componenttest.SetupTelemetry(component.NewID("TestPprofExtension"))
require.NoError(t, err, "SetupTelemetry should succeed")
pprofExt := newServer(config, tt.TelemetrySettings())
require.NotNil(t, pprofExt)

require.NoError(t, pprofExt.Start(context.Background(), componenttest.NewNopHost()))
Expand All @@ -89,7 +95,9 @@ func TestPerformanceProfilerMultipleShutdowns(t *testing.T) {
},
}

pprofExt := newServer(config, zap.NewNop())
tt, err := componenttest.SetupTelemetry(component.NewID("TestPprofExtension"))
require.NoError(t, err, "SetupTelemetry should succeed")
pprofExt := newServer(config, tt.TelemetrySettings())
require.NotNil(t, pprofExt)

require.NoError(t, pprofExt.Start(context.Background(), componenttest.NewNopHost()))
Expand All @@ -103,8 +111,9 @@ func TestPerformanceProfilerShutdownWithoutStart(t *testing.T) {
Endpoint: testutil.GetAvailableLocalAddress(t),
},
}

pprofExt := newServer(config, zap.NewNop())
tt, err := componenttest.SetupTelemetry(component.NewID("TestPprofExtension"))
require.NoError(t, err, "SetupTelemetry should succeed")
pprofExt := newServer(config, tt.TelemetrySettings())
require.NotNil(t, pprofExt)

require.NoError(t, pprofExt.Shutdown(context.Background()))
Expand All @@ -124,8 +133,9 @@ func TestPerformanceProfilerLifecycleWithFile(t *testing.T) {
},
SaveToFile: tmpFile.Name(),
}

pprofExt := newServer(config, zap.NewNop())
tt, err := componenttest.SetupTelemetry(component.NewID("TestPprofExtension"))
require.NoError(t, err, "SetupTelemetry should succeed")
pprofExt := newServer(config, tt.TelemetrySettings())
require.NotNil(t, pprofExt)

require.NoError(t, pprofExt.Start(context.Background(), componenttest.NewNopHost()))
Expand Down

0 comments on commit 54e4531

Please sign in to comment.