Move away from global tracers. (#6591)

* Move away from global tracers.

This PR moves away from global tracers and instead initialises
a tracer provider at Service setup and passes it where it needs to be.

* Change tracing provider to be set via options.

Also change name for GetServiceTraceProvider.

* Add changelog.

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

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."`
+}

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")

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 {
+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(tracing.TraceProvider))...)
+	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),

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,
+	}
+}

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()
+}

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
+	}
+}

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),
 	}

services/proxy/pkg/tracing/tracing.go

@@ -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
-}