harden switching extension components by renaming the prefix (#3015)

* harden switching extension components by renaming the prefix

* forgot to commit consistency change

* fix tests

* forgot to remove old lines in test file

* add tests

* update docs and improve loop on waiting for isClosed

Author: schmikei

extension/badgerextension/README.md

@@ -7,6 +7,7 @@ The Badger Extension provides persistent storage for OpenTelemetry Collector com
 | Field                                 | Type     | Default           | Required | Description                                                                                  |
 | ------------------------------------- | -------- | ----------------- | -------- | -------------------------------------------------------------------------------------------- |
 | directory.path                        | string   |                   | `true`   | Directory path where BadgerDB files will be stored. Each component gets a subdirectory.      |
+| directory.path_prefix                 | string   | `badger`          | `false`  | Optional prefix added to component directory names. Prevents naming collisions when multiple storage extensions share the same directory. Set to empty string to disable. |
 | sync_writes                           | bool     | `true`           | `false`  | Whether to sync writes to disk immediately. `false` survives process crashes via mmap.       |
 | memory.table_size                     | int64    | 67108864 (64MB)   | `false`  | Size of each memtable in bytes. Larger values improve write performance but use more memory. |
 | memory.block_cache_size               | int64    | 268435456 (256MB) | `false`  | Size of block cache in bytes. Larger values improve read performance but use more memory.    |
@@ -89,15 +90,35 @@ service:
 
 Each component that uses the badger extension gets an isolated database instance at:
 
+```
+{directory.path}/{path_prefix}_{kind}_{type}_{component_name}_{name}/
+```
+
+When `directory.path_prefix` is not set or is empty, the format becomes:
+
 ```
 {directory.path}/{kind}_{type}_{component_name}_{name}/
 ```
 
-For example, with `directory.path: $OIQ_OTEL_COLLECTOR_STORAGE`:
+For example, with `directory.path: $OIQ_OTEL_COLLECTOR_STORAGE` and default prefix:
 
 ```
-$OIQ_OTEL_COLLECTOR_STORAGE/processor_batch_default/
-$OIQ_OTEL_COLLECTOR_STORAGE/exporter_otlp_backup/
+$OIQ_OTEL_COLLECTOR_STORAGE/badger_processor_batch_default/
+$OIQ_OTEL_COLLECTOR_STORAGE/badger_exporter_otlp_backup/
+```
+
+When sharing a storage directory with other extensions, the prefix prevents naming collisions:
+
+```yaml
+extensions:
+  badger:
+    directory:
+      path: /var/lib/otelcol/storage
+      path_prefix: badger
+  pebble:
+    directory:
+      path: /var/lib/otelcol/storage
+      path_prefix: pebble
 ```
 
-This ensures components cannot interfere with each other's data.
+This ensures components cannot interfere with each other's data, even when using multiple storage backends in the same directory.

extension/badgerextension/config.go

@@ -32,8 +32,9 @@ type Config struct {
 
 // DirectoryConfig configures the file storage parameters for the badger storage extension
 type DirectoryConfig struct {
-	Path string   `mapstructure:"path"`
-	_    struct{} // prevent unkeyed literal initialization
+	Path       string   `mapstructure:"path"`
+	PathPrefix string   `mapstructure:"path_prefix"`
+	_          struct{} // prevent unkeyed literal initialization
 }
 
 // BlobGarbageCollectionConfig configures the blob garbage collection

extension/badgerextension/extension.go

@@ -16,6 +16,7 @@ package badgerextension
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"path/filepath"
 	"strings"
@@ -75,6 +76,10 @@ func (b *badgerExtension) GetClient(_ context.Context, kind component.Kind, ent
 	}
 	fullName = strings.ReplaceAll(fullName, " ", "")
 
+	if b.cfg.Directory.PathPrefix != "" {
+		fullName = fmt.Sprintf("%s_%s", b.cfg.Directory.PathPrefix, fullName)
+	}
+
 	if b.clients != nil {
 		b.clientsMutex.RLock()
 		client, ok := b.clients[fullName]
@@ -185,13 +190,21 @@ func (b *badgerExtension) monitor(ctx context.Context) {
 		case <-ctx.Done():
 			return
 		case <-ticker.C:
-			b.mb.ExtensionStorageClientCount.Record(ctx, int64(len(b.clients)))
 			b.clientsMutex.RLock()
+			clientCount := len(b.clients)
+			// make a shallow copy of the clients under lock so we can process them outside of the lock
+			clients := make([]client.Client, 0, clientCount)
 			for _, c := range b.clients {
+				clients = append(clients, c)
+			}
+			b.clientsMutex.RUnlock()
+
+			b.mb.ExtensionStorageClientCount.Record(ctx, int64(clientCount))
+			// process clients outside of lock to avoid blocking other operations
+			for _, c := range clients {
 				diskUsage, err := c.GetDiskUsage()
 				if err != nil {
 					b.logger.Error("failed to get disk usage", zap.Error(err))
-					b.clientsMutex.RUnlock()
 					continue
 				}
 
@@ -213,7 +226,6 @@ func (b *badgerExtension) monitor(ctx context.Context) {
 					b.otelAttrs,
 					metric.WithAttributes(attribute.String(internal.OperationTypeAttribute, internal.OperationTypeDelete)))
 			}
-			b.clientsMutex.RUnlock()
 		}
 	}
 }
@@ -248,12 +260,13 @@ func (b *badgerExtension) Shutdown(ctx context.Context) error {
 	b.clientsMutex.Lock()
 	defer b.clientsMutex.Unlock()
 
+	var shutdownErrors error
 	for _, c := range b.clients {
 		if err := c.Close(ctx); err != nil {
-			return fmt.Errorf("failed to close badger client: %w", err)
+			shutdownErrors = errors.Join(shutdownErrors, fmt.Errorf("failed to close badger client: %w", err))
 		}
 	}
-	return nil
+	return shutdownErrors
 }
 
 func kindString(k component.Kind) string {

extension/badgerextension/extension_test.go

@@ -468,6 +468,66 @@ func TestBadgerExtension_ClientOptions(t *testing.T) {
 	})
 }
 
