Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[jaeger-v2] Refactor Configuration For Query Service #5998

Merged
merged 21 commits into from
Sep 25, 2024
Merged
Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions cmd/jaeger/config-badger.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,11 @@ extensions:
http:

jaeger_query:
trace_storage: some_store
trace_storage_archive: another_store
ui_config: ./cmd/jaeger/config-ui.json
storage:
traces: some_store
traces_archive: another_store
ui:
config_file: ./cmd/jaeger/config-ui.json

jaeger_storage:
backends:
Expand Down
8 changes: 5 additions & 3 deletions cmd/jaeger/config-cassandra.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,11 @@ extensions:
http:

jaeger_query:
trace_storage: some_storage
trace_storage_archive: another_storage
ui_config: ./cmd/jaeger/config-ui.json
storage:
traces: some_storage
traces_archive: another_storage
ui:
config_file: ./cmd/jaeger/config-ui.json

jaeger_storage:
backends:
Expand Down
8 changes: 5 additions & 3 deletions cmd/jaeger/config-elasticsearch.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,11 @@ extensions:
http:

jaeger_query:
trace_storage: some_storage
trace_storage_archive: another_storage
ui_config: ./cmd/jaeger/config-ui.json
storage:
traces: some_storage
traces_archive: another_storage
ui:
config_file: ./cmd/jaeger/config-ui.json

jaeger_storage:
backends:
Expand Down
3 changes: 2 additions & 1 deletion cmd/jaeger/config-kafka-ingester.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ extensions:
endpoint: 0.0.0.0:14133

jaeger_query:
trace_storage: some_storage
storage:
traces: some_storage

jaeger_storage:
backends:
Expand Down
8 changes: 5 additions & 3 deletions cmd/jaeger/config-opensearch.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,11 @@ extensions:
http:

jaeger_query:
trace_storage: some_storage
trace_storage_archive: another_storage
ui_config: ./cmd/jaeger/config-ui.json
storage:
traces: some_storage
traces_archive: another_storage
ui:
config_file: ./cmd/jaeger/config-ui.json

jaeger_storage:
backends:
Expand Down
6 changes: 4 additions & 2 deletions cmd/jaeger/config-remote-storage.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,10 @@ extensions:
http:

jaeger_query:
trace_storage: some-storage
ui_config: ./cmd/jaeger/config-ui.json
storage:
traces: some-storage
ui:
config_file: ./cmd/jaeger/config-ui.json

jaeger_storage:
backends:
Expand Down
3 changes: 2 additions & 1 deletion cmd/jaeger/config-tail-sampling-always-sample.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ extensions:
use_v2: true
http:
jaeger_query:
trace_storage: some_storage
storage:
traces: some_storage
jaeger_storage:
backends:
some_storage:
Expand Down
3 changes: 2 additions & 1 deletion cmd/jaeger/config-tail-sampling-service-name-policy.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ extensions:
use_v2: true
http:
jaeger_query:
trace_storage: some_storage
storage:
traces: some_storage
jaeger_storage:
backends:
some_storage:
Expand Down
8 changes: 5 additions & 3 deletions cmd/jaeger/config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,11 @@ extensions:
# endpoint: 0.0.0.0:55679

jaeger_query:
trace_storage: some_store
trace_storage_archive: another_store
ui_config: ./cmd/jaeger/config-ui.json
storage:
traces: some_store
traces_archive: another_store
ui:
config_file: ./cmd/jaeger/config-ui.json

jaeger_storage:
backends:
Expand Down
3 changes: 2 additions & 1 deletion cmd/jaeger/internal/all-in-one.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@ service:

extensions:
jaeger_query:
trace_storage: some_storage
storage:
traces: some_storage

jaeger_storage:
backends:
Expand Down
22 changes: 15 additions & 7 deletions cmd/jaeger/internal/extension/jaegerquery/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,20 +10,28 @@ 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.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"`
// 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.
GRPC configgrpc.ServerConfig `mapstructure:"grpc"`
}

type Storage struct {
// TracesPrimary contains the name of the primary trace storage that is being queried.
TracesPrimary string `mapstructure:"traces" valid:"required"`
// 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"`
}

