Verify extension lifecycle

component/componenttest/assert_no_error_host.go

@@ -0,0 +1,55 @@
+// Copyright The OpenTelemetry Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//       http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package componenttest
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+
+	"go.opentelemetry.io/collector/component"
+	"go.opentelemetry.io/collector/config/configmodels"
+)
+
+// assertNoErrorHost implements a component.Host that asserts that
+// there were no errors.
+type assertNoErrorHost struct {
+	t *testing.T
+}
+
+var _ component.Host = (*assertNoErrorHost)(nil)
+
+// NewAssertNoError returns a new instance of assertNoErrorHost.
+func NewAssertNoError(t *testing.T) component.Host {
+	return &assertNoErrorHost{
+		t: t,
+	}
+}
+
+func (aneh *assertNoErrorHost) ReportFatalError(err error) {
+	assert.NoError(aneh.t, err)
+}
+
+func (aneh *assertNoErrorHost) GetFactory(_ component.Kind, _ configmodels.Type) component.Factory {
+	return nil
+}
+
+func (aneh *assertNoErrorHost) GetExtensions() map[configmodels.NamedEntity]component.Extension {
+	return nil
+}
+
+func (aneh *assertNoErrorHost) GetExporters() map[configmodels.DataType]map[configmodels.NamedEntity]component.Exporter {
+	return nil
+}

component/componenttest/lifecycle_verifier.go

@@ -0,0 +1,79 @@
+// Copyright The OpenTelemetry Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//       http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package componenttest
+
+import (
+	"context"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"go.uber.org/zap"
+
+	"go.opentelemetry.io/collector/component"
+	"go.opentelemetry.io/collector/config/configmodels"
+)
+
+// GetExtensionConfigFn is used customize the configuration passed to the verification.
+// This is used to change ports or provide values required but not provided by the
+// default configuration.
+type GetExtensionConfigFn func() configmodels.Extension
+
+// VerityExtensionLifecycle is used to test if an extension type can handle the typical
+// lifecycle of a component. The getConfigFn parameter only need to be specified if
+// the test can't be done with the default configuration for the component.
+func VerityExtensionLifecycle(t *testing.T, factory component.ExtensionFactory, getConfigFn GetExtensionConfigFn) {
+	ctx := context.Background()
+	host := NewAssertNoError(t)
+	extCreateParams := component.ExtensionCreateParams{
+		Logger:               zap.NewNop(),
+		ApplicationStartInfo: component.DefaultApplicationStartInfo(),
+	}
+
+	var activeExt, builtExt component.Extension
+	defer func() {
+		// If the shutdown happens here there were already some errors on the test.
+		// Ignore errors on this attempt to clean-up.
+		if activeExt != nil {
+			_ = activeExt.Shutdown(ctx)
+		}
+		if builtExt != nil {
+			_ = builtExt.Shutdown(ctx)
+		}
+	}()
+
+	if getConfigFn == nil {
+		getConfigFn = factory.CreateDefaultConfig
+	}
+
+	for i := 0; i < 3; i++ {
+		var err error
+		builtExt, err = factory.CreateExtension(ctx, extCreateParams, getConfigFn())
+		require.NoError(t, err, "Extension type: %s", factory.Type())
+
+		if activeExt != nil {
+			assert.NoError(t, activeExt.Shutdown(ctx), "Extension type: %s", factory.Type())
+			activeExt = nil
+		}
+
+		require.NoError(t, builtExt.Start(ctx, host), "Extension type: %s", factory.Type())
+		activeExt = builtExt
+		builtExt = nil
+
+		// The component may start go routines, give them a chance to run.
+		time.Sleep(100 * time.Millisecond)
+	}
+}

extension/healthcheckextension/factory.go

