Merge pull request #1918 from opencloud-eu/otlp-tracing

update otlp tracing

pkg/config/config.go

@@ -62,7 +62,6 @@ type Runtime struct {
 type Config struct {
 	*shared.Commons `yaml:"shared"`
 
-	Tracing        *shared.Tracing        `yaml:"tracing"`
 	Log            *shared.Log            `yaml:"log"`
 	Cache          *shared.Cache          `yaml:"cache"`
 	GRPCClientTLS  *shared.GRPCClientTLS  `yaml:"grpc_client_tls"`

pkg/config/parser/parse.go

@@ -40,9 +40,6 @@ func ParseConfig(cfg *config.Config, skipValidate bool) error {
 // EnsureDefaults ensures that all pointers in the
 // OpenCloud config (not the services configs) are initialized
 func EnsureDefaults(cfg *config.Config) {
-	if cfg.Tracing == nil {
-		cfg.Tracing = &shared.Tracing{}
-	}
 	if cfg.Log == nil {
 		cfg.Log = &shared.Log{}
 	}
@@ -71,7 +68,6 @@ func EnsureCommons(cfg *config.Config) {
 	}
 
 	cfg.Commons.Log = structs.CopyOrZeroValue(cfg.Log)
-	cfg.Commons.Tracing = structs.CopyOrZeroValue(cfg.Tracing)
 	cfg.Commons.Cache = structs.CopyOrZeroValue(cfg.Cache)
 
 	if cfg.GRPCClientTLS != nil {

pkg/shared/shared_types.go

@@ -18,14 +18,6 @@ type Log struct {
 	File   string `yaml:"file" env:"OC_LOG_FILE" desc:"The path to the log file. Activates logging to this file if set." introductionVersion:"1.0.0"`
 }
 
-// Tracing defines the available tracing configuration.
-type Tracing struct {
-	Enabled   bool   `yaml:"enabled" env:"OC_TRACING_ENABLED" desc:"Activates tracing." introductionVersion:"1.0.0"`
-	Type      string `yaml:"type" env:"OC_TRACING_TYPE" desc:"The type of tracing. Defaults to '', which is the same as 'jaeger'. Allowed tracing types are 'jaeger' and '' as of now." introductionVersion:"1.0.0"`
-	Endpoint  string `yaml:"endpoint" env:"OC_TRACING_ENDPOINT" desc:"The endpoint of the tracing agent." introductionVersion:"1.0.0"`
-	Collector string `yaml:"collector" env:"OC_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." introductionVersion:"1.0.0"`
-}
-
 // TokenManager is the config for using the reva token manager
 type TokenManager struct {
 	JWTSecret string `mask:"password" yaml:"jwt_secret" env:"OC_JWT_SECRET" desc:"The secret to mint and validate jwt tokens." introductionVersion:"1.0.0"`
@@ -69,8 +61,8 @@ type Cache struct {
 // Commons holds configuration that are common to all extensions. Each extension can then decide whether
 // to overwrite its values.
 type Commons struct {
+	TracesExporter     string          `yaml:"traces_exporter" env:"OTEL_TRACES_EXPORTER" desc:"The exporter used for traces. Supports 'otlp', 'console' and 'none' (default)." introductionVersion:"%%NEXT%%"`
 	Log                *Log            `yaml:"log"`
-	Tracing            *Tracing        `yaml:"tracing"`
 	Cache              *Cache          `yaml:"cache"`
 	GRPCClientTLS      *GRPCClientTLS  `yaml:"grpc_client_tls"`
 	GRPCServiceTLS     *GRPCServiceTLS `yaml:"grpc_service_tls"`

pkg/tracing/config.go

@@ -1,14 +0,0 @@
-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:"OC_TRACING_ENABLED" desc:"Activates tracing." introductionVersion:"1.0.0"`
-	Type      string `yaml:"type" env:"OC_TRACING_TYPE" desc:"The type of tracing. Defaults to \"\", which is the same as \"jaeger\". Allowed tracing types are \"jaeger\" and \"\" as of now." introductionVersion:"1.0.0"`
-	Endpoint  string `yaml:"endpoint" env:"OC_TRACING_ENDPOINT" desc:"The endpoint of the tracing agent." introductionVersion:"1.0.0"`
-	Collector string `yaml:"collector" env:"OC_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." introductionVersion:"1.0.0"`
-}

pkg/tracing/tracing.go

@@ -3,22 +3,16 @@ package tracing
 import (
 	"context"
 	"fmt"
-	"net/url"
-	"reflect"
-	"strings"
-	"time"
 
 	rtrace "github.com/opencloud-eu/reva/v2/pkg/trace"
 	"go.opentelemetry.io/otel/attribute"
-	"go.opentelemetry.io/otel/exporters/jaeger"
 	"go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc"
+	"go.opentelemetry.io/otel/exporters/stdout/stdouttrace"
 	"go.opentelemetry.io/otel/propagation"
 	"go.opentelemetry.io/otel/sdk/resource"
 	sdktrace "go.opentelemetry.io/otel/sdk/trace"
-	semconv "go.opentelemetry.io/otel/semconv/v1.4.0"
+	semconv "go.opentelemetry.io/otel/semconv/v1.37.0"
 	"go.opentelemetry.io/otel/trace"
-	"google.golang.org/grpc"
-	"google.golang.org/grpc/credentials/insecure"
 )
 
 // Propagator ensures the importer module uses the same trace propagation strategy.
@@ -27,25 +21,9 @@ var Propagator = propagation.NewCompositeTextMapPropagator(
 	propagation.TraceContext{},
 )
 
-// GetServiceTraceProvider returns a configured open-telemetry trace provider.
-func GetServiceTraceProvider(c ConfigConverter, serviceName string) (trace.TracerProvider, error) {
-	var cfg Config
-	if c == nil || reflect.ValueOf(c).IsNil() {
-		cfg = Config{Enabled: false}
-	} else {
-		cfg = c.Convert()
-	}
-
-	if cfg.Enabled {
-		return GetTraceProvider(cfg.Endpoint, cfg.Collector, serviceName, cfg.Type)
-	}
-
-	tp := sdktrace.NewTracerProvider(
-		sdktrace.WithSampler(sdktrace.NeverSample()),
-	)
-	rtrace.SetDefaultTracerProvider(tp)
-
-	return tp, nil
+// Deprecated: GetServiceTraceProvider returns a configured open-telemetry trace provider. Use GetTraceProvider.
+func GetServiceTraceProvider(exporter, serviceName string) (trace.TracerProvider, error) {
+	return GetTraceProvider(context.Background(), exporter, serviceName)
 }
 
 // GetPropagator gets a configured propagator.
@@ -57,116 +35,83 @@ func GetPropagator() propagation.TextMapPropagator {
 }
 
 // GetTraceProvider returns a configured open-telemetry trace provider.
-func GetTraceProvider(endpoint, collector, serviceName, traceType string) (*sdktrace.TracerProvider, error) {
-	switch t := traceType; t {
-	case "", "jaeger":
-		var (
-			exp *jaeger.Exporter
-			err error
-		)
+func GetTraceProvider(ctx context.Context, exporter, serviceName string) (*sdktrace.TracerProvider, error) {
 
-		if endpoint != "" {
-			var agentHost string
-			var agentPort string
+	// Create resource - shared across all exporters
+	resources, err := createResource(ctx, serviceName)
+	if err != nil {
+		return nil, fmt.Errorf("failed to create resource: %w", err)
+	}
 
-			agentHost, agentPort, err = parseAgentConfig(endpoint)
-			if err != nil {
-				return nil, err
-			}
+	var tp *sdktrace.TracerProvider
 
-			exp, err = jaeger.New(
-				jaeger.WithAgentEndpoint(
-					jaeger.WithAgentHost(agentHost),
-					jaeger.WithAgentPort(agentPort),
-				),
-			)
-		} else if collector != "" {
-			exp, err = jaeger.New(
-				jaeger.WithCollectorEndpoint(
-					jaeger.WithEndpoint(collector),
-				),
-			)
-		} else {
-			return sdktrace.NewTracerProvider(sdktrace.WithSampler(sdktrace.NeverSample())), nil
-		}
-		if err != nil {
-			return nil, err
-		}
-
-		tp := sdktrace.NewTracerProvider(
-			sdktrace.WithBatcher(exp),
-			sdktrace.WithResource(resource.NewWithAttributes(
-				semconv.SchemaURL,
-				semconv.ServiceNameKey.String(serviceName)),
-			),
-		)
-		rtrace.SetDefaultTracerProvider(tp)
-		return tp, nil
-	case "otlp":
-		ctx, cancel := context.WithTimeout(context.Background(), time.Second)
-		defer cancel()
-		conn, err := grpc.DialContext(ctx, endpoint,
-			// Note the use of insecure transport here. TLS is recommended in production.
-			grpc.WithTransportCredentials(insecure.NewCredentials()),
-			grpc.WithBlock(),
-		)
-		if err != nil {
-			return nil, fmt.Errorf("failed to create gRPC connection to collector: %w", err)
-		}
-		exporter, err := otlptracegrpc.New(
-			context.Background(),
-			otlptracegrpc.WithGRPCConn(conn),
-		)
-		if err != nil {
-			return nil, err
-		}
-		resources, err := resource.New(
-			context.Background(),
-			resource.WithAttributes(
-				attribute.String("service.name", serviceName),
-				attribute.String("library.language", "go"),
-			),
-		)
-		if err != nil {
-			return nil, err
-		}
-
-		tp := sdktrace.NewTracerProvider(
-			sdktrace.WithSampler(sdktrace.AlwaysSample()),
-			sdktrace.WithBatcher(exporter),
+	switch exporter {
+	case "", "none":
+		// No-op exporter - never sample
+		tp = sdktrace.NewTracerProvider(
+			sdktrace.WithSampler(sdktrace.NeverSample()),
 			sdktrace.WithResource(resources),
 		)
-		rtrace.SetDefaultTracerProvider(tp)
-		return tp, nil
-	case "agent":
-		fallthrough
-	case "zipkin":
-		fallthrough
+
+	case "console":
+		// Console exporter - prints to stdout (useful for debugging)
+		consoleExporter, err := stdouttrace.New(
+			stdouttrace.WithPrettyPrint(),
+		)
+		if err != nil {
+			return nil, fmt.Errorf("failed to create console exporter: %w", err)
+		}
+
+		// Use SimpleSpanProcessor for console to get immediate output
+		tp = sdktrace.NewTracerProvider(
+			sdktrace.WithSpanProcessor(sdktrace.NewSimpleSpanProcessor(consoleExporter)),
+			sdktrace.WithResource(resources),
+		)
+
+	case "otlp":
+		// OTLP exporter - connects to collector
+		// This automatically reads:
+		// - OTEL_EXPORTER_OTLP_ENDPOINT
+		// - OTEL_EXPORTER_OTLP_TRACES_ENDPOINT (takes precedence)
+		// - OTEL_EXPORTER_OTLP_HEADERS
+		// - OTEL_EXPORTER_OTLP_INSECURE
+		// - OTEL_EXPORTER_OTLP_CERTIFICATE (for custom CA)
+		otlpExporter, err := otlptracegrpc.New(ctx)
+		if err != nil {
+			return nil, fmt.Errorf("failed to create OTLP exporter: %w", err)
+		}
+
+		// Create tracer provider
+		// This automatically reads:
+		// - OTEL_TRACES_SAMPLER
+		// - OTEL_TRACES_SAMPLER_ARG
+		tp = sdktrace.NewTracerProvider(
+			sdktrace.WithBatcher(otlpExporter),
+			sdktrace.WithResource(resources),
+		)
+
 	default:
-		return nil, fmt.Errorf("unknown trace type %s", traceType)
+		return nil, fmt.Errorf("unsupported trace exporter: %q (supported: none, console, otlp)", exporter)
 	}
+
+	// Set as global default
+	rtrace.SetDefaultTracerProvider(tp)
+
+	return tp, nil
 }
 
-func parseAgentConfig(ae string) (string, string, error) {
-	u, err := url.Parse(ae)
-	// as per url.go:
-	// [...] Trying to parse a hostname and path
-	// without a scheme is invalid but may not necessarily return an
-	// error, due to parsing ambiguities.
-	if err == nil && u.Hostname() != "" && u.Port() != "" {
-		return u.Hostname(), u.Port(), nil
-	}
-
-	p := strings.Split(ae, ":")
-	if len(p) != 2 {
-		return "", "", fmt.Errorf("invalid agent endpoint `%s`. expected format: `hostname:port`", ae)
-	}
-
-	switch {
-	case p[0] == "" && p[1] == "": // case ae = ":"
-		return "", "", fmt.Errorf("invalid agent endpoint `%s`. expected format: `hostname:port`", ae)
-	case p[0] == "":
-		return "", "", fmt.Errorf("invalid agent endpoint `%s`. expected format: `hostname:port`", ae)
-	}
-	return p[0], p[1], nil
+// createResource creates a resource with service information
+func createResource(ctx context.Context, serviceName string) (*resource.Resource, error) {
+	return resource.New(ctx,
+		// Reads OTEL_RESOURCE_ATTRIBUTES and OTEL_SERVICE_NAME
+		resource.WithFromEnv(),
+		// Host and process information
+		resource.WithHost(),
+		resource.WithProcess(),
+		// Service attributes
+		resource.WithAttributes(
+			semconv.ServiceName(serviceName),
+			attribute.String("library.language", "go"),
+		),
+	)
 }

pkg/tracing/tracing_test.go

@@ -1,77 +0,0 @@
-package tracing
-
-import "testing"
-
-func Test_parseAgentConfig(t *testing.T) {
-	type args struct {
-		ae string
-	}
-	tests := []struct {
-		name    string
-		args    args
-		want    string
-		want1   string
-		wantErr bool
-	}{
-		{
-			name: "docker-style config",
-			args: args{
-				ae: "docker-jaeger:6666",
-			},
-			want:    "docker-jaeger",
-			want1:   "6666",
-			wantErr: false,
-		},
-		{
-			name: "agent in an url config",
-			args: args{
-				ae: "https://example-agent.com:6666",
-			},
-			want:    "example-agent.com",
-			want1:   "6666",
-			wantErr: false,
-		},
-		{
-			name: "agent as ipv4",
-			args: args{
-				ae: "127.0.0.1:6666",
-			},
-			want:    "127.0.0.1",
-			want1:   "6666",
-			wantErr: false,
-		},
-		{
-			name: "no hostname config should error",
-			args: args{
-				ae: ":6666",
-			},
-			want:    "",
-			want1:   "",
-			wantErr: true,
-		},
-		{
-			name: "no hostname nor port but separator should error",
-			args: args{
-				ae: ":",
-			},
-			want:    "",
-			want1:   "",
-			wantErr: true,
-		},
-	}
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			got, got1, err := parseAgentConfig(tt.args.ae)
-			if (err != nil) != tt.wantErr {
-				t.Errorf("parseAgentConfig() error = %v, wantErr %v", err, tt.wantErr)
-				return
-			}
-			if got != tt.want {
-				t.Errorf("parseAgentConfig() got = %v, want %v", got, tt.want)
-			}
-			if got1 != tt.want1 {
-				t.Errorf("parseAgentConfig() got1 = %v, want %v", got1, tt.want1)
-			}
-		})
-	}
-}