+func TestBadgerExtension_PathPrefix(t *testing.T) {
+	t.Run("applies path prefix to client names", func(t *testing.T) {
+		logger := zap.NewNop()
+		cfg := &Config{
+			Directory: &DirectoryConfig{
+				Path:       t.TempDir(),
+				PathPrefix: "badger",
+			},
+		}
+
+		ext := newBadgerExtension(logger, cfg, componenttest.NewNopTelemetrySettings(), testID).(*badgerExtension)
+
+		client, err := ext.GetClient(
+			context.Background(),
+			component.KindExporter,
+			component.MustNewID("otlp"),
+			"logs",
+		)
+		require.NoError(t, err)
+		require.NotNil(t, client)
+
+		// Verify that the client is stored with the prefixed key
+		assert.Len(t, ext.clients, 1)
+		for key := range ext.clients {
+			assert.Contains(t, key, "badger_")
+		}
+
+		err = ext.Shutdown(context.Background())
+		require.NoError(t, err)
+	})
+
+	t.Run("empty path prefix works without prefix", func(t *testing.T) {
+		logger := zap.NewNop()
+		cfg := &Config{
+			Directory: &DirectoryConfig{
+				Path:       t.TempDir(),
+				PathPrefix: "",
+			},
+		}
+		require.NoError(t, cfg.Validate())
+		ext := newBadgerExtension(logger, cfg, componenttest.NewNopTelemetrySettings(), testID).(*badgerExtension)
+
+		client, err := ext.GetClient(
+			context.Background(),
+			component.KindExporter,
+			component.MustNewID("otlp"),
+			"logs",
+		)
+		require.NoError(t, err)
+		require.NotNil(t, client)
+
+		// With empty prefix, the key should not have underscore prefix pattern
+		assert.Len(t, ext.clients, 1)
+
+		// Cleanup
+		err = ext.Shutdown(context.Background())
+		require.NoError(t, err)
+	})
+}
+
 func TestKindString(t *testing.T) {
 	tests := []struct {
 		name     string

extension/badgerextension/factory.go

@@ -39,6 +39,9 @@ func NewFactory() extension.Factory {
 func createDefaultConfig() component.Config {
 	return &Config{
 		SyncWrites: true,
+		Directory: &DirectoryConfig{
+			PathPrefix: "badger",
+		},
 		Memory: &MemoryConfig{
 			TableSize:      64 * 1024 * 1024,  // Default: 64MB
 			BlockCacheSize: 256 * 1024 * 1024, // Default: 256MB

extension/badgerextension/factory_test.go

@@ -33,7 +33,9 @@ func TestCreateDefaultConfig(t *testing.T) {
 
 	// Verify default values
 	require.True(t, badgerCfg.SyncWrites)
-	require.Nil(t, badgerCfg.Directory)
+	require.NotNil(t, badgerCfg.Directory)
+	require.Empty(t, badgerCfg.Directory.Path)
+	require.Equal(t, "badger", badgerCfg.Directory.PathPrefix)
 
 	require.NotNil(t, badgerCfg.Memory)
 	require.Equal(t, int64(64*1024*1024), badgerCfg.Memory.TableSize)

extension/badgerextension/internal/client/client.go

@@ -20,6 +20,7 @@ import (
 	"errors"
 	"fmt"
 	"sync/atomic"
+	"time"
 
 	"github.com/dgraph-io/badger/v4"
 	"go.opentelemetry.io/collector/extension/xextension/storage"
@@ -184,8 +185,35 @@ func (c *client) Batch(ctx context.Context, ops ...*storage.Operation) error {
 	return nil
 }
 
-func (c *client) Close(_ context.Context) error {
-	return c.db.Close()
+func (c *client) Close(ctx context.Context) error {
+	err := c.db.Close()
+	if err != nil {
+		return fmt.Errorf("failed to close badger client: %w", err)
+	}
+
+	isFullyClosed := make(chan struct{})
+	go func() {
+		ticker := time.NewTicker(100 * time.Millisecond)
+		defer ticker.Stop()
+		defer close(isFullyClosed)
+		for {
+			select {
+			case <-ctx.Done():
+				return
+			case <-ticker.C:
+				if c.db.IsClosed() {
+					return
+				}
+			}
+		}
+	}()
+
+	select {
+	case <-ctx.Done():
+		return ctx.Err()
+	case <-isFullyClosed:
+		return nil
+	}
 }
 
 func (c *client) RunValueLogGC(discardRatio float64) error {

extension/pebbleextension/README.md

@@ -4,11 +4,12 @@ The Pebble Extension provides persistent storage for OpenTelemetry Collector com
 
 ## Configuration
 
-| Field              | Type     | Default | Required | Description                                                                                       |
-| ------------------ | -------- | ------- | -------- | ------------------------------------------------------------------------------------------------- |
-| directory.path     | string   |         | `true`   | Directory path where Pebble database files will be stored. Each component gets a subdirectory.    |
-| cache.size         | int64    | `0`     | `false`  | Size in bytes of the block cache. When 0, uses Pebble's default cache behavior. Larger values improve read performance at the cost of memory usage. |
-| sync               | bool     | `false` | `false`  | Whether to sync writes to disk immediately. `false` provides better performance while still being durable. |
+| Field              | Type     | Default   | Required | Description                                                                                       |
+| ------------------ | -------- | --------- | -------- | ------------------------------------------------------------------------------------------------- |
+| directory.path     | string   |           | `true`   | Directory path where Pebble database files will be stored. Each component gets a subdirectory.    |
+| directory.path_prefix | string | `pebble` | `false`  | Optional prefix added to component directory names. Prevents naming collisions when multiple storage extensions share the same directory. Set to empty string to disable. |
+| cache.size         | int64    | `0`       | `false`  | Size in bytes of the block cache. When 0, uses Pebble's default cache behavior. Larger values improve read performance at the cost of memory usage. |
+| sync               | bool     | `true`    | `false`  | Whether to sync writes to disk immediately. `true` provides safer durability guarantees. |
 
 ## Example Configuration
 
@@ -71,15 +72,35 @@ service:
 
 Each component that uses the pebble extension gets an isolated database instance at:
 
+```
+{directory.path}/{path_prefix}_{kind}_{type}_{component_name}_{name}/
+```
+
+When `directory.path_prefix` is not set or is empty, the format becomes:
+
 ```
 {directory.path}/{kind}_{type}_{component_name}_{name}/
 ```
 
-For example, with `directory.path: $OIQ_OTEL_COLLECTOR_STORAGE`:
+For example, with `directory.path: $OIQ_OTEL_COLLECTOR_STORAGE` and default prefix:
 
 ```
-$OIQ_OTEL_COLLECTOR_STORAGE/processor_batch_default/
-$OIQ_OTEL_COLLECTOR_STORAGE/exporter_otlp_backup/
+$OIQ_OTEL_COLLECTOR_STORAGE/pebble_processor_batch_default/
+$OIQ_OTEL_COLLECTOR_STORAGE/pebble_exporter_otlp_backup/
+```
+
+When sharing a storage directory with other extensions, the prefix prevents naming collisions:
+
+```yaml
+extensions:
+  badger:
+    directory:
+      path: /var/lib/otelcol/storage
+      path_prefix: badger
+  pebble:
+    directory:
+      path: /var/lib/otelcol/storage
+      path_prefix: pebble
 ```
 
-This ensures components cannot interfere with each other's data.
+This ensures components cannot interfere with each other's data, even when using multiple storage backends in the same directory.

extension/pebbleextension/config.go

@@ -26,7 +26,8 @@ type Config struct {
 
 // DirectoryConfig is the configuration for the directory
 type DirectoryConfig struct {
-	Path string `mapstructure:"path"`
+	Path       string `mapstructure:"path"`
+	PathPrefix string `mapstructure:"path_prefix"`
 }
 
 // CacheConfig is the configuration for the cache

extension/pebbleextension/extension.go

@@ -56,6 +56,10 @@ func (p *pebbleExtension) GetClient(_ context.Context, kind component.Kind, ent
 	}
 	fullName = strings.ReplaceAll(fullName, " ", "")
 
+	if p.cfg.Directory.PathPrefix != "" {
+		fullName = fmt.Sprintf("%s_%s", p.cfg.Directory.PathPrefix, fullName)
+	}
+
 	if p.clients != nil {
 		p.clientsMutex.RLock()
 		client, ok := p.clients[fullName]