@@ -16,8 +16,6 @@ package healthcheckextension
 
 import (
 	"context"
-	"errors"
-	"sync/atomic"
 
 	"go.opentelemetry.io/collector/component"
 	"go.opentelemetry.io/collector/config/configmodels"
@@ -50,24 +48,5 @@ func createDefaultConfig() configmodels.Extension {
 func createExtension(_ context.Context, params component.ExtensionCreateParams, cfg configmodels.Extension) (component.Extension, error) {
 	config := cfg.(*Config)
 
-	// The runtime settings are global to the application, so while in principle it
-	// is possible to have more than one instance, running multiple does not bring
-	// any value to the service.
-	// In order to avoid this issue we will allow the creation of a single
-	// instance once per process while keeping the private function that allow
-	// the creation of multiple instances for unit tests. Summary: only a single
-	// instance can be created via the factory.
-	if !atomic.CompareAndSwapInt32(&instanceState, instanceNotCreated, instanceCreated) {
-		return nil, errors.New("only a single health check extension instance can be created per process")
-	}
-
 	return newServer(*config, params.Logger), nil
 }
-
-// See comment in createExtension how these are used.
-var instanceState int32
-
-const (
-	instanceNotCreated int32 = 0
-	instanceCreated    int32 = 1
-)

extension/healthcheckextension/factory_test.go

@@ -16,7 +16,6 @@ package healthcheckextension
 
 import (
 	"context"
-	"sync/atomic"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -44,9 +43,6 @@ func TestFactory_CreateDefaultConfig(t *testing.T) {
 	ext, err := createExtension(context.Background(), component.ExtensionCreateParams{Logger: zap.NewNop()}, cfg)
 	require.NoError(t, err)
 	require.NotNil(t, ext)
-
-	// Restore instance tracking from factory, for other tests.
-	atomic.StoreInt32(&instanceState, instanceNotCreated)
 }
 
 func TestFactory_CreateExtension(t *testing.T) {
@@ -56,23 +52,4 @@ func TestFactory_CreateExtension(t *testing.T) {
 	ext, err := createExtension(context.Background(), component.ExtensionCreateParams{Logger: zap.NewNop()}, cfg)
 	require.NoError(t, err)
 	require.NotNil(t, ext)
-
-	// Restore instance tracking from factory, for other tests.
-	atomic.StoreInt32(&instanceState, instanceNotCreated)
-}
-
-func TestFactory_CreateExtensionOnlyOnce(t *testing.T) {
-	cfg := createDefaultConfig().(*Config)
-	cfg.Port = testutil.GetAvailablePort(t)
-
-	ext, err := createExtension(context.Background(), component.ExtensionCreateParams{Logger: zap.NewNop()}, cfg)
-	require.NoError(t, err)
-	require.NotNil(t, ext)
-
-	ext1, err := createExtension(context.Background(), component.ExtensionCreateParams{Logger: zap.NewNop()}, cfg)
-	require.Error(t, err)
-	require.Nil(t, ext1)
-
-	// Restore instance tracking from factory, for other tests.
-	atomic.StoreInt32(&instanceState, instanceNotCreated)
 }

extension/healthcheckextension/healthcheckextension.go

@@ -43,8 +43,7 @@ func (hc *healthCheckExtension) Start(_ context.Context, host component.Host) er
 	portStr := ":" + strconv.Itoa(int(hc.config.Port))
 	ln, err := net.Listen("tcp", portStr)
 	if err != nil {
-		host.ReportFatalError(err)
-		return nil
+		return err
 	}
 
 	// Mount HC handler

extension/healthcheckextension/healthcheckextension_test.go

@@ -21,7 +21,6 @@ import (
 	"runtime"
 	"strconv"
 	"testing"
-	"time"
 
 	"github.com/stretchr/testify/require"
 	"go.uber.org/zap"
@@ -86,14 +85,8 @@ func TestHealthCheckExtensionPortAlreadyInUse(t *testing.T) {
 	hcExt := newServer(config, zap.NewNop())
 	require.NotNil(t, hcExt)
 
-	// Health check will report port already in use in a goroutine, use the error waiting
-	// host to get it.
-	mh := componenttest.NewErrorWaitingHost()
-	require.NoError(t, hcExt.Start(context.Background(), mh))
-
-	receivedError, receivedErr := mh.WaitForFatalError(500 * time.Millisecond)
-	require.True(t, receivedError)
-	require.Error(t, receivedErr)
+	mh := componenttest.NewAssertNoError(t)
+	require.Error(t, hcExt.Start(context.Background(), mh))
 }
 
 func TestHealthCheckMultipleStarts(t *testing.T) {
@@ -104,17 +97,11 @@ func TestHealthCheckMultipleStarts(t *testing.T) {
 	hcExt := newServer(config, zap.NewNop())
 	require.NotNil(t, hcExt)
 
-	mh := componenttest.NewErrorWaitingHost()
+	mh := componenttest.NewAssertNoError(t)
 	require.NoError(t, hcExt.Start(context.Background(), mh))
 	defer hcExt.Shutdown(context.Background())
 
-	// Health check will report already in use in a goroutine, use the error waiting
-	// host to get it.
-	require.NoError(t, hcExt.Start(context.Background(), mh))
-
-	receivedError, receivedErr := mh.WaitForFatalError(500 * time.Millisecond)
-	require.True(t, receivedError)
-	require.Error(t, receivedErr)
+	require.Error(t, hcExt.Start(context.Background(), mh))
 }
 
 func TestHealthCheckMultipleShutdowns(t *testing.T) {

extension/pprofextension/factory.go

@@ -17,7 +17,6 @@ package pprofextension
 import (
 	"context"
 	"errors"
-	"sync/atomic"
 
 	"go.opentelemetry.io/collector/component"
 	"go.opentelemetry.io/collector/config/configmodels"
@@ -53,25 +52,5 @@ func createExtension(_ context.Context, params component.ExtensionCreateParams,
 		return nil, errors.New("\"endpoint\" is required when using the \"pprof\" extension")
 	}
 
-	// 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
-	// this issue we will allow the creation of a single instance once per process
-	// while keeping the private function that allow the creation of multiple
-	// instances for unit tests. Summary: only a single instance can be created
-	// via the factory.
-	// TODO: Move this as an option to extensionhelper.
-	if !atomic.CompareAndSwapInt32(&instanceState, instanceNotCreated, instanceCreated) {
-		return nil, errors.New("only a single pprof extension instance can be created per process")
-	}
-
 	return newServer(*config, params.Logger), nil
 }
-
-// See comment in createExtension how these are used.
-var instanceState int32
-
-const (
-	instanceNotCreated int32 = 0
-	instanceCreated    int32 = 1
-)

extension/pprofextension/factory_test.go

@@ -16,7 +16,6 @@ package pprofextension
 
 import (
 	"context"
-	"sync/atomic"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -44,9 +43,6 @@ func TestFactory_CreateDefaultConfig(t *testing.T) {
 	ext, err := createExtension(context.Background(), component.ExtensionCreateParams{Logger: zap.NewNop()}, cfg)
 	require.NoError(t, err)
 	require.NotNil(t, ext)
-
-	// Restore instance tracking from factory, for other tests.
-	atomic.StoreInt32(&instanceState, instanceNotCreated)
 }
 
 func TestFactory_CreateExtension(t *testing.T) {
@@ -56,23 +52,4 @@ func TestFactory_CreateExtension(t *testing.T) {
 	ext, err := createExtension(context.Background(), component.ExtensionCreateParams{Logger: zap.NewNop()}, cfg)
 	require.NoError(t, err)
 	require.NotNil(t, ext)
-
-	// Restore instance tracking from factory, for other tests.
-	atomic.StoreInt32(&instanceState, instanceNotCreated)
-}
-
-func TestFactory_CreateExtensionOnlyOnce(t *testing.T) {
-	cfg := createDefaultConfig().(*Config)
-	cfg.Endpoint = testutil.GetAvailableLocalAddress(t)
-
-	ext, err := createExtension(context.Background(), component.ExtensionCreateParams{Logger: zap.NewNop()}, cfg)
-	require.NoError(t, err)
-	require.NotNil(t, ext)
-
-	ext1, err := createExtension(context.Background(), component.ExtensionCreateParams{Logger: zap.NewNop()}, cfg)
-	require.Error(t, err)
-	require.Nil(t, ext1)
-
-	// Restore instance tracking from factory, for other tests.
-	atomic.StoreInt32(&instanceState, instanceNotCreated)
 }