From 541652ee43ece403b4e420a000059134b43db8b5 Mon Sep 17 00:00:00 2001 From: Mahad Zaryab Date: Tue, 17 Sep 2024 19:35:11 -0600 Subject: [PATCH 01/18] Make Configuration For Query Service Logical Signed-off-by: Mahad Zaryab --- .../internal/extension/jaegerquery/config.go | 20 +++++++++++++------ .../extension/jaegerquery/config_test.go | 4 +++- 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/cmd/jaeger/internal/extension/jaegerquery/config.go b/cmd/jaeger/internal/extension/jaegerquery/config.go index 9abaf0c1a64..b74d2e74018 100644 --- a/cmd/jaeger/internal/extension/jaegerquery/config.go +++ b/cmd/jaeger/internal/extension/jaegerquery/config.go @@ -18,12 +18,20 @@ var _ component.ConfigValidator = (*Config)(nil) // Config represents the configuration for jaeger-query, type Config struct { queryApp.QueryOptionsBase `mapstructure:",squash"` - TraceStoragePrimary string `valid:"required" mapstructure:"trace_storage"` - TraceStorageArchive string `valid:"optional" mapstructure:"trace_storage_archive"` - MetricStorage string `valid:"optional" mapstructure:"metric_storage"` - HTTP confighttp.ServerConfig `mapstructure:",squash"` - GRPC configgrpc.ServerConfig `mapstructure:",squash"` - Tenancy tenancy.Options `mapstructure:"multi_tenancy"` + Connection Connection `mapstructure:"connection"` + Storage Storage `mapstructure:"storage"` +} + +type Connection struct { + Tenancy tenancy.Options `mapstructure:"multi_tenancy"` + HTTP confighttp.ServerConfig `mapstructure:"http"` + GRPC configgrpc.ServerConfig `mapstructure:"grpc"` +} + +type Storage struct { + TracePrimary string `mapstructure:"trace" valid:"required" ` + TraceArchive string `mapstructure:"trace_archive" valid:"optional"` + Metric string `mapstructure:"metric" valid:"optional"` } func (cfg *Config) Validate() error { diff --git a/cmd/jaeger/internal/extension/jaegerquery/config_test.go b/cmd/jaeger/internal/extension/jaegerquery/config_test.go index 94911971e32..9cc6e85f5af 100644 --- a/cmd/jaeger/internal/extension/jaegerquery/config_test.go +++ b/cmd/jaeger/internal/extension/jaegerquery/config_test.go @@ -24,7 +24,9 @@ func Test_Validate(t *testing.T) { { name: "Non empty-config", config: &Config{ - TraceStoragePrimary: "some-storage", + Storage: Storage{ + TracePrimary: "some-storage", + }, }, expectedErr: "", }, From cd3e9528d1c28ff39ed93e33678ef1f2765c35b9 Mon Sep 17 00:00:00 2001 From: Mahad Zaryab Date: Tue, 17 Sep 2024 19:35:37 -0600 Subject: [PATCH 02/18] Update References To Configuration Signed-off-by: Mahad Zaryab --- .../internal/extension/jaegerquery/factory.go | 16 +++--- .../internal/extension/jaegerquery/server.go | 16 +++--- .../extension/jaegerquery/server_test.go | 54 +++++++++++++------ 3 files changed, 54 insertions(+), 32 deletions(-) diff --git a/cmd/jaeger/internal/extension/jaegerquery/factory.go b/cmd/jaeger/internal/extension/jaegerquery/factory.go index 9d50bc55ea5..d0793e34a04 100644 --- a/cmd/jaeger/internal/extension/jaegerquery/factory.go +++ b/cmd/jaeger/internal/extension/jaegerquery/factory.go @@ -27,13 +27,15 @@ func NewFactory() extension.Factory { func createDefaultConfig() component.Config { return &Config{ - HTTP: confighttp.ServerConfig{ - Endpoint: ports.PortToHostPort(ports.QueryHTTP), - }, - GRPC: configgrpc.ServerConfig{ - NetAddr: confignet.AddrConfig{ - Endpoint: ports.PortToHostPort(ports.QueryGRPC), - Transport: confignet.TransportTypeTCP, + Connection: Connection{ + HTTP: confighttp.ServerConfig{ + Endpoint: ports.PortToHostPort(ports.QueryHTTP), + }, + GRPC: configgrpc.ServerConfig{ + NetAddr: confignet.AddrConfig{ + Endpoint: ports.PortToHostPort(ports.QueryGRPC), + Transport: confignet.TransportTypeTCP, + }, }, }, } diff --git a/cmd/jaeger/internal/extension/jaegerquery/server.go b/cmd/jaeger/internal/extension/jaegerquery/server.go index 0d217a7d806..19ac2fab9ed 100644 --- a/cmd/jaeger/internal/extension/jaegerquery/server.go +++ b/cmd/jaeger/internal/extension/jaegerquery/server.go @@ -53,9 +53,9 @@ func (s *server) Start(_ context.Context, host component.Host) error { mf := otelmetrics.NewFactory(s.telset.MeterProvider) baseFactory := mf.Namespace(metrics.NSOptions{Name: "jaeger"}) queryMetricsFactory := baseFactory.Namespace(metrics.NSOptions{Name: "query"}) - f, err := jaegerstorage.GetStorageFactory(s.config.TraceStoragePrimary, host) + f, err := jaegerstorage.GetStorageFactory(s.config.Storage.TracePrimary, host) if err != nil { - return fmt.Errorf("cannot find primary storage %s: %w", s.config.TraceStoragePrimary, err) + return fmt.Errorf("cannot find primary storage %s: %w", s.config.Storage.TracePrimary, err) } spanReader, err := f.CreateSpanReader() @@ -122,12 +122,12 @@ func (s *server) Start(_ context.Context, host component.Host) error { } func (s *server) addArchiveStorage(opts *querysvc.QueryServiceOptions, host component.Host) error { - if s.config.TraceStorageArchive == "" { + if s.config.Storage.TraceArchive == "" { s.telset.Logger.Info("Archive storage not configured") return nil } - f, err := jaegerstorage.GetStorageFactory(s.config.TraceStorageArchive, host) + f, err := jaegerstorage.GetStorageFactory(s.config.Storage.TraceArchive, host) if err != nil { return fmt.Errorf("cannot find archive storage factory: %w", err) } @@ -139,12 +139,12 @@ func (s *server) addArchiveStorage(opts *querysvc.QueryServiceOptions, host comp } func (s *server) createMetricReader(host component.Host) (metricsstore.Reader, error) { - if s.config.MetricStorage == "" { + if s.config.Storage.Metric == "" { s.telset.Logger.Info("Metric storage not configured") return disabled.NewMetricsReader() } - mf, err := jaegerstorage.GetMetricsFactory(s.config.MetricStorage, host) + mf, err := jaegerstorage.GetMetricsFactory(s.config.Storage.Metric, host) if err != nil { return nil, fmt.Errorf("cannot find metrics storage factory: %w", err) } @@ -161,8 +161,8 @@ func (s *server) makeQueryOptions() *queryApp.QueryOptions { QueryOptionsBase: s.config.QueryOptionsBase, // TODO utilize OTEL helpers for creating HTTP/GRPC servers - HTTPHostPort: s.config.HTTP.Endpoint, - GRPCHostPort: s.config.GRPC.NetAddr.Endpoint, + HTTPHostPort: s.config.Connection.HTTP.Endpoint, + GRPCHostPort: s.config.Connection.GRPC.NetAddr.Endpoint, // TODO handle TLS } } diff --git a/cmd/jaeger/internal/extension/jaegerquery/server_test.go b/cmd/jaeger/internal/extension/jaegerquery/server_test.go index 71da9176726..7f6ef984efe 100644 --- a/cmd/jaeger/internal/extension/jaegerquery/server_test.go +++ b/cmd/jaeger/internal/extension/jaegerquery/server_test.go @@ -132,53 +132,67 @@ func TestServerStart(t *testing.T) { { name: "Non-empty config with fake storage host", config: &Config{ - TraceStorageArchive: "jaeger_storage", - TraceStoragePrimary: "jaeger_storage", - MetricStorage: "jaeger_metrics_storage", + Storage: Storage{ + TraceArchive: "jaeger_storage", + TracePrimary: "jaeger_storage", + Metric: "jaeger_metrics_storage", + }, }, }, { name: "factory error", config: &Config{ - TraceStoragePrimary: "need-factory-error", + Storage: Storage{ + TracePrimary: "need-factory-error", + }, }, expectedErr: "cannot find primary storage", }, { name: "span reader error", config: &Config{ - TraceStoragePrimary: "need-span-reader-error", + Storage: Storage{ + TracePrimary: "need-span-reader-error", + }, }, expectedErr: "cannot create span reader", }, { name: "dependency error", config: &Config{ - TraceStoragePrimary: "need-dependency-reader-error", + Storage: Storage{ + TracePrimary: "need-dependency-reader-error", + }, }, expectedErr: "cannot create dependencies reader", }, { name: "storage archive error", config: &Config{ - TraceStorageArchive: "need-factory-error", - TraceStoragePrimary: "jaeger_storage", + Storage: Storage{ + TraceArchive: "need-factory-error", + TracePrimary: "jaeger_storage", + }, }, expectedErr: "cannot find archive storage factory", }, { name: "metrics storage error", config: &Config{ - MetricStorage: "need-factory-error", - TraceStoragePrimary: "jaeger_storage", + Storage: Storage{ + Metric: "need-factory-error", + TracePrimary: "jaeger_storage", + }, }, expectedErr: "cannot find metrics storage factory", }, { name: " metrics reader error", config: &Config{ - MetricStorage: "need-metrics-reader-error", - TraceStoragePrimary: "jaeger_storage", + Storage: Storage{ + Metric: "need-metrics-reader-error", + TracePrimary: "jaeger_storage", + }, }, expectedErr: "cannot create metrics reader", }, @@ -190,8 +204,8 @@ func TestServerStart(t *testing.T) { Logger: zaptest.NewLogger(t, zaptest.WrapOptions(zap.AddCaller())), MeterProvider: noopMeter.NewMeterProvider(), } - tt.config.HTTP.Endpoint = ":0" - tt.config.GRPC.NetAddr.Endpoint = ":0" + tt.config.Connection.HTTP.Endpoint = ":0" + tt.config.Connection.GRPC.NetAddr.Endpoint = ":0" server := newServer(tt.config, telemetrySettings) err := server.Start(context.Background(), host) if tt.expectedErr == "" { @@ -249,7 +263,9 @@ func TestServerAddArchiveStorage(t *testing.T) { { name: "Archive storage set", config: &Config{ - TraceStorageArchive: "random-value", + Storage: Storage{ + TraceArchive: "random-value", + }, }, qSvcOpts: &querysvc.QueryServiceOptions{}, expectedOutput: "", @@ -258,7 +274,9 @@ func TestServerAddArchiveStorage(t *testing.T) { { name: "Archive storage not supported", config: &Config{ - TraceStorageArchive: "badger", + Storage: Storage{ + TraceArchive: "badger", + }, }, qSvcOpts: &querysvc.QueryServiceOptions{}, extension: fakeStorageExt{}, @@ -308,7 +326,9 @@ func TestServerAddMetricsStorage(t *testing.T) { { name: "Metrics storage set", config: &Config{ - MetricStorage: "random-value", + Storage: Storage{ + Metric: "random-value", + }, }, expectedOutput: "", expectedErr: "cannot find metrics storage factory: cannot find extension", From e6c23390870d7f688f48fff67360f60f6dff4bd4 Mon Sep 17 00:00:00 2001 From: Mahad Zaryab Date: Tue, 17 Sep 2024 19:35:53 -0600 Subject: [PATCH 03/18] Update Existing Configuration Files Signed-off-by: Mahad Zaryab --- cmd/jaeger/config-badger.yaml | 5 +++-- cmd/jaeger/config-cassandra.yaml | 5 +++-- cmd/jaeger/config-elasticsearch.yaml | 5 +++-- cmd/jaeger/config-kafka-ingester.yaml | 3 ++- cmd/jaeger/config-opensearch.yaml | 5 +++-- cmd/jaeger/config-remote-storage.yaml | 3 ++- cmd/jaeger/config-tail-sampling-always-sample.yaml | 3 ++- cmd/jaeger/config-tail-sampling-service-name-policy.yaml | 3 ++- cmd/jaeger/config.yaml | 5 +++-- cmd/jaeger/internal/all-in-one.yaml | 3 ++- docker-compose/monitor/jaeger-v2-config.yml | 5 +++-- docker-compose/tail-sampling/jaeger-v2-config.yml | 3 ++- 12 files changed, 30 insertions(+), 18 deletions(-) diff --git a/cmd/jaeger/config-badger.yaml b/cmd/jaeger/config-badger.yaml index 7cbbdccd8bb..266d4d3cad1 100644 --- a/cmd/jaeger/config-badger.yaml +++ b/cmd/jaeger/config-badger.yaml @@ -12,8 +12,9 @@ extensions: http: jaeger_query: - trace_storage: some_store - trace_storage_archive: another_store + storage: + trace: some_store + trace_archive: another_store ui_config: ./cmd/jaeger/config-ui.json jaeger_storage: diff --git a/cmd/jaeger/config-cassandra.yaml b/cmd/jaeger/config-cassandra.yaml index 893b3a1800f..108cacd0f1f 100644 --- a/cmd/jaeger/config-cassandra.yaml +++ b/cmd/jaeger/config-cassandra.yaml @@ -12,8 +12,9 @@ extensions: http: jaeger_query: - trace_storage: some_storage - trace_storage_archive: another_storage + storage: + trace: some_storage + trace_archive: another_storage ui_config: ./cmd/jaeger/config-ui.json jaeger_storage: diff --git a/cmd/jaeger/config-elasticsearch.yaml b/cmd/jaeger/config-elasticsearch.yaml index 42cf78571c6..42dc9d12cf4 100644 --- a/cmd/jaeger/config-elasticsearch.yaml +++ b/cmd/jaeger/config-elasticsearch.yaml @@ -12,8 +12,9 @@ extensions: http: jaeger_query: - trace_storage: some_storage - trace_storage_archive: another_storage + storage: + trace: some_storage + trace_archive: another_storage ui_config: ./cmd/jaeger/config-ui.json jaeger_storage: diff --git a/cmd/jaeger/config-kafka-ingester.yaml b/cmd/jaeger/config-kafka-ingester.yaml index 9adfc308a9c..0bfe9c64fd4 100644 --- a/cmd/jaeger/config-kafka-ingester.yaml +++ b/cmd/jaeger/config-kafka-ingester.yaml @@ -19,7 +19,8 @@ extensions: endpoint: 0.0.0.0:14133 jaeger_query: - trace_storage: some_storage + storage: + trace: some_storage jaeger_storage: backends: diff --git a/cmd/jaeger/config-opensearch.yaml b/cmd/jaeger/config-opensearch.yaml index 19be6e941c1..e9946dd1ccd 100644 --- a/cmd/jaeger/config-opensearch.yaml +++ b/cmd/jaeger/config-opensearch.yaml @@ -12,8 +12,9 @@ extensions: http: jaeger_query: - trace_storage: some_storage - trace_storage_archive: another_storage + storage: + trace: some_storage + trace_archive: another_storage ui_config: ./cmd/jaeger/config-ui.json jaeger_storage: diff --git a/cmd/jaeger/config-remote-storage.yaml b/cmd/jaeger/config-remote-storage.yaml index 7c8b3dd7228..89eef7a81ee 100644 --- a/cmd/jaeger/config-remote-storage.yaml +++ b/cmd/jaeger/config-remote-storage.yaml @@ -12,7 +12,8 @@ extensions: http: jaeger_query: - trace_storage: some-storage + storage: + trace: some-storage ui_config: ./cmd/jaeger/config-ui.json jaeger_storage: diff --git a/cmd/jaeger/config-tail-sampling-always-sample.yaml b/cmd/jaeger/config-tail-sampling-always-sample.yaml index 648950fc4fc..19516b0da81 100644 --- a/cmd/jaeger/config-tail-sampling-always-sample.yaml +++ b/cmd/jaeger/config-tail-sampling-always-sample.yaml @@ -14,7 +14,8 @@ extensions: use_v2: true http: jaeger_query: - trace_storage: some_storage + storage: + trace: some_storage jaeger_storage: backends: some_storage: diff --git a/cmd/jaeger/config-tail-sampling-service-name-policy.yaml b/cmd/jaeger/config-tail-sampling-service-name-policy.yaml index fc4abff70af..125b33a082a 100644 --- a/cmd/jaeger/config-tail-sampling-service-name-policy.yaml +++ b/cmd/jaeger/config-tail-sampling-service-name-policy.yaml @@ -14,7 +14,8 @@ extensions: use_v2: true http: jaeger_query: - trace_storage: some_storage + storage: + trace: some_storage jaeger_storage: backends: some_storage: diff --git a/cmd/jaeger/config.yaml b/cmd/jaeger/config.yaml index f068e81ef6a..bf349920bf2 100644 --- a/cmd/jaeger/config.yaml +++ b/cmd/jaeger/config.yaml @@ -17,8 +17,9 @@ extensions: # endpoint: 0.0.0.0:55679 jaeger_query: - trace_storage: some_store - trace_storage_archive: another_store + storage: + trace: some_store + trace_archive: another_store ui_config: ./cmd/jaeger/config-ui.json jaeger_storage: diff --git a/cmd/jaeger/internal/all-in-one.yaml b/cmd/jaeger/internal/all-in-one.yaml index 4dad6d883ef..7f709775c95 100644 --- a/cmd/jaeger/internal/all-in-one.yaml +++ b/cmd/jaeger/internal/all-in-one.yaml @@ -16,7 +16,8 @@ service: extensions: jaeger_query: - trace_storage: some_storage + storage: + trace: some_storage jaeger_storage: backends: diff --git a/docker-compose/monitor/jaeger-v2-config.yml b/docker-compose/monitor/jaeger-v2-config.yml index bc7160c5bb8..14b2de057ea 100644 --- a/docker-compose/monitor/jaeger-v2-config.yml +++ b/docker-compose/monitor/jaeger-v2-config.yml @@ -14,8 +14,9 @@ service: extensions: jaeger_query: - trace_storage: some_storage - metric_storage: some_metrics_storage + storage: + trace: some_storage + metric: some_metrics_storage jaeger_storage: backends: some_storage: diff --git a/docker-compose/tail-sampling/jaeger-v2-config.yml b/docker-compose/tail-sampling/jaeger-v2-config.yml index 545ecf4ea8b..3f32cef4ef3 100644 --- a/docker-compose/tail-sampling/jaeger-v2-config.yml +++ b/docker-compose/tail-sampling/jaeger-v2-config.yml @@ -14,7 +14,8 @@ extensions: use_v2: true http: jaeger_query: - trace_storage: some_storage + storage: + trace: some_storage jaeger_storage: backends: some_storage: From d5931df84ebafc4017e40517e5b157dc62cdce54 Mon Sep 17 00:00:00 2001 From: Mahad Zaryab Date: Tue, 17 Sep 2024 19:48:23 -0600 Subject: [PATCH 04/18] Fix Failing Unit Tests Signed-off-by: Mahad Zaryab --- cmd/jaeger/internal/extension/jaegerquery/config_test.go | 2 +- cmd/jaeger/internal/integration/e2e_integration.go | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/cmd/jaeger/internal/extension/jaegerquery/config_test.go b/cmd/jaeger/internal/extension/jaegerquery/config_test.go index 9cc6e85f5af..c438906de30 100644 --- a/cmd/jaeger/internal/extension/jaegerquery/config_test.go +++ b/cmd/jaeger/internal/extension/jaegerquery/config_test.go @@ -19,7 +19,7 @@ func Test_Validate(t *testing.T) { { name: "Empty config", config: &Config{}, - expectedErr: "TraceStoragePrimary: non zero value required", + expectedErr: "Storage.TracePrimary: non zero value required", }, { name: "Non empty-config", diff --git a/cmd/jaeger/internal/integration/e2e_integration.go b/cmd/jaeger/internal/integration/e2e_integration.go index fdfaa15c663..14f78b20497 100644 --- a/cmd/jaeger/internal/integration/e2e_integration.go +++ b/cmd/jaeger/internal/integration/e2e_integration.go @@ -223,7 +223,9 @@ func createStorageCleanerConfig(t *testing.T, configFile string, storage string) extensions := extensionsAny.(map[string]any) queryAny, ok := extensions["jaeger_query"] require.True(t, ok) - traceStorageAny, ok := queryAny.(map[string]any)["trace_storage"] + storageAny, ok := queryAny.(map[string]any)["storage"] + require.True(t, ok) + traceStorageAny, ok := storageAny.(map[string]any)["trace"] require.True(t, ok) traceStorage := traceStorageAny.(string) extensions["storage_cleaner"] = map[string]string{"trace_storage": traceStorage} From a3345f02d1ef68b6a1e64ad118dfae863fdc22ac Mon Sep 17 00:00:00 2001 From: Mahad Zaryab Date: Wed, 18 Sep 2024 08:13:28 -0600 Subject: [PATCH 05/18] Remove Repeated Fields Out of Base Configuration Signed-off-by: Mahad Zaryab --- cmd/jaeger/internal/extension/jaegerquery/server.go | 2 +- cmd/query/app/flags.go | 8 ++++---- cmd/query/app/server_test.go | 6 ++---- 3 files changed, 7 insertions(+), 9 deletions(-) diff --git a/cmd/jaeger/internal/extension/jaegerquery/server.go b/cmd/jaeger/internal/extension/jaegerquery/server.go index 19ac2fab9ed..23274e8fd1b 100644 --- a/cmd/jaeger/internal/extension/jaegerquery/server.go +++ b/cmd/jaeger/internal/extension/jaegerquery/server.go @@ -81,7 +81,7 @@ func (s *server) Start(_ context.Context, host component.Host) error { return err } - tm := tenancy.NewManager(&s.config.Tenancy) + tm := tenancy.NewManager(&s.config.Connection.Tenancy) // TODO OTel-collector does not initialize the tracer currently // https://github.com/open-telemetry/opentelemetry-collector/issues/7532 diff --git a/cmd/query/app/flags.go b/cmd/query/app/flags.go index 34d6fde6d98..f8991855925 100644 --- a/cmd/query/app/flags.go +++ b/cmd/query/app/flags.go @@ -67,12 +67,8 @@ type QueryOptionsBase struct { UIConfig string `valid:"optional" mapstructure:"ui_config"` // BearerTokenPropagation activate/deactivate bearer token propagation to storage BearerTokenPropagation bool - // AdditionalHeaders - AdditionalHeaders http.Header // MaxClockSkewAdjust is the maximum duration by which jaeger-query will adjust a span MaxClockSkewAdjust time.Duration - // Tenancy configures tenancy for query - Tenancy tenancy.Options // EnableTracing determines whether traces will be emitted by jaeger-query. EnableTracing bool } @@ -81,6 +77,10 @@ type QueryOptionsBase struct { type QueryOptions struct { QueryOptionsBase + // Tenancy configures tenancy for query + Tenancy tenancy.Options + // AdditionalHeaders + AdditionalHeaders http.Header // HTTPHostPort is the host:port address that the query service listens in on for http requests HTTPHostPort string // GRPCHostPort is the host:port address that the query service listens in on for gRPC requests diff --git a/cmd/query/app/server_test.go b/cmd/query/app/server_test.go index b028dfbaed7..d5a22a2f34b 100644 --- a/cmd/query/app/server_test.go +++ b/cmd/query/app/server_test.go @@ -721,10 +721,8 @@ func TestServerHTTPTenancy(t *testing.T) { serverOptions := &QueryOptions{ HTTPHostPort: ":8080", GRPCHostPort: ":8080", - QueryOptionsBase: QueryOptionsBase{ - Tenancy: tenancy.Options{ - Enabled: true, - }, + Tenancy: tenancy.Options{ + Enabled: true, }, } tenancyMgr := tenancy.NewManager(&serverOptions.Tenancy) From 32b1ffbc9c6af7327c5a52a3d59fa71461f84fb5 Mon Sep 17 00:00:00 2001 From: Mahad Zaryab Date: Wed, 18 Sep 2024 21:43:39 -0600 Subject: [PATCH 06/18] Perform More Config Groupings And Rename Structs Signed-off-by: Mahad Zaryab --- cmd/all-in-one/main.go | 4 +- .../internal/extension/jaegerquery/config.go | 22 +++++--- .../internal/extension/jaegerquery/factory.go | 4 ++ .../internal/extension/jaegerquery/server.go | 6 +-- .../internal/integration/e2e_integration.go | 2 +- cmd/query/app/flags.go | 54 +++++++++---------- cmd/query/app/flags_test.go | 16 +++--- cmd/query/app/server.go | 8 +-- cmd/query/app/server_test.go | 38 ++++++------- cmd/query/app/static_handler.go | 8 +-- cmd/query/app/static_handler_test.go | 20 +++---- cmd/query/app/token_propagation_test.go | 4 +- cmd/query/main.go | 2 +- 13 files changed, 98 insertions(+), 90 deletions(-) diff --git a/cmd/all-in-one/main.go b/cmd/all-in-one/main.go index 15c8c8cd5e2..3a3dc0f838f 100644 --- a/cmd/all-in-one/main.go +++ b/cmd/all-in-one/main.go @@ -142,7 +142,7 @@ by default uses only in-memory database.`, if err != nil { logger.Fatal("Failed to initialize collector", zap.Error(err)) } - qOpts, err := new(queryApp.QueryOptions).InitFromViper(v, logger) + qOpts, err := new(queryApp.Options).InitFromViper(v, logger) if err != nil { logger.Fatal("Failed to configure query service", zap.Error(err)) } @@ -263,7 +263,7 @@ func startAgent( func startQuery( svc *flags.Service, - qOpts *queryApp.QueryOptions, + qOpts *queryApp.Options, queryOpts *querysvc.QueryServiceOptions, spanReader spanstore.Reader, depReader dependencystore.Reader, diff --git a/cmd/jaeger/internal/extension/jaegerquery/config.go b/cmd/jaeger/internal/extension/jaegerquery/config.go index b74d2e74018..f6927325c37 100644 --- a/cmd/jaeger/internal/extension/jaegerquery/config.go +++ b/cmd/jaeger/internal/extension/jaegerquery/config.go @@ -17,21 +17,27 @@ var _ component.ConfigValidator = (*Config)(nil) // Config represents the configuration for jaeger-query, type Config struct { - queryApp.QueryOptionsBase `mapstructure:",squash"` - Connection Connection `mapstructure:"connection"` - Storage Storage `mapstructure:"storage"` + queryApp.OptionsBase `mapstructure:",squash"` + Connection Connection `mapstructure:"connection"` + Storage Storage `mapstructure:"storage"` } type Connection struct { - Tenancy tenancy.Options `mapstructure:"multi_tenancy"` - HTTP confighttp.ServerConfig `mapstructure:"http"` - GRPC configgrpc.ServerConfig `mapstructure:"grpc"` + // Tenancy holds the multi-tenancy configuration. + Tenancy tenancy.Options `mapstructure:"multi_tenancy"` + // HTTP holds the HTTP configuration that the query service uses to serve requests. + HTTP confighttp.ServerConfig `mapstructure:"http"` + // GRPC holds the GRPC configuration that the query service uses to serve requests. + GRPC configgrpc.ServerConfig `mapstructure:"grpc"` } type Storage struct { - TracePrimary string `mapstructure:"trace" valid:"required" ` + // TracePrimary contains the name of the primary trace storage that is being queried. + TracePrimary string `mapstructure:"trace" valid:"required"` + // TraceArchive contains the name of the archive trace storage that is being queried. TraceArchive string `mapstructure:"trace_archive" valid:"optional"` - Metric string `mapstructure:"metric" valid:"optional"` + // Metric contains the name of the metric storage that is being queried. + Metric string `mapstructure:"metric" valid:"optional"` } func (cfg *Config) Validate() error { diff --git a/cmd/jaeger/internal/extension/jaegerquery/factory.go b/cmd/jaeger/internal/extension/jaegerquery/factory.go index d0793e34a04..fcd047c2854 100644 --- a/cmd/jaeger/internal/extension/jaegerquery/factory.go +++ b/cmd/jaeger/internal/extension/jaegerquery/factory.go @@ -12,6 +12,7 @@ import ( "go.opentelemetry.io/collector/config/confignet" "go.opentelemetry.io/collector/extension" + "github.com/jaegertracing/jaeger/cmd/query/app" "github.com/jaegertracing/jaeger/ports" ) @@ -27,6 +28,9 @@ func NewFactory() extension.Factory { func createDefaultConfig() component.Config { return &Config{ + OptionsBase: app.OptionsBase{ + BasePath: "/", + }, Connection: Connection{ HTTP: confighttp.ServerConfig{ Endpoint: ports.PortToHostPort(ports.QueryHTTP), diff --git a/cmd/jaeger/internal/extension/jaegerquery/server.go b/cmd/jaeger/internal/extension/jaegerquery/server.go index 23274e8fd1b..d703a9dffd1 100644 --- a/cmd/jaeger/internal/extension/jaegerquery/server.go +++ b/cmd/jaeger/internal/extension/jaegerquery/server.go @@ -156,9 +156,9 @@ func (s *server) createMetricReader(host component.Host) (metricsstore.Reader, e return metricsReader, err } -func (s *server) makeQueryOptions() *queryApp.QueryOptions { - return &queryApp.QueryOptions{ - QueryOptionsBase: s.config.QueryOptionsBase, +func (s *server) makeQueryOptions() *queryApp.Options { + return &queryApp.Options{ + OptionsBase: s.config.OptionsBase, // TODO utilize OTEL helpers for creating HTTP/GRPC servers HTTPHostPort: s.config.Connection.HTTP.Endpoint, diff --git a/cmd/jaeger/internal/integration/e2e_integration.go b/cmd/jaeger/internal/integration/e2e_integration.go index 14f78b20497..ea5745d555f 100644 --- a/cmd/jaeger/internal/integration/e2e_integration.go +++ b/cmd/jaeger/internal/integration/e2e_integration.go @@ -126,7 +126,7 @@ func (s *E2EStorageIntegration) e2eInitialize(t *testing.T, storage string) { Path: "./cmd/jaeger/jaeger", Args: []string{"jaeger", "--config", configFile}, // Change the working directory to the root of this project - // since the binary config file jaeger_query's ui_config points to + // since the binary config file jaeger_query's ui.config_file points to // "./cmd/jaeger/config-ui.json" Dir: "../../../..", }, diff --git a/cmd/query/app/flags.go b/cmd/query/app/flags.go index f8991855925..4025f654bc3 100644 --- a/cmd/query/app/flags.go +++ b/cmd/query/app/flags.go @@ -48,34 +48,32 @@ var tlsHTTPFlagsConfig = tlscfg.ServerFlagsConfig{ Prefix: "query.http", } -// QueryOptionsStaticAssets contains configuration for handling static assets -type QueryOptionsStaticAssets struct { - // Path is the path for the static assets for the UI (https://github.com/uber/jaeger-ui) - Path string `valid:"optional" mapstructure:"path"` - // LogAccess tells static handler to log access to static assets, useful in debugging - LogAccess bool `valid:"optional" mapstructure:"log_access"` +type UI struct { + // ConfigFile is the path to a configuration file for the UI. + ConfigFile string `mapstructure:"config_file" valid:"optional"` + // AssetsPath is the path for the static assets for the UI (https://github.com/uber/jaeger-ui). + AssetsPath string `mapstructure:"assets_path" valid:"optional" ` + // LogAccess tells static handler to log access to static assets, useful in debugging. + LogAccess bool `mapstructure:"log_access" valid:"optional"` } -// QueryOptionsBase holds configuration for query service shared with jaeger(v2) -type QueryOptionsBase struct { - // BasePath is the base path for all HTTP routes - BasePath string - - StaticAssets QueryOptionsStaticAssets `valid:"optional" mapstructure:"static_assets"` - - // UIConfig is the path to a configuration file for the UI - UIConfig string `valid:"optional" mapstructure:"ui_config"` - // BearerTokenPropagation activate/deactivate bearer token propagation to storage - BearerTokenPropagation bool - // MaxClockSkewAdjust is the maximum duration by which jaeger-query will adjust a span - MaxClockSkewAdjust time.Duration +// OptionsBase holds configuration for query service shared with jaeger-v2 +type OptionsBase struct { + // BasePath is the base path for all HTTP routes. + BasePath string `mapstructure:"base_path"` + // UI contains configuration related to the Jaeger UI. + UI UI `mapstructure:"ui" valid:"optional"` + // BearerTokenPropagation activate/deactivate bearer token propagation to storage. + BearerTokenPropagation bool `mapstructure:"bearer_token_propagation"` + // MaxClockSkewAdjust is the maximum duration by which jaeger-query will adjust a span. + MaxClockSkewAdjust time.Duration `mapstructure:"max_clock_skew_adjust"` // EnableTracing determines whether traces will be emitted by jaeger-query. - EnableTracing bool + EnableTracing bool `mapstructure:"enable_tracing"` } -// QueryOptions holds configuration for query service -type QueryOptions struct { - QueryOptionsBase +// Options holds configuration for query service +type Options struct { + OptionsBase // Tenancy configures tenancy for query Tenancy tenancy.Options @@ -108,7 +106,7 @@ func AddFlags(flagSet *flag.FlagSet) { } // InitFromViper initializes QueryOptions with properties from viper -func (qOpts *QueryOptions) InitFromViper(v *viper.Viper, logger *zap.Logger) (*QueryOptions, error) { +func (qOpts *Options) InitFromViper(v *viper.Viper, logger *zap.Logger) (*Options, error) { qOpts.HTTPHostPort = v.GetString(queryHTTPHostPort) qOpts.GRPCHostPort = v.GetString(queryGRPCHostPort) tlsGrpc, err := tlsGRPCFlagsConfig.InitFromViper(v) @@ -122,9 +120,9 @@ func (qOpts *QueryOptions) InitFromViper(v *viper.Viper, logger *zap.Logger) (*Q } qOpts.TLSHTTP = tlsHTTP qOpts.BasePath = v.GetString(queryBasePath) - qOpts.StaticAssets.Path = v.GetString(queryStaticFiles) - qOpts.StaticAssets.LogAccess = v.GetBool(queryLogStaticAssetsAccess) - qOpts.UIConfig = v.GetString(queryUIConfig) + qOpts.UI.AssetsPath = v.GetString(queryStaticFiles) + qOpts.UI.LogAccess = v.GetBool(queryLogStaticAssetsAccess) + qOpts.UI.ConfigFile = v.GetString(queryUIConfig) qOpts.BearerTokenPropagation = v.GetBool(queryTokenPropagation) qOpts.MaxClockSkewAdjust = v.GetDuration(queryMaxClockSkewAdjust) @@ -141,7 +139,7 @@ func (qOpts *QueryOptions) InitFromViper(v *viper.Viper, logger *zap.Logger) (*Q } // BuildQueryServiceOptions creates a QueryServiceOptions struct with appropriate adjusters and archive config -func (qOpts *QueryOptions) BuildQueryServiceOptions(storageFactory storage.Factory, logger *zap.Logger) *querysvc.QueryServiceOptions { +func (qOpts *Options) BuildQueryServiceOptions(storageFactory storage.Factory, logger *zap.Logger) *querysvc.QueryServiceOptions { opts := &querysvc.QueryServiceOptions{} if !opts.InitArchiveStorage(storageFactory, logger) { logger.Info("Archive storage not initialized") diff --git a/cmd/query/app/flags_test.go b/cmd/query/app/flags_test.go index 2322b9534db..72351224b73 100644 --- a/cmd/query/app/flags_test.go +++ b/cmd/query/app/flags_test.go @@ -33,11 +33,11 @@ func TestQueryBuilderFlags(t *testing.T) { "--query.additional-headers=whatever:thing", "--query.max-clock-skew-adjustment=10s", }) - qOpts, err := new(QueryOptions).InitFromViper(v, zap.NewNop()) + qOpts, err := new(Options).InitFromViper(v, zap.NewNop()) require.NoError(t, err) - assert.Equal(t, "/dev/null", qOpts.StaticAssets.Path) - assert.True(t, qOpts.StaticAssets.LogAccess) - assert.Equal(t, "some.json", qOpts.UIConfig) + assert.Equal(t, "/dev/null", qOpts.UI.AssetsPath) + assert.True(t, qOpts.UI.LogAccess) + assert.Equal(t, "some.json", qOpts.UI.ConfigFile) assert.Equal(t, "/jaeger", qOpts.BasePath) assert.Equal(t, "127.0.0.1:8080", qOpts.HTTPHostPort) assert.Equal(t, "127.0.0.1:8081", qOpts.GRPCHostPort) @@ -53,7 +53,7 @@ func TestQueryBuilderBadHeadersFlags(t *testing.T) { command.ParseFlags([]string{ "--query.additional-headers=malformedheader", }) - qOpts, err := new(QueryOptions).InitFromViper(v, zap.NewNop()) + qOpts, err := new(Options).InitFromViper(v, zap.NewNop()) require.NoError(t, err) assert.Nil(t, qOpts.AdditionalHeaders) } @@ -87,7 +87,7 @@ func TestStringSliceAsHeader(t *testing.T) { func TestBuildQueryServiceOptions(t *testing.T) { v, _ := config.Viperize(AddFlags) - qOpts, err := new(QueryOptions).InitFromViper(v, zap.NewNop()) + qOpts, err := new(Options).InitFromViper(v, zap.NewNop()) require.NoError(t, err) assert.NotNil(t, qOpts) @@ -158,7 +158,7 @@ func TestQueryOptionsPortAllocationFromFlags(t *testing.T) { t.Run(test.name, func(t *testing.T) { v, command := config.Viperize(AddFlags) command.ParseFlags(test.flagsArray) - qOpts, err := new(QueryOptions).InitFromViper(v, zap.NewNop()) + qOpts, err := new(Options).InitFromViper(v, zap.NewNop()) require.NoError(t, err) assert.Equal(t, test.expectedHTTPHostPort, qOpts.HTTPHostPort) @@ -177,7 +177,7 @@ func TestQueryOptions_FailedTLSFlags(t *testing.T) { "--query." + proto + ".tls.cert=blah", // invalid unless tls.enabled }) require.NoError(t, err) - _, err = new(QueryOptions).InitFromViper(v, zap.NewNop()) + _, err = new(Options).InitFromViper(v, zap.NewNop()) require.Error(t, err) assert.Contains(t, err.Error(), "failed to process "+test+" TLS options") }) diff --git a/cmd/query/app/server.go b/cmd/query/app/server.go index 1e61b0cd09d..5f0d5a4aaca 100644 --- a/cmd/query/app/server.go +++ b/cmd/query/app/server.go @@ -39,7 +39,7 @@ import ( // Server runs HTTP, Mux and a grpc server type Server struct { querySvc *querysvc.QueryService - queryOptions *QueryOptions + queryOptions *Options conn net.Listener grpcConn net.Listener @@ -55,7 +55,7 @@ type Server struct { // NewServer creates and initializes Server func NewServer(querySvc *querysvc.QueryService, metricsQuerySvc querysvc.MetricsQueryService, - options *QueryOptions, + options *Options, tm *tenancy.Manager, telset telemetery.Setting, ) (*Server, error) { @@ -93,7 +93,7 @@ func NewServer(querySvc *querysvc.QueryService, }, nil } -func createGRPCServer(querySvc *querysvc.QueryService, metricsQuerySvc querysvc.MetricsQueryService, options *QueryOptions, tm *tenancy.Manager, telset telemetery.Setting) (*grpc.Server, error) { +func createGRPCServer(querySvc *querysvc.QueryService, metricsQuerySvc querysvc.MetricsQueryService, options *Options, tm *tenancy.Manager, telset telemetery.Setting) (*grpc.Server, error) { var grpcOpts []grpc.ServerOption if options.TLSGRPC.Enabled { @@ -143,7 +143,7 @@ var _ io.Closer = (*httpServer)(nil) func createHTTPServer( querySvc *querysvc.QueryService, metricsQuerySvc querysvc.MetricsQueryService, - queryOpts *QueryOptions, + queryOpts *Options, tm *tenancy.Manager, telset telemetery.Setting, ) (*httpServer, error) { diff --git a/cmd/query/app/server_test.go b/cmd/query/app/server_test.go index d5a22a2f34b..8d7d567728a 100644 --- a/cmd/query/app/server_test.go +++ b/cmd/query/app/server_test.go @@ -49,7 +49,7 @@ func initTelSet(logger *zap.Logger, tracerProvider *jtracer.JTracer, hc *healthc func TestServerError(t *testing.T) { srv := &Server{ - queryOptions: &QueryOptions{ + queryOptions: &Options{ HTTPHostPort: ":-1", }, } @@ -66,7 +66,7 @@ func TestCreateTLSServerSinglePortError(t *testing.T) { } telset := initTelSet(zaptest.NewLogger(t), jtracer.NoOp(), healthcheck.New()) _, err := NewServer(&querysvc.QueryService{}, nil, - &QueryOptions{HTTPHostPort: ":8080", GRPCHostPort: ":8080", TLSGRPC: tlsCfg, TLSHTTP: tlsCfg}, + &Options{HTTPHostPort: ":8080", GRPCHostPort: ":8080", TLSGRPC: tlsCfg, TLSHTTP: tlsCfg}, tenancy.NewManager(&tenancy.Options{}), telset) require.Error(t, err) } @@ -80,7 +80,7 @@ func TestCreateTLSGrpcServerError(t *testing.T) { } telset := initTelSet(zaptest.NewLogger(t), jtracer.NoOp(), healthcheck.New()) _, err := NewServer(&querysvc.QueryService{}, nil, - &QueryOptions{HTTPHostPort: ":8080", GRPCHostPort: ":8081", TLSGRPC: tlsCfg}, + &Options{HTTPHostPort: ":8080", GRPCHostPort: ":8081", TLSGRPC: tlsCfg}, tenancy.NewManager(&tenancy.Options{}), telset) require.Error(t, err) } @@ -94,7 +94,7 @@ func TestCreateTLSHttpServerError(t *testing.T) { } telset := initTelSet(zaptest.NewLogger(t), jtracer.NoOp(), healthcheck.New()) _, err := NewServer(&querysvc.QueryService{}, nil, - &QueryOptions{HTTPHostPort: ":8080", GRPCHostPort: ":8081", TLSHTTP: tlsCfg}, + &Options{HTTPHostPort: ":8080", GRPCHostPort: ":8081", TLSHTTP: tlsCfg}, tenancy.NewManager(&tenancy.Options{}), telset) require.Error(t, err) } @@ -341,12 +341,12 @@ func TestServerHTTPTLS(t *testing.T) { tlsGrpc = enabledTLSCfg } - serverOptions := &QueryOptions{ + serverOptions := &Options{ GRPCHostPort: ":0", HTTPHostPort: ":0", TLSHTTP: test.TLS, TLSGRPC: tlsGrpc, - QueryOptionsBase: QueryOptionsBase{ + OptionsBase: OptionsBase{ BearerTokenPropagation: true, }, } @@ -478,12 +478,12 @@ func TestServerGRPCTLS(t *testing.T) { if test.HTTPTLSEnabled { tlsHttp = enabledTLSCfg } - serverOptions := &QueryOptions{ + serverOptions := &Options{ GRPCHostPort: ":0", HTTPHostPort: ":0", TLSHTTP: tlsHttp, TLSGRPC: test.TLS, - QueryOptionsBase: QueryOptionsBase{ + OptionsBase: OptionsBase{ BearerTokenPropagation: true, }, } @@ -536,10 +536,10 @@ func TestServerGRPCTLS(t *testing.T) { func TestServerBadHostPort(t *testing.T) { telset := initTelSet(zaptest.NewLogger(t), jtracer.NoOp(), healthcheck.New()) _, err := NewServer(&querysvc.QueryService{}, nil, - &QueryOptions{ + &Options{ HTTPHostPort: "8080", // bad string, not :port GRPCHostPort: "127.0.0.1:8081", - QueryOptionsBase: QueryOptionsBase{ + OptionsBase: OptionsBase{ BearerTokenPropagation: true, }, }, @@ -548,10 +548,10 @@ func TestServerBadHostPort(t *testing.T) { require.Error(t, err) _, err = NewServer(&querysvc.QueryService{}, nil, - &QueryOptions{ + &Options{ HTTPHostPort: "127.0.0.1:8081", GRPCHostPort: "9123", // bad string, not :port - QueryOptionsBase: QueryOptionsBase{ + OptionsBase: OptionsBase{ BearerTokenPropagation: true, }, }, @@ -580,10 +580,10 @@ func TestServerInUseHostPort(t *testing.T) { server, err := NewServer( &querysvc.QueryService{}, nil, - &QueryOptions{ + &Options{ HTTPHostPort: tc.httpHostPort, GRPCHostPort: tc.grpcHostPort, - QueryOptionsBase: QueryOptionsBase{ + OptionsBase: OptionsBase{ BearerTokenPropagation: true, }, }, @@ -604,10 +604,10 @@ func TestServerSinglePort(t *testing.T) { querySvc := makeQuerySvc() telset := initTelSet(flagsSvc.Logger, jtracer.NoOp(), flagsSvc.HC()) server, err := NewServer(querySvc.qs, nil, - &QueryOptions{ + &Options{ GRPCHostPort: hostPort, HTTPHostPort: hostPort, - QueryOptionsBase: QueryOptionsBase{ + OptionsBase: OptionsBase{ BearerTokenPropagation: true, }, }, @@ -645,7 +645,7 @@ func TestServerGracefulExit(t *testing.T) { querySvc := makeQuerySvc() telset := initTelSet(flagsSvc.Logger, jtracer.NoOp(), flagsSvc.HC()) server, err := NewServer(querySvc.qs, nil, - &QueryOptions{GRPCHostPort: hostPort, HTTPHostPort: hostPort}, + &Options{GRPCHostPort: hostPort, HTTPHostPort: hostPort}, tenancy.NewManager(&tenancy.Options{}), telset) require.NoError(t, err) require.NoError(t, server.Start()) @@ -678,7 +678,7 @@ func TestServerHandlesPortZero(t *testing.T) { querySvc := &querysvc.QueryService{} telset := initTelSet(flagsSvc.Logger, jtracer.NoOp(), flagsSvc.HC()) server, err := NewServer(querySvc, nil, - &QueryOptions{GRPCHostPort: ":0", HTTPHostPort: ":0"}, + &Options{GRPCHostPort: ":0", HTTPHostPort: ":0"}, tenancy.NewManager(&tenancy.Options{}), telset) require.NoError(t, err) @@ -718,7 +718,7 @@ func TestServerHTTPTenancy(t *testing.T) { }, } - serverOptions := &QueryOptions{ + serverOptions := &Options{ HTTPHostPort: ":8080", GRPCHostPort: ":8080", Tenancy: tenancy.Options{ diff --git a/cmd/query/app/static_handler.go b/cmd/query/app/static_handler.go index 02faad69dce..bca274c4695 100644 --- a/cmd/query/app/static_handler.go +++ b/cmd/query/app/static_handler.go @@ -35,13 +35,13 @@ var ( ) // RegisterStaticHandler adds handler for static assets to the router. -func RegisterStaticHandler(r *mux.Router, logger *zap.Logger, qOpts *QueryOptions, qCapabilities querysvc.StorageCapabilities) io.Closer { - staticHandler, err := NewStaticAssetsHandler(qOpts.StaticAssets.Path, StaticAssetsHandlerOptions{ +func RegisterStaticHandler(r *mux.Router, logger *zap.Logger, qOpts *Options, qCapabilities querysvc.StorageCapabilities) io.Closer { + staticHandler, err := NewStaticAssetsHandler(qOpts.UI.AssetsPath, StaticAssetsHandlerOptions{ BasePath: qOpts.BasePath, - UIConfigPath: qOpts.UIConfig, + UIConfigPath: qOpts.UI.ConfigFile, StorageCapabilities: qCapabilities, Logger: logger, - LogAccess: qOpts.StaticAssets.LogAccess, + LogAccess: qOpts.UI.LogAccess, }) if err != nil { logger.Panic("Could not create static assets handler", zap.Error(err)) diff --git a/cmd/query/app/static_handler_test.go b/cmd/query/app/static_handler_test.go index f8ed15ce5a7..cd801b09227 100644 --- a/cmd/query/app/static_handler_test.go +++ b/cmd/query/app/static_handler_test.go @@ -43,10 +43,10 @@ func TestRegisterStaticHandlerPanic(t *testing.T) { closer := RegisterStaticHandler( mux.NewRouter(), logger, - &QueryOptions{ - QueryOptionsBase: QueryOptionsBase{ - StaticAssets: QueryOptionsStaticAssets{ - Path: "/foo/bar", + &Options{ + OptionsBase: OptionsBase{ + UI: UI{ + AssetsPath: "/foo/bar", }, }, }, @@ -110,14 +110,14 @@ func TestRegisterStaticHandler(t *testing.T) { if testCase.subroute { r = r.PathPrefix(testCase.basePath).Subrouter() } - closer := RegisterStaticHandler(r, logger, &QueryOptions{ - QueryOptionsBase: QueryOptionsBase{ - StaticAssets: QueryOptionsStaticAssets{ - Path: "fixture", - LogAccess: testCase.logAccess, + closer := RegisterStaticHandler(r, logger, &Options{ + OptionsBase: OptionsBase{ + UI: UI{ + ConfigFile: testCase.UIConfigPath, + AssetsPath: "fixture", + LogAccess: testCase.logAccess, }, BasePath: testCase.basePath, - UIConfig: testCase.UIConfigPath, }, }, querysvc.StorageCapabilities{ArchiveStorage: testCase.archiveStorage}, diff --git a/cmd/query/app/token_propagation_test.go b/cmd/query/app/token_propagation_test.go index b625f1d9f51..5d6ba95b94f 100644 --- a/cmd/query/app/token_propagation_test.go +++ b/cmd/query/app/token_propagation_test.go @@ -87,10 +87,10 @@ func runQueryService(t *testing.T, esURL string) *Server { ReportStatus: telemetery.HCAdapter(flagsSvc.HC()), } server, err := NewServer(querySvc, nil, - &QueryOptions{ + &Options{ GRPCHostPort: ":0", HTTPHostPort: ":0", - QueryOptionsBase: QueryOptionsBase{ + OptionsBase: OptionsBase{ BearerTokenPropagation: true, }, }, diff --git a/cmd/query/main.go b/cmd/query/main.go index 77f77b5e2b6..ed5d3c7abe8 100644 --- a/cmd/query/main.go +++ b/cmd/query/main.go @@ -64,7 +64,7 @@ func main() { metricsFactory := baseFactory.Namespace(metrics.NSOptions{Name: "query"}) version.NewInfoMetrics(metricsFactory) - queryOpts, err := new(app.QueryOptions).InitFromViper(v, logger) + queryOpts, err := new(app.Options).InitFromViper(v, logger) if err != nil { logger.Fatal("Failed to configure query service", zap.Error(err)) } From 597330c5d1ffd54b2f9fcac083d15cd02f336f58 Mon Sep 17 00:00:00 2001 From: Mahad Zaryab Date: Wed, 18 Sep 2024 21:43:59 -0600 Subject: [PATCH 07/18] Update Existing Configurations Signed-off-by: Mahad Zaryab --- cmd/jaeger/config-badger.yaml | 3 ++- cmd/jaeger/config-cassandra.yaml | 3 ++- cmd/jaeger/config-elasticsearch.yaml | 3 ++- cmd/jaeger/config-opensearch.yaml | 3 ++- cmd/jaeger/config-remote-storage.yaml | 3 ++- cmd/jaeger/config.yaml | 3 ++- 6 files changed, 12 insertions(+), 6 deletions(-) diff --git a/cmd/jaeger/config-badger.yaml b/cmd/jaeger/config-badger.yaml index 266d4d3cad1..fe6dc89da9f 100644 --- a/cmd/jaeger/config-badger.yaml +++ b/cmd/jaeger/config-badger.yaml @@ -15,7 +15,8 @@ extensions: storage: trace: some_store trace_archive: another_store - ui_config: ./cmd/jaeger/config-ui.json + ui: + config_file: ./cmd/jaeger/config-ui.json jaeger_storage: backends: diff --git a/cmd/jaeger/config-cassandra.yaml b/cmd/jaeger/config-cassandra.yaml index 108cacd0f1f..c132a12da59 100644 --- a/cmd/jaeger/config-cassandra.yaml +++ b/cmd/jaeger/config-cassandra.yaml @@ -15,7 +15,8 @@ extensions: storage: trace: some_storage trace_archive: another_storage - ui_config: ./cmd/jaeger/config-ui.json + ui: + config_file: ./cmd/jaeger/config-ui.json jaeger_storage: backends: diff --git a/cmd/jaeger/config-elasticsearch.yaml b/cmd/jaeger/config-elasticsearch.yaml index 42dc9d12cf4..f58d5b0b1e6 100644 --- a/cmd/jaeger/config-elasticsearch.yaml +++ b/cmd/jaeger/config-elasticsearch.yaml @@ -15,7 +15,8 @@ extensions: storage: trace: some_storage trace_archive: another_storage - ui_config: ./cmd/jaeger/config-ui.json + ui: + config_file: ./cmd/jaeger/config-ui.json jaeger_storage: backends: diff --git a/cmd/jaeger/config-opensearch.yaml b/cmd/jaeger/config-opensearch.yaml index e9946dd1ccd..6f4dfc4e0c6 100644 --- a/cmd/jaeger/config-opensearch.yaml +++ b/cmd/jaeger/config-opensearch.yaml @@ -15,7 +15,8 @@ extensions: storage: trace: some_storage trace_archive: another_storage - ui_config: ./cmd/jaeger/config-ui.json + ui: + config_file: ./cmd/jaeger/config-ui.json jaeger_storage: backends: diff --git a/cmd/jaeger/config-remote-storage.yaml b/cmd/jaeger/config-remote-storage.yaml index 89eef7a81ee..456169ad369 100644 --- a/cmd/jaeger/config-remote-storage.yaml +++ b/cmd/jaeger/config-remote-storage.yaml @@ -14,7 +14,8 @@ extensions: jaeger_query: storage: trace: some-storage - ui_config: ./cmd/jaeger/config-ui.json + ui: + config_file: ./cmd/jaeger/config-ui.json jaeger_storage: backends: diff --git a/cmd/jaeger/config.yaml b/cmd/jaeger/config.yaml index bf349920bf2..78a0ac89172 100644 --- a/cmd/jaeger/config.yaml +++ b/cmd/jaeger/config.yaml @@ -20,7 +20,8 @@ extensions: storage: trace: some_store trace_archive: another_store - ui_config: ./cmd/jaeger/config-ui.json + ui: + config_file: ./cmd/jaeger/config-ui.json jaeger_storage: backends: From c3e130d7a0821b34efeb0b9df9e77c2a90e3c495 Mon Sep 17 00:00:00 2001 From: Mahad Zaryab Date: Sun, 22 Sep 2024 14:58:15 -0400 Subject: [PATCH 08/18] Revert Struct Renaming And Move Tenancy To Base Config Signed-off-by: Mahad Zaryab --- cmd/all-in-one/main.go | 4 +- .../internal/extension/jaegerquery/config.go | 9 ++-- .../internal/extension/jaegerquery/factory.go | 2 +- .../internal/extension/jaegerquery/server.go | 8 ++-- cmd/query/app/flags.go | 18 ++++---- cmd/query/app/flags_test.go | 10 ++--- cmd/query/app/server.go | 8 ++-- cmd/query/app/server_test.go | 44 ++++++++++--------- cmd/query/app/static_handler.go | 2 +- cmd/query/app/static_handler_test.go | 8 ++-- cmd/query/app/token_propagation_test.go | 4 +- cmd/query/main.go | 2 +- 12 files changed, 59 insertions(+), 60 deletions(-) diff --git a/cmd/all-in-one/main.go b/cmd/all-in-one/main.go index 3a3dc0f838f..15c8c8cd5e2 100644 --- a/cmd/all-in-one/main.go +++ b/cmd/all-in-one/main.go @@ -142,7 +142,7 @@ by default uses only in-memory database.`, if err != nil { logger.Fatal("Failed to initialize collector", zap.Error(err)) } - qOpts, err := new(queryApp.Options).InitFromViper(v, logger) + qOpts, err := new(queryApp.QueryOptions).InitFromViper(v, logger) if err != nil { logger.Fatal("Failed to configure query service", zap.Error(err)) } @@ -263,7 +263,7 @@ func startAgent( func startQuery( svc *flags.Service, - qOpts *queryApp.Options, + qOpts *queryApp.QueryOptions, queryOpts *querysvc.QueryServiceOptions, spanReader spanstore.Reader, depReader dependencystore.Reader, diff --git a/cmd/jaeger/internal/extension/jaegerquery/config.go b/cmd/jaeger/internal/extension/jaegerquery/config.go index f6927325c37..2c955186393 100644 --- a/cmd/jaeger/internal/extension/jaegerquery/config.go +++ b/cmd/jaeger/internal/extension/jaegerquery/config.go @@ -10,21 +10,18 @@ import ( "go.opentelemetry.io/collector/config/confighttp" queryApp "github.com/jaegertracing/jaeger/cmd/query/app" - "github.com/jaegertracing/jaeger/pkg/tenancy" ) var _ component.ConfigValidator = (*Config)(nil) // Config represents the configuration for jaeger-query, type Config struct { - queryApp.OptionsBase `mapstructure:",squash"` - Connection Connection `mapstructure:"connection"` - Storage Storage `mapstructure:"storage"` + queryApp.QueryOptionsBase `mapstructure:",squash"` + Connection Connection `mapstructure:"connection"` + Storage Storage `mapstructure:"storage"` } type Connection struct { - // Tenancy holds the multi-tenancy configuration. - Tenancy tenancy.Options `mapstructure:"multi_tenancy"` // HTTP holds the HTTP configuration that the query service uses to serve requests. HTTP confighttp.ServerConfig `mapstructure:"http"` // GRPC holds the GRPC configuration that the query service uses to serve requests. diff --git a/cmd/jaeger/internal/extension/jaegerquery/factory.go b/cmd/jaeger/internal/extension/jaegerquery/factory.go index fcd047c2854..fa40ae1749a 100644 --- a/cmd/jaeger/internal/extension/jaegerquery/factory.go +++ b/cmd/jaeger/internal/extension/jaegerquery/factory.go @@ -28,7 +28,7 @@ func NewFactory() extension.Factory { func createDefaultConfig() component.Config { return &Config{ - OptionsBase: app.OptionsBase{ + QueryOptionsBase: app.QueryOptionsBase{ BasePath: "/", }, Connection: Connection{ diff --git a/cmd/jaeger/internal/extension/jaegerquery/server.go b/cmd/jaeger/internal/extension/jaegerquery/server.go index 3363cbf3e4a..f0eb4d6be89 100644 --- a/cmd/jaeger/internal/extension/jaegerquery/server.go +++ b/cmd/jaeger/internal/extension/jaegerquery/server.go @@ -83,7 +83,7 @@ func (s *server) Start(_ context.Context, host component.Host) error { return err } - tm := tenancy.NewManager(&s.config.Connection.Tenancy) + tm := tenancy.NewManager(&s.config.QueryOptionsBase.Tenancy) // TODO OTel-collector does not initialize the tracer currently // https://github.com/open-telemetry/opentelemetry-collector/issues/7532 @@ -158,9 +158,9 @@ func (s *server) createMetricReader(host component.Host) (metricsstore.Reader, e return metricsReader, err } -func (s *server) makeQueryOptions() *queryApp.Options { - return &queryApp.Options{ - OptionsBase: s.config.OptionsBase, +func (s *server) makeQueryOptions() *queryApp.QueryOptions { + return &queryApp.QueryOptions{ + QueryOptionsBase: s.config.QueryOptionsBase, // TODO utilize OTEL helpers for creating HTTP/GRPC servers HTTPHostPort: s.config.Connection.HTTP.Endpoint, diff --git a/cmd/query/app/flags.go b/cmd/query/app/flags.go index 4025f654bc3..45c0619fa86 100644 --- a/cmd/query/app/flags.go +++ b/cmd/query/app/flags.go @@ -57,26 +57,26 @@ type UI struct { LogAccess bool `mapstructure:"log_access" valid:"optional"` } -// OptionsBase holds configuration for query service shared with jaeger-v2 -type OptionsBase struct { +// QueryOptionsBase holds configuration for query service shared with jaeger-v2 +type QueryOptionsBase struct { // BasePath is the base path for all HTTP routes. BasePath string `mapstructure:"base_path"` // UI contains configuration related to the Jaeger UI. UI UI `mapstructure:"ui" valid:"optional"` // BearerTokenPropagation activate/deactivate bearer token propagation to storage. BearerTokenPropagation bool `mapstructure:"bearer_token_propagation"` + // Tenancy holds the multi-tenancy configuration. + Tenancy tenancy.Options `mapstructure:"multi_tenancy"` // MaxClockSkewAdjust is the maximum duration by which jaeger-query will adjust a span. MaxClockSkewAdjust time.Duration `mapstructure:"max_clock_skew_adjust"` // EnableTracing determines whether traces will be emitted by jaeger-query. EnableTracing bool `mapstructure:"enable_tracing"` } -// Options holds configuration for query service -type Options struct { - OptionsBase +// QueryOptions holds configuration for query service +type QueryOptions struct { + QueryOptionsBase - // Tenancy configures tenancy for query - Tenancy tenancy.Options // AdditionalHeaders AdditionalHeaders http.Header // HTTPHostPort is the host:port address that the query service listens in on for http requests @@ -106,7 +106,7 @@ func AddFlags(flagSet *flag.FlagSet) { } // InitFromViper initializes QueryOptions with properties from viper -func (qOpts *Options) InitFromViper(v *viper.Viper, logger *zap.Logger) (*Options, error) { +func (qOpts *QueryOptions) InitFromViper(v *viper.Viper, logger *zap.Logger) (*QueryOptions, error) { qOpts.HTTPHostPort = v.GetString(queryHTTPHostPort) qOpts.GRPCHostPort = v.GetString(queryGRPCHostPort) tlsGrpc, err := tlsGRPCFlagsConfig.InitFromViper(v) @@ -139,7 +139,7 @@ func (qOpts *Options) InitFromViper(v *viper.Viper, logger *zap.Logger) (*Option } // BuildQueryServiceOptions creates a QueryServiceOptions struct with appropriate adjusters and archive config -func (qOpts *Options) BuildQueryServiceOptions(storageFactory storage.Factory, logger *zap.Logger) *querysvc.QueryServiceOptions { +func (qOpts *QueryOptions) BuildQueryServiceOptions(storageFactory storage.Factory, logger *zap.Logger) *querysvc.QueryServiceOptions { opts := &querysvc.QueryServiceOptions{} if !opts.InitArchiveStorage(storageFactory, logger) { logger.Info("Archive storage not initialized") diff --git a/cmd/query/app/flags_test.go b/cmd/query/app/flags_test.go index 72351224b73..8fc83418b3d 100644 --- a/cmd/query/app/flags_test.go +++ b/cmd/query/app/flags_test.go @@ -33,7 +33,7 @@ func TestQueryBuilderFlags(t *testing.T) { "--query.additional-headers=whatever:thing", "--query.max-clock-skew-adjustment=10s", }) - qOpts, err := new(Options).InitFromViper(v, zap.NewNop()) + qOpts, err := new(QueryOptions).InitFromViper(v, zap.NewNop()) require.NoError(t, err) assert.Equal(t, "/dev/null", qOpts.UI.AssetsPath) assert.True(t, qOpts.UI.LogAccess) @@ -53,7 +53,7 @@ func TestQueryBuilderBadHeadersFlags(t *testing.T) { command.ParseFlags([]string{ "--query.additional-headers=malformedheader", }) - qOpts, err := new(Options).InitFromViper(v, zap.NewNop()) + qOpts, err := new(QueryOptions).InitFromViper(v, zap.NewNop()) require.NoError(t, err) assert.Nil(t, qOpts.AdditionalHeaders) } @@ -87,7 +87,7 @@ func TestStringSliceAsHeader(t *testing.T) { func TestBuildQueryServiceOptions(t *testing.T) { v, _ := config.Viperize(AddFlags) - qOpts, err := new(Options).InitFromViper(v, zap.NewNop()) + qOpts, err := new(QueryOptions).InitFromViper(v, zap.NewNop()) require.NoError(t, err) assert.NotNil(t, qOpts) @@ -158,7 +158,7 @@ func TestQueryOptionsPortAllocationFromFlags(t *testing.T) { t.Run(test.name, func(t *testing.T) { v, command := config.Viperize(AddFlags) command.ParseFlags(test.flagsArray) - qOpts, err := new(Options).InitFromViper(v, zap.NewNop()) + qOpts, err := new(QueryOptions).InitFromViper(v, zap.NewNop()) require.NoError(t, err) assert.Equal(t, test.expectedHTTPHostPort, qOpts.HTTPHostPort) @@ -177,7 +177,7 @@ func TestQueryOptions_FailedTLSFlags(t *testing.T) { "--query." + proto + ".tls.cert=blah", // invalid unless tls.enabled }) require.NoError(t, err) - _, err = new(Options).InitFromViper(v, zap.NewNop()) + _, err = new(QueryOptions).InitFromViper(v, zap.NewNop()) require.Error(t, err) assert.Contains(t, err.Error(), "failed to process "+test+" TLS options") }) diff --git a/cmd/query/app/server.go b/cmd/query/app/server.go index 5f0d5a4aaca..1e61b0cd09d 100644 --- a/cmd/query/app/server.go +++ b/cmd/query/app/server.go @@ -39,7 +39,7 @@ import ( // Server runs HTTP, Mux and a grpc server type Server struct { querySvc *querysvc.QueryService - queryOptions *Options + queryOptions *QueryOptions conn net.Listener grpcConn net.Listener @@ -55,7 +55,7 @@ type Server struct { // NewServer creates and initializes Server func NewServer(querySvc *querysvc.QueryService, metricsQuerySvc querysvc.MetricsQueryService, - options *Options, + options *QueryOptions, tm *tenancy.Manager, telset telemetery.Setting, ) (*Server, error) { @@ -93,7 +93,7 @@ func NewServer(querySvc *querysvc.QueryService, }, nil } -func createGRPCServer(querySvc *querysvc.QueryService, metricsQuerySvc querysvc.MetricsQueryService, options *Options, tm *tenancy.Manager, telset telemetery.Setting) (*grpc.Server, error) { +func createGRPCServer(querySvc *querysvc.QueryService, metricsQuerySvc querysvc.MetricsQueryService, options *QueryOptions, tm *tenancy.Manager, telset telemetery.Setting) (*grpc.Server, error) { var grpcOpts []grpc.ServerOption if options.TLSGRPC.Enabled { @@ -143,7 +143,7 @@ var _ io.Closer = (*httpServer)(nil) func createHTTPServer( querySvc *querysvc.QueryService, metricsQuerySvc querysvc.MetricsQueryService, - queryOpts *Options, + queryOpts *QueryOptions, tm *tenancy.Manager, telset telemetery.Setting, ) (*httpServer, error) { diff --git a/cmd/query/app/server_test.go b/cmd/query/app/server_test.go index 8d7d567728a..b028dfbaed7 100644 --- a/cmd/query/app/server_test.go +++ b/cmd/query/app/server_test.go @@ -49,7 +49,7 @@ func initTelSet(logger *zap.Logger, tracerProvider *jtracer.JTracer, hc *healthc func TestServerError(t *testing.T) { srv := &Server{ - queryOptions: &Options{ + queryOptions: &QueryOptions{ HTTPHostPort: ":-1", }, } @@ -66,7 +66,7 @@ func TestCreateTLSServerSinglePortError(t *testing.T) { } telset := initTelSet(zaptest.NewLogger(t), jtracer.NoOp(), healthcheck.New()) _, err := NewServer(&querysvc.QueryService{}, nil, - &Options{HTTPHostPort: ":8080", GRPCHostPort: ":8080", TLSGRPC: tlsCfg, TLSHTTP: tlsCfg}, + &QueryOptions{HTTPHostPort: ":8080", GRPCHostPort: ":8080", TLSGRPC: tlsCfg, TLSHTTP: tlsCfg}, tenancy.NewManager(&tenancy.Options{}), telset) require.Error(t, err) } @@ -80,7 +80,7 @@ func TestCreateTLSGrpcServerError(t *testing.T) { } telset := initTelSet(zaptest.NewLogger(t), jtracer.NoOp(), healthcheck.New()) _, err := NewServer(&querysvc.QueryService{}, nil, - &Options{HTTPHostPort: ":8080", GRPCHostPort: ":8081", TLSGRPC: tlsCfg}, + &QueryOptions{HTTPHostPort: ":8080", GRPCHostPort: ":8081", TLSGRPC: tlsCfg}, tenancy.NewManager(&tenancy.Options{}), telset) require.Error(t, err) } @@ -94,7 +94,7 @@ func TestCreateTLSHttpServerError(t *testing.T) { } telset := initTelSet(zaptest.NewLogger(t), jtracer.NoOp(), healthcheck.New()) _, err := NewServer(&querysvc.QueryService{}, nil, - &Options{HTTPHostPort: ":8080", GRPCHostPort: ":8081", TLSHTTP: tlsCfg}, + &QueryOptions{HTTPHostPort: ":8080", GRPCHostPort: ":8081", TLSHTTP: tlsCfg}, tenancy.NewManager(&tenancy.Options{}), telset) require.Error(t, err) } @@ -341,12 +341,12 @@ func TestServerHTTPTLS(t *testing.T) { tlsGrpc = enabledTLSCfg } - serverOptions := &Options{ + serverOptions := &QueryOptions{ GRPCHostPort: ":0", HTTPHostPort: ":0", TLSHTTP: test.TLS, TLSGRPC: tlsGrpc, - OptionsBase: OptionsBase{ + QueryOptionsBase: QueryOptionsBase{ BearerTokenPropagation: true, }, } @@ -478,12 +478,12 @@ func TestServerGRPCTLS(t *testing.T) { if test.HTTPTLSEnabled { tlsHttp = enabledTLSCfg } - serverOptions := &Options{ + serverOptions := &QueryOptions{ GRPCHostPort: ":0", HTTPHostPort: ":0", TLSHTTP: tlsHttp, TLSGRPC: test.TLS, - OptionsBase: OptionsBase{ + QueryOptionsBase: QueryOptionsBase{ BearerTokenPropagation: true, }, } @@ -536,10 +536,10 @@ func TestServerGRPCTLS(t *testing.T) { func TestServerBadHostPort(t *testing.T) { telset := initTelSet(zaptest.NewLogger(t), jtracer.NoOp(), healthcheck.New()) _, err := NewServer(&querysvc.QueryService{}, nil, - &Options{ + &QueryOptions{ HTTPHostPort: "8080", // bad string, not :port GRPCHostPort: "127.0.0.1:8081", - OptionsBase: OptionsBase{ + QueryOptionsBase: QueryOptionsBase{ BearerTokenPropagation: true, }, }, @@ -548,10 +548,10 @@ func TestServerBadHostPort(t *testing.T) { require.Error(t, err) _, err = NewServer(&querysvc.QueryService{}, nil, - &Options{ + &QueryOptions{ HTTPHostPort: "127.0.0.1:8081", GRPCHostPort: "9123", // bad string, not :port - OptionsBase: OptionsBase{ + QueryOptionsBase: QueryOptionsBase{ BearerTokenPropagation: true, }, }, @@ -580,10 +580,10 @@ func TestServerInUseHostPort(t *testing.T) { server, err := NewServer( &querysvc.QueryService{}, nil, - &Options{ + &QueryOptions{ HTTPHostPort: tc.httpHostPort, GRPCHostPort: tc.grpcHostPort, - OptionsBase: OptionsBase{ + QueryOptionsBase: QueryOptionsBase{ BearerTokenPropagation: true, }, }, @@ -604,10 +604,10 @@ func TestServerSinglePort(t *testing.T) { querySvc := makeQuerySvc() telset := initTelSet(flagsSvc.Logger, jtracer.NoOp(), flagsSvc.HC()) server, err := NewServer(querySvc.qs, nil, - &Options{ + &QueryOptions{ GRPCHostPort: hostPort, HTTPHostPort: hostPort, - OptionsBase: OptionsBase{ + QueryOptionsBase: QueryOptionsBase{ BearerTokenPropagation: true, }, }, @@ -645,7 +645,7 @@ func TestServerGracefulExit(t *testing.T) { querySvc := makeQuerySvc() telset := initTelSet(flagsSvc.Logger, jtracer.NoOp(), flagsSvc.HC()) server, err := NewServer(querySvc.qs, nil, - &Options{GRPCHostPort: hostPort, HTTPHostPort: hostPort}, + &QueryOptions{GRPCHostPort: hostPort, HTTPHostPort: hostPort}, tenancy.NewManager(&tenancy.Options{}), telset) require.NoError(t, err) require.NoError(t, server.Start()) @@ -678,7 +678,7 @@ func TestServerHandlesPortZero(t *testing.T) { querySvc := &querysvc.QueryService{} telset := initTelSet(flagsSvc.Logger, jtracer.NoOp(), flagsSvc.HC()) server, err := NewServer(querySvc, nil, - &Options{GRPCHostPort: ":0", HTTPHostPort: ":0"}, + &QueryOptions{GRPCHostPort: ":0", HTTPHostPort: ":0"}, tenancy.NewManager(&tenancy.Options{}), telset) require.NoError(t, err) @@ -718,11 +718,13 @@ func TestServerHTTPTenancy(t *testing.T) { }, } - serverOptions := &Options{ + serverOptions := &QueryOptions{ HTTPHostPort: ":8080", GRPCHostPort: ":8080", - Tenancy: tenancy.Options{ - Enabled: true, + QueryOptionsBase: QueryOptionsBase{ + Tenancy: tenancy.Options{ + Enabled: true, + }, }, } tenancyMgr := tenancy.NewManager(&serverOptions.Tenancy) diff --git a/cmd/query/app/static_handler.go b/cmd/query/app/static_handler.go index bca274c4695..82f8db87eed 100644 --- a/cmd/query/app/static_handler.go +++ b/cmd/query/app/static_handler.go @@ -35,7 +35,7 @@ var ( ) // RegisterStaticHandler adds handler for static assets to the router. -func RegisterStaticHandler(r *mux.Router, logger *zap.Logger, qOpts *Options, qCapabilities querysvc.StorageCapabilities) io.Closer { +func RegisterStaticHandler(r *mux.Router, logger *zap.Logger, qOpts *QueryOptions, qCapabilities querysvc.StorageCapabilities) io.Closer { staticHandler, err := NewStaticAssetsHandler(qOpts.UI.AssetsPath, StaticAssetsHandlerOptions{ BasePath: qOpts.BasePath, UIConfigPath: qOpts.UI.ConfigFile, diff --git a/cmd/query/app/static_handler_test.go b/cmd/query/app/static_handler_test.go index cd801b09227..d53944f56b3 100644 --- a/cmd/query/app/static_handler_test.go +++ b/cmd/query/app/static_handler_test.go @@ -43,8 +43,8 @@ func TestRegisterStaticHandlerPanic(t *testing.T) { closer := RegisterStaticHandler( mux.NewRouter(), logger, - &Options{ - OptionsBase: OptionsBase{ + &QueryOptions{ + QueryOptionsBase: QueryOptionsBase{ UI: UI{ AssetsPath: "/foo/bar", }, @@ -110,8 +110,8 @@ func TestRegisterStaticHandler(t *testing.T) { if testCase.subroute { r = r.PathPrefix(testCase.basePath).Subrouter() } - closer := RegisterStaticHandler(r, logger, &Options{ - OptionsBase: OptionsBase{ + closer := RegisterStaticHandler(r, logger, &QueryOptions{ + QueryOptionsBase: QueryOptionsBase{ UI: UI{ ConfigFile: testCase.UIConfigPath, AssetsPath: "fixture", diff --git a/cmd/query/app/token_propagation_test.go b/cmd/query/app/token_propagation_test.go index 5d6ba95b94f..b625f1d9f51 100644 --- a/cmd/query/app/token_propagation_test.go +++ b/cmd/query/app/token_propagation_test.go @@ -87,10 +87,10 @@ func runQueryService(t *testing.T, esURL string) *Server { ReportStatus: telemetery.HCAdapter(flagsSvc.HC()), } server, err := NewServer(querySvc, nil, - &Options{ + &QueryOptions{ GRPCHostPort: ":0", HTTPHostPort: ":0", - OptionsBase: OptionsBase{ + QueryOptionsBase: QueryOptionsBase{ BearerTokenPropagation: true, }, }, diff --git a/cmd/query/main.go b/cmd/query/main.go index ed5d3c7abe8..77f77b5e2b6 100644 --- a/cmd/query/main.go +++ b/cmd/query/main.go @@ -64,7 +64,7 @@ func main() { metricsFactory := baseFactory.Namespace(metrics.NSOptions{Name: "query"}) version.NewInfoMetrics(metricsFactory) - queryOpts, err := new(app.Options).InitFromViper(v, logger) + queryOpts, err := new(app.QueryOptions).InitFromViper(v, logger) if err != nil { logger.Fatal("Failed to configure query service", zap.Error(err)) } From 5f70fd77d81ce2f575afd50c13acbcf0e903eacb Mon Sep 17 00:00:00 2001 From: Mahad Zaryab Date: Sun, 22 Sep 2024 17:00:23 -0400 Subject: [PATCH 09/18] Address Feedback From PR Review Signed-off-by: Mahad Zaryab --- .../internal/extension/jaegerquery/config.go | 8 +++--- .../extension/jaegerquery/config_test.go | 2 +- .../internal/extension/jaegerquery/server.go | 8 +++--- .../extension/jaegerquery/server_test.go | 26 +++++++++---------- cmd/query/app/flags.go | 14 +++++----- cmd/query/app/flags_test.go | 6 ++--- cmd/query/app/static_handler.go | 6 ++--- cmd/query/app/static_handler_test.go | 4 +-- 8 files changed, 37 insertions(+), 37 deletions(-) diff --git a/cmd/jaeger/internal/extension/jaegerquery/config.go b/cmd/jaeger/internal/extension/jaegerquery/config.go index 2c955186393..de24f4d4c70 100644 --- a/cmd/jaeger/internal/extension/jaegerquery/config.go +++ b/cmd/jaeger/internal/extension/jaegerquery/config.go @@ -29,12 +29,12 @@ type Connection struct { } type Storage struct { - // TracePrimary contains the name of the primary trace storage that is being queried. - TracePrimary string `mapstructure:"trace" valid:"required"` + // TracesPrimary contains the name of the primary trace storage that is being queried. + TracesPrimary string `mapstructure:"traces" valid:"required"` // TraceArchive contains the name of the archive trace storage that is being queried. TraceArchive string `mapstructure:"trace_archive" valid:"optional"` - // Metric contains the name of the metric storage that is being queried. - Metric string `mapstructure:"metric" valid:"optional"` + // Metrics contains the name of the metric storage that is being queried. + Metrics string `mapstructure:"metrics" valid:"optional"` } func (cfg *Config) Validate() error { diff --git a/cmd/jaeger/internal/extension/jaegerquery/config_test.go b/cmd/jaeger/internal/extension/jaegerquery/config_test.go index c438906de30..43e0c66caa2 100644 --- a/cmd/jaeger/internal/extension/jaegerquery/config_test.go +++ b/cmd/jaeger/internal/extension/jaegerquery/config_test.go @@ -25,7 +25,7 @@ func Test_Validate(t *testing.T) { name: "Non empty-config", config: &Config{ Storage: Storage{ - TracePrimary: "some-storage", + TracesPrimary: "some-storage", }, }, expectedErr: "", diff --git a/cmd/jaeger/internal/extension/jaegerquery/server.go b/cmd/jaeger/internal/extension/jaegerquery/server.go index f0eb4d6be89..895c1bd5a2a 100644 --- a/cmd/jaeger/internal/extension/jaegerquery/server.go +++ b/cmd/jaeger/internal/extension/jaegerquery/server.go @@ -55,9 +55,9 @@ func (s *server) Start(_ context.Context, host component.Host) error { mf := otelmetrics.NewFactory(s.telset.MeterProvider) baseFactory := mf.Namespace(metrics.NSOptions{Name: "jaeger"}) queryMetricsFactory := baseFactory.Namespace(metrics.NSOptions{Name: "query"}) - f, err := jaegerstorage.GetStorageFactory(s.config.Storage.TracePrimary, host) + f, err := jaegerstorage.GetStorageFactory(s.config.Storage.TracesPrimary, host) if err != nil { - return fmt.Errorf("cannot find primary storage %s: %w", s.config.Storage.TracePrimary, err) + return fmt.Errorf("cannot find primary storage %s: %w", s.config.Storage.TracesPrimary, err) } spanReader, err := f.CreateSpanReader() @@ -141,12 +141,12 @@ func (s *server) addArchiveStorage(opts *querysvc.QueryServiceOptions, host comp } func (s *server) createMetricReader(host component.Host) (metricsstore.Reader, error) { - if s.config.Storage.Metric == "" { + if s.config.Storage.Metrics == "" { s.telset.Logger.Info("Metric storage not configured") return disabled.NewMetricsReader() } - mf, err := jaegerstorage.GetMetricsFactory(s.config.Storage.Metric, host) + mf, err := jaegerstorage.GetMetricsFactory(s.config.Storage.Metrics, host) if err != nil { return nil, fmt.Errorf("cannot find metrics storage factory: %w", err) } diff --git a/cmd/jaeger/internal/extension/jaegerquery/server_test.go b/cmd/jaeger/internal/extension/jaegerquery/server_test.go index dacb0b27f0e..224de60f1c4 100644 --- a/cmd/jaeger/internal/extension/jaegerquery/server_test.go +++ b/cmd/jaeger/internal/extension/jaegerquery/server_test.go @@ -135,9 +135,9 @@ func TestServerStart(t *testing.T) { name: "Non-empty config with fake storage host", config: &Config{ Storage: Storage{ - TraceArchive: "jaeger_storage", - TracePrimary: "jaeger_storage", - Metric: "jaeger_metrics_storage", + TraceArchive: "jaeger_storage", + TracesPrimary: "jaeger_storage", + Metrics: "jaeger_metrics_storage", }, }, }, @@ -145,7 +145,7 @@ func TestServerStart(t *testing.T) { name: "factory error", config: &Config{ Storage: Storage{ - TracePrimary: "need-factory-error", + TracesPrimary: "need-factory-error", }, }, expectedErr: "cannot find primary storage", @@ -154,7 +154,7 @@ func TestServerStart(t *testing.T) { name: "span reader error", config: &Config{ Storage: Storage{ - TracePrimary: "need-span-reader-error", + TracesPrimary: "need-span-reader-error", }, }, expectedErr: "cannot create span reader", @@ -163,7 +163,7 @@ func TestServerStart(t *testing.T) { name: "dependency error", config: &Config{ Storage: Storage{ - TracePrimary: "need-dependency-reader-error", + TracesPrimary: "need-dependency-reader-error", }, }, expectedErr: "cannot create dependencies reader", @@ -172,8 +172,8 @@ func TestServerStart(t *testing.T) { name: "storage archive error", config: &Config{ Storage: Storage{ - TraceArchive: "need-factory-error", - TracePrimary: "jaeger_storage", + TraceArchive: "need-factory-error", + TracesPrimary: "jaeger_storage", }, }, expectedErr: "cannot find archive storage factory", @@ -182,8 +182,8 @@ func TestServerStart(t *testing.T) { name: "metrics storage error", config: &Config{ Storage: Storage{ - Metric: "need-factory-error", - TracePrimary: "jaeger_storage", + Metrics: "need-factory-error", + TracesPrimary: "jaeger_storage", }, }, expectedErr: "cannot find metrics storage factory", @@ -192,8 +192,8 @@ func TestServerStart(t *testing.T) { name: " metrics reader error", config: &Config{ Storage: Storage{ - Metric: "need-metrics-reader-error", - TracePrimary: "jaeger_storage", + Metrics: "need-metrics-reader-error", + TracesPrimary: "jaeger_storage", }, }, expectedErr: "cannot create metrics reader", @@ -332,7 +332,7 @@ func TestServerAddMetricsStorage(t *testing.T) { name: "Metrics storage set", config: &Config{ Storage: Storage{ - Metric: "random-value", + Metrics: "random-value", }, }, expectedOutput: "", diff --git a/cmd/query/app/flags.go b/cmd/query/app/flags.go index 45c0619fa86..9629a36eba5 100644 --- a/cmd/query/app/flags.go +++ b/cmd/query/app/flags.go @@ -48,7 +48,7 @@ var tlsHTTPFlagsConfig = tlscfg.ServerFlagsConfig{ Prefix: "query.http", } -type UI struct { +type UIConfig struct { // ConfigFile is the path to a configuration file for the UI. ConfigFile string `mapstructure:"config_file" valid:"optional"` // AssetsPath is the path for the static assets for the UI (https://github.com/uber/jaeger-ui). @@ -61,14 +61,14 @@ type UI struct { type QueryOptionsBase struct { // BasePath is the base path for all HTTP routes. BasePath string `mapstructure:"base_path"` - // UI contains configuration related to the Jaeger UI. - UI UI `mapstructure:"ui" valid:"optional"` + // UIConfig contains configuration related to the Jaeger UIConfig. + UIConfig UIConfig `mapstructure:"ui"` // BearerTokenPropagation activate/deactivate bearer token propagation to storage. BearerTokenPropagation bool `mapstructure:"bearer_token_propagation"` // Tenancy holds the multi-tenancy configuration. Tenancy tenancy.Options `mapstructure:"multi_tenancy"` // MaxClockSkewAdjust is the maximum duration by which jaeger-query will adjust a span. - MaxClockSkewAdjust time.Duration `mapstructure:"max_clock_skew_adjust"` + MaxClockSkewAdjust time.Duration `mapstructure:"max_clock_skew_adjust" valid:"optional"` // EnableTracing determines whether traces will be emitted by jaeger-query. EnableTracing bool `mapstructure:"enable_tracing"` } @@ -120,9 +120,9 @@ func (qOpts *QueryOptions) InitFromViper(v *viper.Viper, logger *zap.Logger) (*Q } qOpts.TLSHTTP = tlsHTTP qOpts.BasePath = v.GetString(queryBasePath) - qOpts.UI.AssetsPath = v.GetString(queryStaticFiles) - qOpts.UI.LogAccess = v.GetBool(queryLogStaticAssetsAccess) - qOpts.UI.ConfigFile = v.GetString(queryUIConfig) + qOpts.UIConfig.AssetsPath = v.GetString(queryStaticFiles) + qOpts.UIConfig.LogAccess = v.GetBool(queryLogStaticAssetsAccess) + qOpts.UIConfig.ConfigFile = v.GetString(queryUIConfig) qOpts.BearerTokenPropagation = v.GetBool(queryTokenPropagation) qOpts.MaxClockSkewAdjust = v.GetDuration(queryMaxClockSkewAdjust) diff --git a/cmd/query/app/flags_test.go b/cmd/query/app/flags_test.go index 8fc83418b3d..c5d9ae8b0ec 100644 --- a/cmd/query/app/flags_test.go +++ b/cmd/query/app/flags_test.go @@ -35,9 +35,9 @@ func TestQueryBuilderFlags(t *testing.T) { }) qOpts, err := new(QueryOptions).InitFromViper(v, zap.NewNop()) require.NoError(t, err) - assert.Equal(t, "/dev/null", qOpts.UI.AssetsPath) - assert.True(t, qOpts.UI.LogAccess) - assert.Equal(t, "some.json", qOpts.UI.ConfigFile) + assert.Equal(t, "/dev/null", qOpts.UIConfig.AssetsPath) + assert.True(t, qOpts.UIConfig.LogAccess) + assert.Equal(t, "some.json", qOpts.UIConfig.ConfigFile) assert.Equal(t, "/jaeger", qOpts.BasePath) assert.Equal(t, "127.0.0.1:8080", qOpts.HTTPHostPort) assert.Equal(t, "127.0.0.1:8081", qOpts.GRPCHostPort) diff --git a/cmd/query/app/static_handler.go b/cmd/query/app/static_handler.go index 82f8db87eed..89dffe606c6 100644 --- a/cmd/query/app/static_handler.go +++ b/cmd/query/app/static_handler.go @@ -36,12 +36,12 @@ var ( // RegisterStaticHandler adds handler for static assets to the router. func RegisterStaticHandler(r *mux.Router, logger *zap.Logger, qOpts *QueryOptions, qCapabilities querysvc.StorageCapabilities) io.Closer { - staticHandler, err := NewStaticAssetsHandler(qOpts.UI.AssetsPath, StaticAssetsHandlerOptions{ + staticHandler, err := NewStaticAssetsHandler(qOpts.UIConfig.AssetsPath, StaticAssetsHandlerOptions{ BasePath: qOpts.BasePath, - UIConfigPath: qOpts.UI.ConfigFile, + UIConfigPath: qOpts.UIConfig.ConfigFile, StorageCapabilities: qCapabilities, Logger: logger, - LogAccess: qOpts.UI.LogAccess, + LogAccess: qOpts.UIConfig.LogAccess, }) if err != nil { logger.Panic("Could not create static assets handler", zap.Error(err)) diff --git a/cmd/query/app/static_handler_test.go b/cmd/query/app/static_handler_test.go index d53944f56b3..7f40cc4cc60 100644 --- a/cmd/query/app/static_handler_test.go +++ b/cmd/query/app/static_handler_test.go @@ -45,7 +45,7 @@ func TestRegisterStaticHandlerPanic(t *testing.T) { logger, &QueryOptions{ QueryOptionsBase: QueryOptionsBase{ - UI: UI{ + UIConfig: UIConfig{ AssetsPath: "/foo/bar", }, }, @@ -112,7 +112,7 @@ func TestRegisterStaticHandler(t *testing.T) { } closer := RegisterStaticHandler(r, logger, &QueryOptions{ QueryOptionsBase: QueryOptionsBase{ - UI: UI{ + UIConfig: UIConfig{ ConfigFile: testCase.UIConfigPath, AssetsPath: "fixture", LogAccess: testCase.logAccess, From 5351df1fa737eabc505ba8e42191547bdba6e6ca Mon Sep 17 00:00:00 2001 From: Mahad Zaryab Date: Sun, 22 Sep 2024 17:04:33 -0400 Subject: [PATCH 10/18] Use New Names In Config Files Signed-off-by: Mahad Zaryab --- cmd/jaeger/config-badger.yaml | 2 +- cmd/jaeger/config-kafka-ingester.yaml | 2 +- cmd/jaeger/config-opensearch.yaml | 4 ++-- cmd/jaeger/config-remote-storage.yaml | 2 +- cmd/jaeger/config-tail-sampling-always-sample.yaml | 2 +- cmd/jaeger/config-tail-sampling-service-name-policy.yaml | 2 +- cmd/jaeger/config.yaml | 2 +- cmd/jaeger/internal/all-in-one.yaml | 2 +- cmd/jaeger/internal/extension/jaegerquery/config.go | 4 ++-- cmd/jaeger/internal/extension/jaegerquery/server.go | 4 ++-- cmd/jaeger/internal/extension/jaegerquery/server_test.go | 8 ++++---- docker-compose/monitor/jaeger-v2-config.yml | 4 ++-- docker-compose/tail-sampling/jaeger-v2-config.yml | 2 +- 13 files changed, 20 insertions(+), 20 deletions(-) diff --git a/cmd/jaeger/config-badger.yaml b/cmd/jaeger/config-badger.yaml index fe6dc89da9f..01537047696 100644 --- a/cmd/jaeger/config-badger.yaml +++ b/cmd/jaeger/config-badger.yaml @@ -13,7 +13,7 @@ extensions: jaeger_query: storage: - trace: some_store + traces: some_store trace_archive: another_store ui: config_file: ./cmd/jaeger/config-ui.json diff --git a/cmd/jaeger/config-kafka-ingester.yaml b/cmd/jaeger/config-kafka-ingester.yaml index 0bfe9c64fd4..34412849cb5 100644 --- a/cmd/jaeger/config-kafka-ingester.yaml +++ b/cmd/jaeger/config-kafka-ingester.yaml @@ -20,7 +20,7 @@ extensions: jaeger_query: storage: - trace: some_storage + traces: some_storage jaeger_storage: backends: diff --git a/cmd/jaeger/config-opensearch.yaml b/cmd/jaeger/config-opensearch.yaml index 6f4dfc4e0c6..f4d3085179f 100644 --- a/cmd/jaeger/config-opensearch.yaml +++ b/cmd/jaeger/config-opensearch.yaml @@ -13,8 +13,8 @@ extensions: jaeger_query: storage: - trace: some_storage - trace_archive: another_storage + traces: some_storage + traces_archive: another_storage ui: config_file: ./cmd/jaeger/config-ui.json diff --git a/cmd/jaeger/config-remote-storage.yaml b/cmd/jaeger/config-remote-storage.yaml index 456169ad369..1f5cc699df5 100644 --- a/cmd/jaeger/config-remote-storage.yaml +++ b/cmd/jaeger/config-remote-storage.yaml @@ -13,7 +13,7 @@ extensions: jaeger_query: storage: - trace: some-storage + traces: some-storage ui: config_file: ./cmd/jaeger/config-ui.json diff --git a/cmd/jaeger/config-tail-sampling-always-sample.yaml b/cmd/jaeger/config-tail-sampling-always-sample.yaml index 19516b0da81..9f0f238f52b 100644 --- a/cmd/jaeger/config-tail-sampling-always-sample.yaml +++ b/cmd/jaeger/config-tail-sampling-always-sample.yaml @@ -15,7 +15,7 @@ extensions: http: jaeger_query: storage: - trace: some_storage + traces: some_storage jaeger_storage: backends: some_storage: diff --git a/cmd/jaeger/config-tail-sampling-service-name-policy.yaml b/cmd/jaeger/config-tail-sampling-service-name-policy.yaml index 125b33a082a..4b7ffa1fa0a 100644 --- a/cmd/jaeger/config-tail-sampling-service-name-policy.yaml +++ b/cmd/jaeger/config-tail-sampling-service-name-policy.yaml @@ -15,7 +15,7 @@ extensions: http: jaeger_query: storage: - trace: some_storage + traces: some_storage jaeger_storage: backends: some_storage: diff --git a/cmd/jaeger/config.yaml b/cmd/jaeger/config.yaml index 78a0ac89172..4ad21942a27 100644 --- a/cmd/jaeger/config.yaml +++ b/cmd/jaeger/config.yaml @@ -19,7 +19,7 @@ extensions: jaeger_query: storage: trace: some_store - trace_archive: another_store + traces_archive: another_store ui: config_file: ./cmd/jaeger/config-ui.json diff --git a/cmd/jaeger/internal/all-in-one.yaml b/cmd/jaeger/internal/all-in-one.yaml index fd15fc05f44..bc5433b67c5 100644 --- a/cmd/jaeger/internal/all-in-one.yaml +++ b/cmd/jaeger/internal/all-in-one.yaml @@ -17,7 +17,7 @@ service: extensions: jaeger_query: storage: - trace: some_storage + traces: some_storage jaeger_storage: backends: diff --git a/cmd/jaeger/internal/extension/jaegerquery/config.go b/cmd/jaeger/internal/extension/jaegerquery/config.go index de24f4d4c70..c6d63c4c804 100644 --- a/cmd/jaeger/internal/extension/jaegerquery/config.go +++ b/cmd/jaeger/internal/extension/jaegerquery/config.go @@ -31,8 +31,8 @@ type Connection struct { type Storage struct { // TracesPrimary contains the name of the primary trace storage that is being queried. TracesPrimary string `mapstructure:"traces" valid:"required"` - // TraceArchive contains the name of the archive trace storage that is being queried. - TraceArchive string `mapstructure:"trace_archive" valid:"optional"` + // TracesArchive contains the name of the archive trace storage that is being queried. + TracesArchive string `mapstructure:"traces_archive" valid:"optional"` // Metrics contains the name of the metric storage that is being queried. Metrics string `mapstructure:"metrics" valid:"optional"` } diff --git a/cmd/jaeger/internal/extension/jaegerquery/server.go b/cmd/jaeger/internal/extension/jaegerquery/server.go index 895c1bd5a2a..c8f4c3f33db 100644 --- a/cmd/jaeger/internal/extension/jaegerquery/server.go +++ b/cmd/jaeger/internal/extension/jaegerquery/server.go @@ -124,12 +124,12 @@ func (s *server) Start(_ context.Context, host component.Host) error { } func (s *server) addArchiveStorage(opts *querysvc.QueryServiceOptions, host component.Host) error { - if s.config.Storage.TraceArchive == "" { + if s.config.Storage.TracesArchive == "" { s.telset.Logger.Info("Archive storage not configured") return nil } - f, err := jaegerstorage.GetStorageFactory(s.config.Storage.TraceArchive, host) + f, err := jaegerstorage.GetStorageFactory(s.config.Storage.TracesArchive, host) if err != nil { return fmt.Errorf("cannot find archive storage factory: %w", err) } diff --git a/cmd/jaeger/internal/extension/jaegerquery/server_test.go b/cmd/jaeger/internal/extension/jaegerquery/server_test.go index 224de60f1c4..5484ca5c3d4 100644 --- a/cmd/jaeger/internal/extension/jaegerquery/server_test.go +++ b/cmd/jaeger/internal/extension/jaegerquery/server_test.go @@ -135,7 +135,7 @@ func TestServerStart(t *testing.T) { name: "Non-empty config with fake storage host", config: &Config{ Storage: Storage{ - TraceArchive: "jaeger_storage", + TracesArchive: "jaeger_storage", TracesPrimary: "jaeger_storage", Metrics: "jaeger_metrics_storage", }, @@ -172,7 +172,7 @@ func TestServerStart(t *testing.T) { name: "storage archive error", config: &Config{ Storage: Storage{ - TraceArchive: "need-factory-error", + TracesArchive: "need-factory-error", TracesPrimary: "jaeger_storage", }, }, @@ -269,7 +269,7 @@ func TestServerAddArchiveStorage(t *testing.T) { name: "Archive storage set", config: &Config{ Storage: Storage{ - TraceArchive: "random-value", + TracesArchive: "random-value", }, }, qSvcOpts: &querysvc.QueryServiceOptions{}, @@ -280,7 +280,7 @@ func TestServerAddArchiveStorage(t *testing.T) { name: "Archive storage not supported", config: &Config{ Storage: Storage{ - TraceArchive: "badger", + TracesArchive: "badger", }, }, qSvcOpts: &querysvc.QueryServiceOptions{}, diff --git a/docker-compose/monitor/jaeger-v2-config.yml b/docker-compose/monitor/jaeger-v2-config.yml index 14b2de057ea..de1a4ac0616 100644 --- a/docker-compose/monitor/jaeger-v2-config.yml +++ b/docker-compose/monitor/jaeger-v2-config.yml @@ -15,8 +15,8 @@ service: extensions: jaeger_query: storage: - trace: some_storage - metric: some_metrics_storage + traces: some_storage + metrics: some_metrics_storage jaeger_storage: backends: some_storage: diff --git a/docker-compose/tail-sampling/jaeger-v2-config.yml b/docker-compose/tail-sampling/jaeger-v2-config.yml index 3f32cef4ef3..c6f3dbd8838 100644 --- a/docker-compose/tail-sampling/jaeger-v2-config.yml +++ b/docker-compose/tail-sampling/jaeger-v2-config.yml @@ -15,7 +15,7 @@ extensions: http: jaeger_query: storage: - trace: some_storage + traces: some_storage jaeger_storage: backends: some_storage: From 59bcae18d11f5fd826c1897aeb35b66446a44871 Mon Sep 17 00:00:00 2001 From: Mahad Zaryab Date: Sun, 22 Sep 2024 17:44:10 -0400 Subject: [PATCH 11/18] Rename Configs Signed-off-by: Mahad Zaryab --- cmd/jaeger/config-badger.yaml | 2 +- cmd/jaeger/config-cassandra.yaml | 4 ++-- cmd/jaeger/config-elasticsearch.yaml | 4 ++-- cmd/jaeger/config.yaml | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/cmd/jaeger/config-badger.yaml b/cmd/jaeger/config-badger.yaml index 01537047696..3d222bf37bb 100644 --- a/cmd/jaeger/config-badger.yaml +++ b/cmd/jaeger/config-badger.yaml @@ -14,7 +14,7 @@ extensions: jaeger_query: storage: traces: some_store - trace_archive: another_store + traces_archive: another_store ui: config_file: ./cmd/jaeger/config-ui.json diff --git a/cmd/jaeger/config-cassandra.yaml b/cmd/jaeger/config-cassandra.yaml index c132a12da59..90983141cff 100644 --- a/cmd/jaeger/config-cassandra.yaml +++ b/cmd/jaeger/config-cassandra.yaml @@ -13,8 +13,8 @@ extensions: jaeger_query: storage: - trace: some_storage - trace_archive: another_storage + traces: some_storage + traces_archive: another_storage ui: config_file: ./cmd/jaeger/config-ui.json diff --git a/cmd/jaeger/config-elasticsearch.yaml b/cmd/jaeger/config-elasticsearch.yaml index f58d5b0b1e6..dbf5c0375bd 100644 --- a/cmd/jaeger/config-elasticsearch.yaml +++ b/cmd/jaeger/config-elasticsearch.yaml @@ -13,8 +13,8 @@ extensions: jaeger_query: storage: - trace: some_storage - trace_archive: another_storage + traces: some_storage + traces_archive: another_storage ui: config_file: ./cmd/jaeger/config-ui.json diff --git a/cmd/jaeger/config.yaml b/cmd/jaeger/config.yaml index 4ad21942a27..c1615d86460 100644 --- a/cmd/jaeger/config.yaml +++ b/cmd/jaeger/config.yaml @@ -18,7 +18,7 @@ extensions: jaeger_query: storage: - trace: some_store + traces: some_store traces_archive: another_store ui: config_file: ./cmd/jaeger/config-ui.json From c35c6a98f55cea98479facb0bebba1341e9b85b3 Mon Sep 17 00:00:00 2001 From: Mahad Zaryab Date: Sun, 22 Sep 2024 17:53:52 -0400 Subject: [PATCH 12/18] Fix Failing Tests Signed-off-by: Mahad Zaryab --- cmd/jaeger/internal/extension/jaegerquery/config_test.go | 2 +- cmd/jaeger/internal/integration/e2e_integration.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/cmd/jaeger/internal/extension/jaegerquery/config_test.go b/cmd/jaeger/internal/extension/jaegerquery/config_test.go index 43e0c66caa2..249605e0380 100644 --- a/cmd/jaeger/internal/extension/jaegerquery/config_test.go +++ b/cmd/jaeger/internal/extension/jaegerquery/config_test.go @@ -19,7 +19,7 @@ func Test_Validate(t *testing.T) { { name: "Empty config", config: &Config{}, - expectedErr: "Storage.TracePrimary: non zero value required", + expectedErr: "Storage.TracesPrimary: non zero value required", }, { name: "Non empty-config", diff --git a/cmd/jaeger/internal/integration/e2e_integration.go b/cmd/jaeger/internal/integration/e2e_integration.go index ea5745d555f..2bbd61b724d 100644 --- a/cmd/jaeger/internal/integration/e2e_integration.go +++ b/cmd/jaeger/internal/integration/e2e_integration.go @@ -225,10 +225,10 @@ func createStorageCleanerConfig(t *testing.T, configFile string, storage string) require.True(t, ok) storageAny, ok := queryAny.(map[string]any)["storage"] require.True(t, ok) - traceStorageAny, ok := storageAny.(map[string]any)["trace"] + traceStorageAny, ok := storageAny.(map[string]any)["traces"] require.True(t, ok) traceStorage := traceStorageAny.(string) - extensions["storage_cleaner"] = map[string]string{"trace_storage": traceStorage} + extensions["storage_cleaner"] = map[string]string{"traces_storage": traceStorage} jaegerStorageAny, ok := extensions["jaeger_storage"] require.True(t, ok) From b46ecbcf94adb6fff56d193e555850d8ce0b9104 Mon Sep 17 00:00:00 2001 From: Mahad Zaryab Date: Sun, 22 Sep 2024 18:13:29 -0400 Subject: [PATCH 13/18] Fix Typo Signed-off-by: Mahad Zaryab --- cmd/jaeger/internal/integration/e2e_integration.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/jaeger/internal/integration/e2e_integration.go b/cmd/jaeger/internal/integration/e2e_integration.go index 2bbd61b724d..e6805ba5eb2 100644 --- a/cmd/jaeger/internal/integration/e2e_integration.go +++ b/cmd/jaeger/internal/integration/e2e_integration.go @@ -228,7 +228,7 @@ func createStorageCleanerConfig(t *testing.T, configFile string, storage string) traceStorageAny, ok := storageAny.(map[string]any)["traces"] require.True(t, ok) traceStorage := traceStorageAny.(string) - extensions["storage_cleaner"] = map[string]string{"traces_storage": traceStorage} + extensions["storage_cleaner"] = map[string]string{"trace_storage": traceStorage} jaegerStorageAny, ok := extensions["jaeger_storage"] require.True(t, ok) From f6fa90b96f76b7be1475d180f4428912e07db160 Mon Sep 17 00:00:00 2001 From: Mahad Zaryab Date: Sun, 22 Sep 2024 23:52:48 -0400 Subject: [PATCH 14/18] Add UIConfig To Static Assets Handler Config Signed-off-by: Mahad Zaryab --- cmd/query/app/static_handler.go | 21 +++++++++++---------- cmd/query/app/static_handler_test.go | 20 +++++++++++++------- 2 files changed, 24 insertions(+), 17 deletions(-) diff --git a/cmd/query/app/static_handler.go b/cmd/query/app/static_handler.go index 89dffe606c6..8084b218155 100644 --- a/cmd/query/app/static_handler.go +++ b/cmd/query/app/static_handler.go @@ -37,11 +37,13 @@ var ( // RegisterStaticHandler adds handler for static assets to the router. func RegisterStaticHandler(r *mux.Router, logger *zap.Logger, qOpts *QueryOptions, qCapabilities querysvc.StorageCapabilities) io.Closer { staticHandler, err := NewStaticAssetsHandler(qOpts.UIConfig.AssetsPath, StaticAssetsHandlerOptions{ + UIConfig: UIConfig{ + ConfigFile: qOpts.UIConfig.ConfigFile, + LogAccess: qOpts.UIConfig.LogAccess, + }, BasePath: qOpts.BasePath, - UIConfigPath: qOpts.UIConfig.ConfigFile, StorageCapabilities: qCapabilities, Logger: logger, - LogAccess: qOpts.UIConfig.LogAccess, }) if err != nil { logger.Panic("Could not create static assets handler", zap.Error(err)) @@ -62,9 +64,8 @@ type StaticAssetsHandler struct { // StaticAssetsHandlerOptions defines options for NewStaticAssetsHandler type StaticAssetsHandlerOptions struct { + UIConfig UIConfig BasePath string - UIConfigPath string - LogAccess bool StorageCapabilities querysvc.StorageCapabilities Logger *zap.Logger } @@ -91,8 +92,8 @@ func NewStaticAssetsHandler(staticAssetsRoot string, options StaticAssetsHandler return nil, err } - options.Logger.Info("Using UI configuration", zap.String("path", options.UIConfigPath)) - watcher, err := fswatcher.New([]string{options.UIConfigPath}, h.reloadUIConfig, h.options.Logger) + options.Logger.Info("Using UI configuration", zap.String("path", options.UIConfig.ConfigFile)) + watcher, err := fswatcher.New([]string{options.UIConfig.ConfigFile}, h.reloadUIConfig, h.options.Logger) if err != nil { return nil, err } @@ -109,7 +110,7 @@ func (sH *StaticAssetsHandler) loadAndEnrichIndexHTML(open func(string) (http.Fi return nil, fmt.Errorf("cannot load index.html: %w", err) } // replace UI config - if configObject, err := loadUIConfig(sH.options.UIConfigPath); err != nil { + if configObject, err := loadUIConfig(sH.options.UIConfig.ConfigFile); err != nil { return nil, err } else if configObject != nil { indexBytes = configObject.regexp.ReplaceAll(indexBytes, configObject.config) @@ -137,13 +138,13 @@ func (sH *StaticAssetsHandler) loadAndEnrichIndexHTML(open func(string) (http.Fi } func (sH *StaticAssetsHandler) reloadUIConfig() { - sH.options.Logger.Info("reloading UI config", zap.String("filename", sH.options.UIConfigPath)) + sH.options.Logger.Info("reloading UI config", zap.String("filename", sH.options.UIConfig.ConfigFile)) content, err := sH.loadAndEnrichIndexHTML(sH.assetsFS.Open) if err != nil { sH.options.Logger.Error("error while reloading the UI config", zap.Error(err)) } sH.indexHTML.Store(content) - sH.options.Logger.Info("reloaded UI config", zap.String("filename", sH.options.UIConfigPath)) + sH.options.Logger.Info("reloaded UI config", zap.String("filename", sH.options.UIConfig.ConfigFile)) } func loadIndexHTML(open func(string) (http.File, error)) ([]byte, error) { @@ -200,7 +201,7 @@ func loadUIConfig(uiConfig string) (*loadedConfig, error) { } func (sH *StaticAssetsHandler) loggingHandler(handler http.Handler) http.Handler { - if !sH.options.LogAccess { + if !sH.options.UIConfig.LogAccess { return handler } return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { diff --git a/cmd/query/app/static_handler_test.go b/cmd/query/app/static_handler_test.go index 7f40cc4cc60..40446bdbfda 100644 --- a/cmd/query/app/static_handler_test.go +++ b/cmd/query/app/static_handler_test.go @@ -158,16 +158,20 @@ func TestRegisterStaticHandler(t *testing.T) { func TestNewStaticAssetsHandlerErrors(t *testing.T) { _, err := NewStaticAssetsHandler("fixture", StaticAssetsHandlerOptions{ - UIConfigPath: "fixture/invalid-config", - Logger: zap.NewNop(), + UIConfig: UIConfig{ + ConfigFile: "fixture/invalid-config", + }, + Logger: zap.NewNop(), }) require.Error(t, err) for _, base := range []string{"x", "x/", "/x/"} { _, err := NewStaticAssetsHandler("fixture", StaticAssetsHandlerOptions{ - UIConfigPath: "fixture/ui-config.json", - BasePath: base, - Logger: zap.NewNop(), + UIConfig: UIConfig{ + ConfigFile: "fixture/ui-config.json", + }, + BasePath: base, + Logger: zap.NewNop(), }) require.Errorf(t, err, "basePath=%s", base) assert.Contains(t, err.Error(), "invalid base path") @@ -195,8 +199,10 @@ func TestHotReloadUIConfig(t *testing.T) { zcore, logObserver := observer.New(zapcore.InfoLevel) logger := zap.New(zcore) h, err := NewStaticAssetsHandler("fixture", StaticAssetsHandlerOptions{ - UIConfigPath: cfgFileName, - Logger: logger, + UIConfig: UIConfig{ + ConfigFile: cfgFileName, + }, + Logger: logger, }) require.NoError(t, err) defer h.Close() From 12e794ea1c33f0741ac531e2805c9f63cd051af9 Mon Sep 17 00:00:00 2001 From: Mahad Zaryab Date: Mon, 23 Sep 2024 20:44:11 -0400 Subject: [PATCH 15/18] Embed UIConfig Into StaticAssetsHandlerOptions Signed-off-by: Mahad Zaryab --- cmd/query/app/static_handler.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/cmd/query/app/static_handler.go b/cmd/query/app/static_handler.go index 8084b218155..193516cf700 100644 --- a/cmd/query/app/static_handler.go +++ b/cmd/query/app/static_handler.go @@ -64,7 +64,7 @@ type StaticAssetsHandler struct { // StaticAssetsHandlerOptions defines options for NewStaticAssetsHandler type StaticAssetsHandlerOptions struct { - UIConfig UIConfig + UIConfig BasePath string StorageCapabilities querysvc.StorageCapabilities Logger *zap.Logger @@ -92,8 +92,8 @@ func NewStaticAssetsHandler(staticAssetsRoot string, options StaticAssetsHandler return nil, err } - options.Logger.Info("Using UI configuration", zap.String("path", options.UIConfig.ConfigFile)) - watcher, err := fswatcher.New([]string{options.UIConfig.ConfigFile}, h.reloadUIConfig, h.options.Logger) + options.Logger.Info("Using UI configuration", zap.String("path", options.ConfigFile)) + watcher, err := fswatcher.New([]string{options.ConfigFile}, h.reloadUIConfig, h.options.Logger) if err != nil { return nil, err } @@ -110,7 +110,7 @@ func (sH *StaticAssetsHandler) loadAndEnrichIndexHTML(open func(string) (http.Fi return nil, fmt.Errorf("cannot load index.html: %w", err) } // replace UI config - if configObject, err := loadUIConfig(sH.options.UIConfig.ConfigFile); err != nil { + if configObject, err := loadUIConfig(sH.options.ConfigFile); err != nil { return nil, err } else if configObject != nil { indexBytes = configObject.regexp.ReplaceAll(indexBytes, configObject.config) @@ -138,13 +138,13 @@ func (sH *StaticAssetsHandler) loadAndEnrichIndexHTML(open func(string) (http.Fi } func (sH *StaticAssetsHandler) reloadUIConfig() { - sH.options.Logger.Info("reloading UI config", zap.String("filename", sH.options.UIConfig.ConfigFile)) + sH.options.Logger.Info("reloading UI config", zap.String("filename", sH.options.ConfigFile)) content, err := sH.loadAndEnrichIndexHTML(sH.assetsFS.Open) if err != nil { sH.options.Logger.Error("error while reloading the UI config", zap.Error(err)) } sH.indexHTML.Store(content) - sH.options.Logger.Info("reloaded UI config", zap.String("filename", sH.options.UIConfig.ConfigFile)) + sH.options.Logger.Info("reloaded UI config", zap.String("filename", sH.options.ConfigFile)) } func loadIndexHTML(open func(string) (http.File, error)) ([]byte, error) { @@ -201,7 +201,7 @@ func loadUIConfig(uiConfig string) (*loadedConfig, error) { } func (sH *StaticAssetsHandler) loggingHandler(handler http.Handler) http.Handler { - if !sH.options.UIConfig.LogAccess { + if !sH.options.LogAccess { return handler } return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { From bce3f612b44f92482d736b549b4ec9d71179d5c5 Mon Sep 17 00:00:00 2001 From: Mahad Zaryab Date: Mon, 23 Sep 2024 20:44:57 -0400 Subject: [PATCH 16/18] Remove Redundant Qualifier Signed-off-by: Mahad Zaryab --- cmd/jaeger/internal/extension/jaegerquery/server.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/jaeger/internal/extension/jaegerquery/server.go b/cmd/jaeger/internal/extension/jaegerquery/server.go index c8f4c3f33db..caf3680205f 100644 --- a/cmd/jaeger/internal/extension/jaegerquery/server.go +++ b/cmd/jaeger/internal/extension/jaegerquery/server.go @@ -83,7 +83,7 @@ func (s *server) Start(_ context.Context, host component.Host) error { return err } - tm := tenancy.NewManager(&s.config.QueryOptionsBase.Tenancy) + tm := tenancy.NewManager(&s.config.Tenancy) // TODO OTel-collector does not initialize the tracer currently // https://github.com/open-telemetry/opentelemetry-collector/issues/7532 From 51d1b9762c2e70ad01108662d2c81f21c43cc9a4 Mon Sep 17 00:00:00 2001 From: Mahad Zaryab Date: Mon, 23 Sep 2024 21:00:13 -0400 Subject: [PATCH 17/18] Address Feedback From PR Review Signed-off-by: Mahad Zaryab --- .../internal/extension/jaegerquery/config.go | 7 ++----- .../internal/extension/jaegerquery/factory.go | 18 ++++++------------ .../internal/extension/jaegerquery/server.go | 4 ++-- .../extension/jaegerquery/server_test.go | 4 ++-- 4 files changed, 12 insertions(+), 21 deletions(-) diff --git a/cmd/jaeger/internal/extension/jaegerquery/config.go b/cmd/jaeger/internal/extension/jaegerquery/config.go index c6d63c4c804..926fe225230 100644 --- a/cmd/jaeger/internal/extension/jaegerquery/config.go +++ b/cmd/jaeger/internal/extension/jaegerquery/config.go @@ -17,11 +17,8 @@ var _ component.ConfigValidator = (*Config)(nil) // Config represents the configuration for jaeger-query, type Config struct { queryApp.QueryOptionsBase `mapstructure:",squash"` - Connection Connection `mapstructure:"connection"` - Storage Storage `mapstructure:"storage"` -} - -type Connection struct { + // Storage holds configuration related to the various data stores that are to be queried. + Storage Storage `mapstructure:"storage"` // HTTP holds the HTTP configuration that the query service uses to serve requests. HTTP confighttp.ServerConfig `mapstructure:"http"` // GRPC holds the GRPC configuration that the query service uses to serve requests. diff --git a/cmd/jaeger/internal/extension/jaegerquery/factory.go b/cmd/jaeger/internal/extension/jaegerquery/factory.go index fa40ae1749a..9d50bc55ea5 100644 --- a/cmd/jaeger/internal/extension/jaegerquery/factory.go +++ b/cmd/jaeger/internal/extension/jaegerquery/factory.go @@ -12,7 +12,6 @@ import ( "go.opentelemetry.io/collector/config/confignet" "go.opentelemetry.io/collector/extension" - "github.com/jaegertracing/jaeger/cmd/query/app" "github.com/jaegertracing/jaeger/ports" ) @@ -28,18 +27,13 @@ func NewFactory() extension.Factory { func createDefaultConfig() component.Config { return &Config{ - QueryOptionsBase: app.QueryOptionsBase{ - BasePath: "/", + HTTP: confighttp.ServerConfig{ + Endpoint: ports.PortToHostPort(ports.QueryHTTP), }, - Connection: Connection{ - HTTP: confighttp.ServerConfig{ - Endpoint: ports.PortToHostPort(ports.QueryHTTP), - }, - GRPC: configgrpc.ServerConfig{ - NetAddr: confignet.AddrConfig{ - Endpoint: ports.PortToHostPort(ports.QueryGRPC), - Transport: confignet.TransportTypeTCP, - }, + GRPC: configgrpc.ServerConfig{ + NetAddr: confignet.AddrConfig{ + Endpoint: ports.PortToHostPort(ports.QueryGRPC), + Transport: confignet.TransportTypeTCP, }, }, } diff --git a/cmd/jaeger/internal/extension/jaegerquery/server.go b/cmd/jaeger/internal/extension/jaegerquery/server.go index caf3680205f..729308f0f10 100644 --- a/cmd/jaeger/internal/extension/jaegerquery/server.go +++ b/cmd/jaeger/internal/extension/jaegerquery/server.go @@ -163,8 +163,8 @@ func (s *server) makeQueryOptions() *queryApp.QueryOptions { QueryOptionsBase: s.config.QueryOptionsBase, // TODO utilize OTEL helpers for creating HTTP/GRPC servers - HTTPHostPort: s.config.Connection.HTTP.Endpoint, - GRPCHostPort: s.config.Connection.GRPC.NetAddr.Endpoint, + HTTPHostPort: s.config.HTTP.Endpoint, + GRPCHostPort: s.config.GRPC.NetAddr.Endpoint, // TODO handle TLS } } diff --git a/cmd/jaeger/internal/extension/jaegerquery/server_test.go b/cmd/jaeger/internal/extension/jaegerquery/server_test.go index 5484ca5c3d4..213ae878379 100644 --- a/cmd/jaeger/internal/extension/jaegerquery/server_test.go +++ b/cmd/jaeger/internal/extension/jaegerquery/server_test.go @@ -209,8 +209,8 @@ func TestServerStart(t *testing.T) { }, MeterProvider: noopmetric.NewMeterProvider(), } - tt.config.Connection.HTTP.Endpoint = ":0" - tt.config.Connection.GRPC.NetAddr.Endpoint = ":0" + tt.config.HTTP.Endpoint = ":0" + tt.config.GRPC.NetAddr.Endpoint = ":0" server := newServer(tt.config, telemetrySettings) err := server.Start(context.Background(), host) if tt.expectedErr == "" { From 0a234d5a2d6a539e3fb2ddfefe1a997c1e99a923 Mon Sep 17 00:00:00 2001 From: Mahad Zaryab Date: Tue, 24 Sep 2024 18:29:32 -0400 Subject: [PATCH 18/18] Pass Field Directly As They Are The Same Type Signed-off-by: Mahad Zaryab --- cmd/query/app/static_handler.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/cmd/query/app/static_handler.go b/cmd/query/app/static_handler.go index 193516cf700..2fde0eebe3f 100644 --- a/cmd/query/app/static_handler.go +++ b/cmd/query/app/static_handler.go @@ -37,10 +37,7 @@ var ( // RegisterStaticHandler adds handler for static assets to the router. func RegisterStaticHandler(r *mux.Router, logger *zap.Logger, qOpts *QueryOptions, qCapabilities querysvc.StorageCapabilities) io.Closer { staticHandler, err := NewStaticAssetsHandler(qOpts.UIConfig.AssetsPath, StaticAssetsHandlerOptions{ - UIConfig: UIConfig{ - ConfigFile: qOpts.UIConfig.ConfigFile, - LogAccess: qOpts.UIConfig.LogAccess, - }, + UIConfig: qOpts.UIConfig, BasePath: qOpts.BasePath, StorageCapabilities: qCapabilities, Logger: logger,