diff --git a/changelog/unreleased/move-proxy-to-service-tracerprovider.md b/changelog/unreleased/move-proxy-to-service-tracerprovider.md new file mode 100644 index 00000000000..f14979cb611 --- /dev/null +++ b/changelog/unreleased/move-proxy-to-service-tracerprovider.md @@ -0,0 +1,6 @@ +Enhancement: Move proxy to service tracerprovider + +This moves the proxy to initialise a service tracer provider at service initialisation time, +instead of using a package global tracer provider. + +https://github.com/owncloud/ocis/pull/6591 diff --git a/ocis-pkg/tracing/config.go b/ocis-pkg/tracing/config.go new file mode 100644 index 00000000000..a4b299c9471 --- /dev/null +++ b/ocis-pkg/tracing/config.go @@ -0,0 +1,14 @@ +package tracing + +// ConfigConverter is the interface for external configuration. +type ConfigConverter interface { + Convert() Config +} + +// Tracing defines the available tracing configuration. +type Config struct { + Enabled bool `yaml:"enabled" env:"OCIS_TRACING_ENABLED" desc:"Activates tracing."` + Type string `yaml:"type" env:"OCIS_TRACING_TYPE" desc:"The type of tracing. Defaults to \"\", which is the same as \"jaeger\". Allowed tracing types are \"jaeger\" and \"\" as of now."` + Endpoint string `yaml:"endpoint" env:"OCIS_TRACING_ENDPOINT" desc:"The endpoint of the tracing agent."` + Collector string `yaml:"collector" env:"OCIS_TRACING_COLLECTOR" desc:"The HTTP endpoint for sending spans directly to a collector, i.e. http://jaeger-collector:14268/api/traces. Only used if the tracing endpoint is unset."` +} diff --git a/ocis-pkg/tracing/tracing.go b/ocis-pkg/tracing/tracing.go index 157a2245428..7f471622824 100644 --- a/ocis-pkg/tracing/tracing.go +++ b/ocis-pkg/tracing/tracing.go @@ -16,6 +16,7 @@ import ( "go.opentelemetry.io/otel/sdk/resource" sdktrace "go.opentelemetry.io/otel/sdk/trace" semconv "go.opentelemetry.io/otel/semconv/v1.4.0" + "go.opentelemetry.io/otel/trace" "google.golang.org/grpc" "google.golang.org/grpc/credentials/insecure" ) @@ -26,6 +27,15 @@ var Propagator = propagation.NewCompositeTextMapPropagator( propagation.TraceContext{}, ) +// GetServiceTraceProvider returns a configured open-telemetry trace provider. +func GetServiceTraceProvider(c ConfigConverter, serviceName string) (trace.TracerProvider, error) { + cfg := c.Convert() + if cfg.Enabled { + return GetTraceProvider(cfg.Endpoint, cfg.Collector, serviceName, cfg.Type) + } + return trace.NewNoopTracerProvider(), nil +} + // GetTraceProvider returns a configured open-telemetry trace provider. func GetTraceProvider(endpoint, collector, serviceName, traceType string) (*sdktrace.TracerProvider, error) { switch t := traceType; t { @@ -92,7 +102,6 @@ func GetTraceProvider(endpoint, collector, serviceName, traceType string) (*sdkt context.Background(), otlptracegrpc.WithGRPCConn(conn), ) - if err != nil { return nil, err } @@ -184,7 +193,6 @@ func Configure(enabled bool, tracingType string, logger log.Logger) { Str("type", tracingType). Msg("Unknown tracing exporter") } - } else { logger.Debug(). Msg("Tracing is not enabled") diff --git a/services/proxy/pkg/command/server.go b/services/proxy/pkg/command/server.go index 2bd5aff62be..c3d95ed669b 100644 --- a/services/proxy/pkg/command/server.go +++ b/services/proxy/pkg/command/server.go @@ -22,6 +22,7 @@ import ( "github.com/owncloud/ocis/v2/ocis-pkg/oidc" "github.com/owncloud/ocis/v2/ocis-pkg/registry" "github.com/owncloud/ocis/v2/ocis-pkg/service/grpc" + "github.com/owncloud/ocis/v2/ocis-pkg/tracing" "github.com/owncloud/ocis/v2/ocis-pkg/version" settingssvc "github.com/owncloud/ocis/v2/protogen/gen/ocis/services/settings/v0" storesvc "github.com/owncloud/ocis/v2/protogen/gen/ocis/services/store/v0" @@ -35,12 +36,12 @@ import ( "github.com/owncloud/ocis/v2/services/proxy/pkg/router" "github.com/owncloud/ocis/v2/services/proxy/pkg/server/debug" proxyHTTP "github.com/owncloud/ocis/v2/services/proxy/pkg/server/http" - "github.com/owncloud/ocis/v2/services/proxy/pkg/tracing" "github.com/owncloud/ocis/v2/services/proxy/pkg/user/backend" "github.com/owncloud/ocis/v2/services/proxy/pkg/userroles" "github.com/urfave/cli/v2" microstore "go-micro.dev/v4/store" "go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp" + "go.opentelemetry.io/otel/trace" ) // Server is the entrypoint for the server command. @@ -63,7 +64,7 @@ func Server(cfg *config.Config) *cli.Command { ) logger := logging.Configure(cfg.Service.Name, cfg.Log) - err := tracing.Configure(cfg) + traceProvider, err := tracing.GetServiceTraceProvider(cfg.Tracing, cfg.Service.Name) if err != nil { return err } @@ -72,7 +73,7 @@ func Server(cfg *config.Config) *cli.Command { return err } - var oidcHTTPClient = &http.Client{ + oidcHTTPClient := &http.Client{ Transport: &http.Transport{ TLSClientConfig: &tls.Config{ MinVersion: tls.VersionTLS12, @@ -91,9 +92,7 @@ func Server(cfg *config.Config) *cli.Command { oidc.WithJWKSOptions(cfg.OIDC.JWKS), ) - var ( - m = metrics.New() - ) + m := metrics.New() gr := run.Group{} ctx, cancel := func() (context.Context, context.CancelFunc) { @@ -125,7 +124,7 @@ func Server(cfg *config.Config) *cli.Command { } { - middlewares := loadMiddlewares(ctx, logger, cfg, userInfoCache) + middlewares := loadMiddlewares(ctx, logger, cfg, userInfoCache, traceProvider) server, err := proxyHTTP.Server( proxyHTTP.Handler(lh.handler()), proxyHTTP.Logger(logger), @@ -134,7 +133,6 @@ func Server(cfg *config.Config) *cli.Command { proxyHTTP.Metrics(metrics.New()), proxyHTTP.Middlewares(middlewares), ) - if err != nil { logger.Error(). Err(err). @@ -162,7 +160,6 @@ func Server(cfg *config.Config) *cli.Command { debug.Context(ctx), debug.Config(cfg), ) - if err != nil { logger.Error().Err(err).Str("server", "debug").Msg("Failed to initialize server") return err @@ -197,7 +194,6 @@ func (h *StaticRouteHandler) handler() http.Handler { // TODO: migrate oidc well knowns here in a second wrapper r.HandleFunc("/*", h.proxy.ServeHTTP) - }) // This is commented out due to a race issue in chi //var methods = []string{"PROPFIND", "DELETE", "PROPPATCH", "MKCOL", "COPY", "MOVE", "LOCK", "UNLOCK", "REPORT"} @@ -272,8 +268,11 @@ func (h *StaticRouteHandler) backchannelLogout(w http.ResponseWriter, r *http.Re render.JSON(w, r, nil) } -func loadMiddlewares(ctx context.Context, logger log.Logger, cfg *config.Config, userInfoCache microstore.Store) alice.Chain { - grpcClient, err := grpc.NewClient(append(grpc.GetClientOptions(cfg.GRPCClientTLS), grpc.WithTraceProvider(tracing.TraceProvider))...) +func loadMiddlewares(ctx context.Context, logger log.Logger, cfg *config.Config, userInfoCache microstore.Store, traceProvider trace.TracerProvider) alice.Chain { + grpcClient, err := grpc.NewClient( + append( + grpc.GetClientOptions(cfg.GRPCClientTLS), + grpc.WithTraceProvider(traceProvider))...) if err != nil { logger.Fatal().Err(err).Msg("Failed to get gateway client") } @@ -330,7 +329,7 @@ func loadMiddlewares(ctx context.Context, logger log.Logger, cfg *config.Config, Msg("Failed to create reva gateway service client") } - var oidcHTTPClient = &http.Client{ + oidcHTTPClient := &http.Client{ Transport: &http.Transport{ TLSClientConfig: &tls.Config{ MinVersion: tls.VersionTLS12, @@ -379,12 +378,12 @@ func loadMiddlewares(ctx context.Context, logger log.Logger, cfg *config.Config, return alice.New( // first make sure we log all requests and redirect to https if necessary otelhttp.NewMiddleware("proxy", - otelhttp.WithTracerProvider(tracing.TraceProvider), + otelhttp.WithTracerProvider(traceProvider), otelhttp.WithSpanNameFormatter(func(name string, r *http.Request) string { return fmt.Sprintf("%s %s", r.Method, r.URL.Path) }), ), - middleware.Tracer(), + middleware.Tracer(traceProvider), pkgmiddleware.TraceContext, chimiddleware.RealIP, chimiddleware.RequestID, @@ -402,6 +401,7 @@ func loadMiddlewares(ctx context.Context, logger log.Logger, cfg *config.Config, middleware.Logger(logger), middleware.OIDCIss(cfg.OIDC.Issuer), middleware.EnableBasicAuth(cfg.EnableBasicAuth), + middleware.TraceProvider(traceProvider), ), middleware.AccountResolver( middleware.Logger(logger), diff --git a/services/proxy/pkg/config/tracing.go b/services/proxy/pkg/config/tracing.go index f8a1bc79dda..388f0a16b29 100644 --- a/services/proxy/pkg/config/tracing.go +++ b/services/proxy/pkg/config/tracing.go @@ -1,5 +1,7 @@ package config +import "github.com/owncloud/ocis/v2/ocis-pkg/tracing" + // Tracing defines the available tracing configuration. type Tracing struct { Enabled bool `yaml:"enabled" env:"OCIS_TRACING_ENABLED;PROXY_TRACING_ENABLED" desc:"Activates tracing."` @@ -7,3 +9,13 @@ type Tracing struct { Endpoint string `yaml:"endpoint" env:"OCIS_TRACING_ENDPOINT;PROXY_TRACING_ENDPOINT" desc:"The endpoint of the tracing agent."` Collector string `yaml:"collector" env:"OCIS_TRACING_COLLECTOR;PROXY_TRACING_COLLECTOR" desc:"The HTTP endpoint for sending spans directly to a collector, i.e. http://jaeger-collector:14268/api/traces. Only used if the tracing endpoint is unset."` } + +// Convert Tracing to the tracing package's Config struct. +func (t Tracing) Convert() tracing.Config { + return tracing.Config{ + Enabled: t.Enabled, + Type: t.Type, + Endpoint: t.Endpoint, + Collector: t.Collector, + } +} diff --git a/services/proxy/pkg/middleware/authentication.go b/services/proxy/pkg/middleware/authentication.go index 9958e840127..f81e4dec183 100644 --- a/services/proxy/pkg/middleware/authentication.go +++ b/services/proxy/pkg/middleware/authentication.go @@ -8,7 +8,6 @@ import ( "strings" "github.com/owncloud/ocis/v2/services/proxy/pkg/router" - proxytracing "github.com/owncloud/ocis/v2/services/proxy/pkg/tracing" "github.com/owncloud/ocis/v2/services/proxy/pkg/webdav" "go.opentelemetry.io/otel/trace" "golang.org/x/text/cases" @@ -50,7 +49,8 @@ type Authenticator interface { func Authentication(auths []Authenticator, opts ...Option) func(next http.Handler) http.Handler { options := newOptions(opts...) configureSupportedChallenges(options) - tracer := proxytracing.TraceProvider.Tracer("proxy") + tracer := getTraceProvider(options).Tracer("proxy") + spanOpts := []trace.SpanStartOption{ trace.WithSpanKind(trace.SpanKindServer), } @@ -206,3 +206,10 @@ func evalRequestURI(l userAgentLocker, r regexp.Regexp) { } l.w.Header().Add(WwwAuthenticate, fmt.Sprintf("%v realm=\"%s\", charset=\"UTF-8\"", caser.String(l.fallback), l.r.Host)) } + +func getTraceProvider(o Options) trace.TracerProvider { + if o.TraceProvider != nil { + return o.TraceProvider + } + return trace.NewNoopTracerProvider() +} diff --git a/services/proxy/pkg/middleware/options.go b/services/proxy/pkg/middleware/options.go index edfb39fc8c0..cd64b44724d 100644 --- a/services/proxy/pkg/middleware/options.go +++ b/services/proxy/pkg/middleware/options.go @@ -14,6 +14,7 @@ import ( "github.com/owncloud/ocis/v2/services/proxy/pkg/user/backend" "github.com/owncloud/ocis/v2/services/proxy/pkg/userroles" store "go-micro.dev/v4/store" + "go.opentelemetry.io/otel/trace" ) // Option defines a single option function. @@ -67,6 +68,8 @@ type Options struct { // RoleQuotas hold userid:quota mappings. These will be used when provisioning new users. // The users will get as much quota as is set for their role. RoleQuotas map[string]uint64 + // TraceProvider sets the tracing provider. + TraceProvider trace.TracerProvider } // newOptions initializes the available default options. @@ -226,3 +229,10 @@ func RoleQuotas(roleQuotas map[string]uint64) Option { o.RoleQuotas = roleQuotas } } + +// TraceProvider sets the tracing provider. +func TraceProvider(tp trace.TracerProvider) Option { + return func(o *Options) { + o.TraceProvider = tp + } +} diff --git a/services/proxy/pkg/middleware/tracing.go b/services/proxy/pkg/middleware/tracing.go index 84760cba277..9b897720dce 100644 --- a/services/proxy/pkg/middleware/tracing.go +++ b/services/proxy/pkg/middleware/tracing.go @@ -6,23 +6,24 @@ import ( chimiddleware "github.com/go-chi/chi/v5/middleware" pkgtrace "github.com/owncloud/ocis/v2/ocis-pkg/tracing" - proxytracing "github.com/owncloud/ocis/v2/services/proxy/pkg/tracing" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/propagation" "go.opentelemetry.io/otel/trace" ) // Tracer provides a middleware to start traces -func Tracer() func(next http.Handler) http.Handler { +func Tracer(tp trace.TracerProvider) func(next http.Handler) http.Handler { return func(next http.Handler) http.Handler { return &tracer{ - next: next, + next: next, + traceProvider: tp, } } } type tracer struct { - next http.Handler + next http.Handler + traceProvider trace.TracerProvider } func (m tracer) ServeHTTP(w http.ResponseWriter, r *http.Request) { @@ -31,7 +32,7 @@ func (m tracer) ServeHTTP(w http.ResponseWriter, r *http.Request) { span trace.Span ) - tracer := proxytracing.TraceProvider.Tracer("proxy") + tracer := m.traceProvider.Tracer("proxy") spanOpts := []trace.SpanStartOption{ trace.WithSpanKind(trace.SpanKindServer), } diff --git a/services/proxy/pkg/tracing/tracing.go b/services/proxy/pkg/tracing/tracing.go deleted file mode 100644 index b8a07b12ca9..00000000000 --- a/services/proxy/pkg/tracing/tracing.go +++ /dev/null @@ -1,23 +0,0 @@ -package tracing - -import ( - pkgtrace "github.com/owncloud/ocis/v2/ocis-pkg/tracing" - "github.com/owncloud/ocis/v2/services/proxy/pkg/config" - "go.opentelemetry.io/otel/trace" -) - -var ( - // TraceProvider is the global trace provider for the proxy service. - TraceProvider = trace.NewNoopTracerProvider() -) - -func Configure(cfg *config.Config) error { - var err error - if cfg.Tracing.Enabled { - if TraceProvider, err = pkgtrace.GetTraceProvider(cfg.Tracing.Endpoint, cfg.Tracing.Collector, cfg.Service.Name, cfg.Tracing.Type); err != nil { - return err - } - } - - return nil -}