func (cfg *Config) Validate() error {
Expand Down
6 changes: 4 additions & 2 deletions cmd/jaeger/internal/extension/jaegerquery/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,14 @@ func Test_Validate(t *testing.T) {
{
name: "Empty config",
config: &Config{},
expectedErr: "TraceStoragePrimary: non zero value required",
expectedErr: "Storage.TracesPrimary: non zero value required",
},
{
name: "Non empty-config",
config: &Config{
TraceStoragePrimary: "some-storage",
Storage: Storage{
TracesPrimary: "some-storage",
},
},
expectedErr: "",
},
Expand Down
12 changes: 6 additions & 6 deletions cmd/jaeger/internal/extension/jaegerquery/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.TraceStoragePrimary, host)
f, err := jaegerstorage.GetStorageFactory(s.config.Storage.TracesPrimary, 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.TracesPrimary, err)
}

spanReader, err := f.CreateSpanReader()
Expand Down Expand Up @@ -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.TraceStorageArchive == "" {
if s.config.Storage.TracesArchive == "" {
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.TracesArchive, host)
if err != nil {
return fmt.Errorf("cannot find archive storage factory: %w", err)
}
Expand All @@ -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.MetricStorage == "" {
if s.config.Storage.Metrics == "" {
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.Metrics, host)
if err != nil {
return nil, fmt.Errorf("cannot find metrics storage factory: %w", err)
}
Expand Down
50 changes: 35 additions & 15 deletions cmd/jaeger/internal/extension/jaegerquery/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,53 +134,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{
TracesArchive: "jaeger_storage",
TracesPrimary: "jaeger_storage",
Metrics: "jaeger_metrics_storage",
},
},
},
{
name: "factory error",
config: &Config{
TraceStoragePrimary: "need-factory-error",
Storage: Storage{
TracesPrimary: "need-factory-error",
},
},
expectedErr: "cannot find primary storage",
},
{
name: "span reader error",
config: &Config{
TraceStoragePrimary: "need-span-reader-error",
Storage: Storage{
TracesPrimary: "need-span-reader-error",
},
},
expectedErr: "cannot create span reader",
},
{
name: "dependency error",
config: &Config{
TraceStoragePrimary: "need-dependency-reader-error",
Storage: Storage{
TracesPrimary: "need-dependency-reader-error",
},
},
expectedErr: "cannot create dependencies reader",
},
{
name: "storage archive error",
config: &Config{
TraceStorageArchive: "need-factory-error",
TraceStoragePrimary: "jaeger_storage",
Storage: Storage{
TracesArchive: "need-factory-error",
TracesPrimary: "jaeger_storage",
},
},
expectedErr: "cannot find archive storage factory",
},
{
name: "metrics storage error",
config: &Config{
MetricStorage: "need-factory-error",
TraceStoragePrimary: "jaeger_storage",
Storage: Storage{
Metrics: "need-factory-error",
TracesPrimary: "jaeger_storage",
},
},
expectedErr: "cannot find metrics storage factory",
},
{
name: " metrics reader error",
config: &Config{
MetricStorage: "need-metrics-reader-error",
TraceStoragePrimary: "jaeger_storage",
Storage: Storage{
Metrics: "need-metrics-reader-error",
TracesPrimary: "jaeger_storage",
},
},
expectedErr: "cannot create metrics reader",
},
Expand Down Expand Up @@ -254,7 +268,9 @@ func TestServerAddArchiveStorage(t *testing.T) {
{
name: "Archive storage set",
config: &Config{
TraceStorageArchive: "random-value",
Storage: Storage{
TracesArchive: "random-value",
},
},
qSvcOpts: &querysvc.QueryServiceOptions{},
expectedOutput: "",
Expand All @@ -263,7 +279,9 @@ func TestServerAddArchiveStorage(t *testing.T) {
{
name: "Archive storage not supported",
config: &Config{
TraceStorageArchive: "badger",
Storage: Storage{
TracesArchive: "badger",
},
},
qSvcOpts: &querysvc.QueryServiceOptions{},
extension: fakeStorageExt{},
Expand Down Expand Up @@ -313,7 +331,9 @@ func TestServerAddMetricsStorage(t *testing.T) {
{
name: "Metrics storage set",
config: &Config{
MetricStorage: "random-value",
Storage: Storage{
Metrics: "random-value",
},
},
expectedOutput: "",
expectedErr: "cannot find metrics storage factory: cannot find extension",
Expand Down
6 changes: 4 additions & 2 deletions cmd/jaeger/internal/integration/e2e_integration.go
Original file line number Diff line number Diff line change
Expand Up @@ -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: "../../../..",
},
Expand Down Expand Up @@ -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)["traces"]
require.True(t, ok)
traceStorage := traceStorageAny.(string)
extensions["storage_cleaner"] = map[string]string{"trace_storage": traceStorage}
Expand Down
Loading
Loading