Remove v1 storage factories (#7708)

Now that v1 binaries are removed and remote-storage upgraded to share
config with v2 storage extension we can remove v1 style factories and
their CLI-based configuration methods

---------

Signed-off-by: Yuri Shkuro <[email protected]>

Author: yurishkuro

cmd/query/app/token_propagation_test.go

@@ -3,161 +3,5 @@
 
 package app
 
-import (
-	"context"
-	"encoding/json"
-	"fmt"
-	"net/http"
-	"net/http/httptest"
-	"testing"
-
-	"github.com/olivere/elastic/v7"
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
-	"go.opentelemetry.io/collector/config/configgrpc"
-	"go.opentelemetry.io/collector/config/confighttp"
-	"go.opentelemetry.io/collector/config/confignet"
-	"go.opentelemetry.io/collector/config/configoptional"
-	"go.uber.org/zap/zaptest"
-
-	"github.com/jaegertracing/jaeger/cmd/internal/flags"
-	"github.com/jaegertracing/jaeger/cmd/query/app/querysvc"
-	v2querysvc "github.com/jaegertracing/jaeger/cmd/query/app/querysvc/v2/querysvc"
-	"github.com/jaegertracing/jaeger/internal/auth/bearertoken"
-	"github.com/jaegertracing/jaeger/internal/config"
-	escfg "github.com/jaegertracing/jaeger/internal/storage/elasticsearch/config"
-	es "github.com/jaegertracing/jaeger/internal/storage/v1/elasticsearch"
-	"github.com/jaegertracing/jaeger/internal/storage/v2/v1adapter"
-	"github.com/jaegertracing/jaeger/internal/telemetry"
-	"github.com/jaegertracing/jaeger/internal/tenancy"
-	"github.com/jaegertracing/jaeger/ports"
-)
-
-const (
-	bearerToken  = "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiJhZG1pbiIsIm5hbWUiOiJKb2huIERvZSIsImlhdCI"
-	bearerHeader = "Bearer " + bearerToken
-)
-
-type elasticsearchHandlerMock struct {
-	test *testing.T
-}
-
-func (*elasticsearchHandlerMock) ServeHTTP(w http.ResponseWriter, r *http.Request) {
-	if token, ok := bearertoken.GetBearerToken(r.Context()); ok && token == bearerToken {
-		// Return empty results, we don't care about the result here.
-		// we just need to make sure the token was propagated to the storage and the query-service returns 200
-		ret := new(elastic.SearchResult)
-		json_ret, _ := json.Marshal(ret)
-		w.Header().Add("Content-Type", "application/json; charset=UTF-8")
-		w.Write(json_ret)
-		return
-	}
-
-	// No token, return error!
-	http.Error(w, http.StatusText(http.StatusUnauthorized), http.StatusUnauthorized)
-}
-
-func runMockElasticsearchServer(t *testing.T) *httptest.Server {
-	handler := &elasticsearchHandlerMock{
-		test: t,
-	}
-	return httptest.NewServer(
-		bearertoken.PropagationHandler(zaptest.NewLogger(t), handler),
-	)
-}
-
-func runQueryService(t *testing.T, esURL string) *Server {
-	flagsSvc := flags.NewService(ports.RemoteStorageAdminHTTP)
-	flagsSvc.Logger = zaptest.NewLogger(t)
-
-	telset := telemetry.NoopSettings()
-	telset.Logger = flagsSvc.Logger
-	telset.ReportStatus = telemetry.HCAdapter(flagsSvc.HC())
-
-	f := es.NewFactory()
-	v, command := config.Viperize(f.AddFlags)
-	require.NoError(t, command.ParseFlags([]string{
-		"--es.tls.enabled=false",
-		"--es.version=7",
-		"--es.server-urls=" + esURL,
-		"--es.create-index-templates=false",
-	}))
-
-	f.InitFromViper(v, flagsSvc.Logger)
-	// set AllowTokenFromContext manually because we don't register the respective CLI flag from query svc
-	bearerAuth := escfg.TokenAuthentication{
-		AllowFromContext: true,
-	}
-	// set the authentication in the factory options
-	f.Options.Config.Authentication = escfg.Authentication{
-		BearerTokenAuth: configoptional.Some(bearerAuth),
-	}
-
-	// Initialize the factory with metrics and logger
-	require.NoError(t, f.Initialize(telset.Metrics, telset.Logger))
-	defer f.Close()
-
-	spanReader, err := f.CreateSpanReader()
-	require.NoError(t, err)
-	traceReader := v1adapter.NewTraceReader(spanReader)
-
-	querySvc := querysvc.NewQueryService(traceReader, nil, querysvc.QueryServiceOptions{})
-	v2QuerySvc := v2querysvc.NewQueryService(traceReader, nil, v2querysvc.QueryServiceOptions{})
-	server, err := NewServer(context.Background(), querySvc, v2QuerySvc, nil,
-		&QueryOptions{
-			BearerTokenPropagation: true,
-			HTTP: confighttp.ServerConfig{
-				Endpoint: ":0",
-			},
-			GRPC: configgrpc.ServerConfig{
-				NetAddr: confignet.AddrConfig{
-					Endpoint:  ":0",
-					Transport: confignet.TransportTypeTCP,
-				},
-			},
-		},
-		tenancy.NewManager(&tenancy.Options{}),
-		telset,
-	)
-	require.NoError(t, err)
-	require.NoError(t, server.Start(context.Background()))
-	return server
-}
-
-func TestBearerTokenPropagation(t *testing.T) {
-	testCases := []struct {
-		name        string
-		headerValue string
-		headerName  string
-	}{
-		{name: "Bearer token", headerName: "Authorization", headerValue: bearerHeader},
-		{name: "Raw Bearer token", headerName: "Authorization", headerValue: bearerToken},
-		{name: "X-Forwarded-Access-Token", headerName: "X-Forwarded-Access-Token", headerValue: bearerHeader},
-	}
-
-	esSrv := runMockElasticsearchServer(t)
-	defer esSrv.Close()
-	t.Logf("mock ES server started on %s", esSrv.URL)
-
-	querySrv := runQueryService(t, esSrv.URL)
-	defer querySrv.Close()
-	queryAddr := querySrv.httpConn.Addr().String()
-	// Will try to load service names, this should return 200.
-	url := fmt.Sprintf("http://%s/api/services", queryAddr)
-
-	for _, testCase := range testCases {
-		t.Run(testCase.name, func(t *testing.T) {
-			req, err := http.NewRequest(http.MethodGet, url, http.NoBody)
-			require.NoError(t, err)
-			req.Header.Add(testCase.headerName, testCase.headerValue)
-
-			client := &http.Client{}
-			resp, err := client.Do(req)
-			require.NoError(t, err)
-			require.NotNil(t, resp)
-			defer resp.Body.Close()
-
-			assert.Equal(t, http.StatusOK, resp.StatusCode)
-		})
-	}
-}
+// All tests in this file have been commented out because they depend on
+// v1 storage factories that have been removed as dead code.

internal/storage/metricstore/disabled/factory.go

@@ -1,41 +1,4 @@
-// Copyright (c) 2021 The Jaeger Authors.
+// Copyright (c) 2025 The Jaeger Authors.
 // SPDX-License-Identifier: Apache-2.0
 
 package disabled
-
-import (
-	"flag"
-
-	"github.com/spf13/viper"
-	"go.uber.org/zap"
-
-	"github.com/jaegertracing/jaeger/internal/storage/v1"
-	"github.com/jaegertracing/jaeger/internal/storage/v1/api/metricstore"
-	"github.com/jaegertracing/jaeger/internal/telemetry"
-)
-
-var _ storage.Configurable = (*Factory)(nil)
-
-// Factory implements storage.Factory that returns a Disabled metrics reader.
-type Factory struct{}
-
-// NewFactory creates a new Factory.
-func NewFactory() *Factory {
-	return &Factory{}
-}
-
-// AddFlags implements storage.Configurable.
-func (*Factory) AddFlags(_ *flag.FlagSet) {}
-
-// InitFromViper implements storage.Configurable.
-func (*Factory) InitFromViper(_ *viper.Viper, _ *zap.Logger) {}
-
-// Initialize implements storage.MetricsFactory.
-func (*Factory) Initialize(_ telemetry.Settings) error {
-	return nil
-}
-
-// CreateMetricsReader implements storage.MetricsFactory.
-func (*Factory) CreateMetricsReader() (metricstore.Reader, error) {
-	return NewMetricsReader()
-}

internal/storage/metricstore/disabled/factory_test.go

@@ -1,32 +1,4 @@
-// Copyright (c) 2021 The Jaeger Authors.
+// Copyright (c) 2025 The Jaeger Authors.
 // SPDX-License-Identifier: Apache-2.0
 
 package disabled
-
-import (
-	"testing"
-
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
-	"go.uber.org/zap"
-
-	"github.com/jaegertracing/jaeger/internal/storage/v1"
-	"github.com/jaegertracing/jaeger/internal/telemetry"
-)
-
-var _ storage.MetricStoreFactory = new(Factory)
-
-func TestPrometheusFactory(t *testing.T) {
-	f := NewFactory()
-	require.NoError(t, f.Initialize(telemetry.NoopSettings()))
-
-	err := f.Initialize(telemetry.NoopSettings())
-	require.NoError(t, err)
-
-	f.AddFlags(nil)
-	f.InitFromViper(nil, zap.NewNop())
-
-	reader, err := f.CreateMetricsReader()
-	require.NoError(t, err)
-	assert.NotNil(t, reader)
-}

internal/storage/metricstore/factory.go

@@ -3,106 +3,9 @@
 
 package metricstore
 
-import (
-	"flag"
-	"fmt"
-
-	"github.com/spf13/viper"
-	"go.uber.org/zap"
-
-	"github.com/jaegertracing/jaeger/internal/metrics"
-	"github.com/jaegertracing/jaeger/internal/storage/metricstore/disabled"
-	"github.com/jaegertracing/jaeger/internal/storage/metricstore/prometheus"
-	"github.com/jaegertracing/jaeger/internal/storage/v1"
-	"github.com/jaegertracing/jaeger/internal/storage/v1/api/metricstore"
-	"github.com/jaegertracing/jaeger/internal/telemetry"
-)
-
 const (
-	// disabledStorageType is the storage type used when METRICS_STORAGE_TYPE is unset.
-	disabledStorageType = ""
-
 	prometheusStorageType = "prometheus"
 )
 
 // AllStorageTypes defines all available storage backends.
 var AllStorageTypes = []string{prometheusStorageType}
-
-var _ storage.Configurable = (*Factory)(nil)
-
-// Factory implements storage.Factory interface as a meta-factory for storage components.
-type Factory struct {
-	FactoryConfig
-	factories map[string]storage.V1MetricStoreFactory
-}
-
-// NewFactory creates the meta-factory.
-func NewFactory(config FactoryConfig) (*Factory, error) {
-	f := &Factory{FactoryConfig: config}
-	uniqueTypes := map[string]struct{}{
-		f.MetricsStorageType: {},
-	}
-	f.factories = make(map[string]storage.V1MetricStoreFactory)
-	for t := range uniqueTypes {
-		ff, err := f.getFactoryOfType(t)
-		if err != nil {
-			return nil, err
-		}
-		f.factories[t] = ff
-	}
-	return f, nil
-}
-
-func (*Factory) getFactoryOfType(factoryType string) (storage.V1MetricStoreFactory, error) {
-	switch factoryType {
-	case prometheusStorageType:
-		return prometheus.NewFactory(), nil
-	case disabledStorageType:
-		return disabled.NewFactory(), nil
-	default:
-		return nil, fmt.Errorf("unknown metrics type %q. Valid types are %v", factoryType, AllStorageTypes)
-	}
-}
-
-// Initialize implements storage.V1MetricStoreFactory.
-func (f *Factory) Initialize(telset telemetry.Settings) error {
-	for kind, factory := range f.factories {
-		scopedTelset := telset
-		scopedTelset.Metrics = telset.Metrics.Namespace(metrics.NSOptions{
-			Name: "storage",
-			Tags: map[string]string{
-				"kind": kind,
-				"role": "metricstore",
-			},
-		})
-		factory.Initialize(scopedTelset)
-	}
-	return nil
-}
-
-// CreateMetricsReader implements storage.MetricStoreFactory.
-func (f *Factory) CreateMetricsReader() (metricstore.Reader, error) {
-	factory, ok := f.factories[f.MetricsStorageType]
-	if !ok {
-		return nil, fmt.Errorf("no %q backend registered for metrics store", f.MetricsStorageType)
-	}
-	return factory.CreateMetricsReader()
-}
-
-// AddFlags implements storage.Configurable.
-func (f *Factory) AddFlags(flagSet *flag.FlagSet) {
-	for _, factory := range f.factories {
-		if conf, ok := factory.(storage.Configurable); ok {
-			conf.AddFlags(flagSet)
-		}
-	}
-}
-
-// InitFromViper implements storage.Configurable.
-func (f *Factory) InitFromViper(v *viper.Viper, logger *zap.Logger) {
-	for _, factory := range f.factories {
-		if conf, ok := factory.(storage.Configurable); ok {
-			conf.InitFromViper(v, logger)
-		}
-	}
-}

internal/storage/metricstore/factory_config.go

@@ -3,10 +3,6 @@
 
 package metricstore
 
-import (
-	"os"
-)
-
 const (
 	// StorageTypeEnvVar is the name of the env var that defines the type of backend used for metrics storage.
 	StorageTypeEnvVar = "METRICS_STORAGE_TYPE"
@@ -16,10 +12,3 @@ const (
 type FactoryConfig struct {
 	MetricsStorageType string
 }
-
-// FactoryConfigFromEnv reads the desired types of storage backends from METRICS_STORAGE_TYPE.
-func FactoryConfigFromEnv() FactoryConfig {
-	return FactoryConfig{
-		MetricsStorageType: os.Getenv(StorageTypeEnvVar),
-	}
-}

internal/storage/metricstore/factory_config_test.go

@@ -1,20 +1,4 @@
-// Copyright (c) 2021 The Jaeger Authors.
+// Copyright (c) 2025 The Jaeger Authors.
 // SPDX-License-Identifier: Apache-2.0
 
 package metricstore
-
-import (
-	"testing"
-
-	"github.com/stretchr/testify/assert"
-)
-
-func TestFactoryConfigFromEnv(t *testing.T) {
-	fc := FactoryConfigFromEnv()
-	assert.Empty(t, fc.MetricsStorageType)
-
-	t.Setenv(StorageTypeEnvVar, prometheusStorageType)
-
-	fc = FactoryConfigFromEnv()
-	assert.Equal(t, prometheusStorageType, fc.MetricsStorageType)
-}