Skip to content

Commit

Permalink
Revert Struct Renaming And Move Tenancy To Base Config
Browse files Browse the repository at this point in the history
Signed-off-by: Mahad Zaryab <[email protected]>
  • Loading branch information
mahadzaryab1 committed Sep 22, 2024
1 parent 4de435d commit c3e130d
Show file tree
Hide file tree
Showing 12 changed files with 59 additions and 60 deletions.
4 changes: 2 additions & 2 deletions cmd/all-in-one/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
Expand Down Expand Up @@ -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,
Expand Down
9 changes: 3 additions & 6 deletions cmd/jaeger/internal/extension/jaegerquery/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion cmd/jaeger/internal/extension/jaegerquery/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ func NewFactory() extension.Factory {

func createDefaultConfig() component.Config {
return &Config{
OptionsBase: app.OptionsBase{
QueryOptionsBase: app.QueryOptionsBase{
BasePath: "/",
},
Connection: Connection{
Expand Down
8 changes: 4 additions & 4 deletions cmd/jaeger/internal/extension/jaegerquery/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down
18 changes: 9 additions & 9 deletions cmd/query/app/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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")
Expand Down
10 changes: 5 additions & 5 deletions cmd/query/app/flags_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
}
Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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)
Expand All @@ -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")
})
Expand Down
8 changes: 4 additions & 4 deletions cmd/query/app/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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) {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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) {
Expand Down
44 changes: 23 additions & 21 deletions cmd/query/app/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
},
}
Expand All @@ -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)
}
Expand All @@ -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)
}
Expand All @@ -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)
}
Expand Down Expand Up @@ -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,
},
}
Expand Down Expand Up @@ -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,
},
}
Expand Down Expand Up @@ -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,
},
},
Expand All @@ -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,
},
},
Expand Down Expand Up @@ -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,
},
},
Expand All @@ -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,
},
},
Expand Down Expand Up @@ -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())
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion cmd/query/app/static_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
8 changes: 4 additions & 4 deletions cmd/query/app/static_handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
},
Expand Down Expand Up @@ -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",
Expand Down
Loading

0 comments on commit c3e130d

Please sign in to comment.