From d9c9243251d145e72a0e155de2f2258899be58d8 Mon Sep 17 00:00:00 2001 From: jonny rhea Date: Mon, 29 Dec 2025 17:21:29 -0600 Subject: [PATCH] address review feedback --- cmd/geth/config.go | 9 +++ cmd/geth/main.go | 9 +-- cmd/keeper/go.mod | 4 +- cmd/keeper/go.sum | 16 +++--- cmd/utils/flags.go | 21 +------ cmd/utils/otel.go | 131 ++++++++++++++++++++++++++++++++++++++++++ go.mod | 7 +-- go.sum | 2 - internal/otel/otel.go | 120 -------------------------------------- 9 files changed, 157 insertions(+), 162 deletions(-) create mode 100644 cmd/utils/otel.go delete mode 100644 internal/otel/otel.go diff --git a/cmd/geth/config.go b/cmd/geth/config.go index 87627467d2..cbbe26be44 100644 --- a/cmd/geth/config.go +++ b/cmd/geth/config.go @@ -242,6 +242,15 @@ func makeFullNode(ctx *cli.Context) *node.Node { // Start metrics export if enabled utils.SetupMetrics(&cfg.Metrics) + // Setup OpenTelemetry tracing if enabled + otelService, err := utils.SetupOTEL(ctx) + if err != nil { + utils.Fatalf("failed to setup OpenTelemetry: %v", err) + } + if otelService != nil { + utils.RegisterOTELService(otelService, stack) + } + backend, eth := utils.RegisterEthService(stack, &cfg.Eth) // Create gauge with geth system and build information diff --git a/cmd/geth/main.go b/cmd/geth/main.go index d1b7a85860..e267e30edd 100644 --- a/cmd/geth/main.go +++ b/cmd/geth/main.go @@ -33,7 +33,6 @@ import ( "github.com/ethereum/go-ethereum/ethclient" "github.com/ethereum/go-ethereum/internal/debug" "github.com/ethereum/go-ethereum/internal/flags" - "github.com/ethereum/go-ethereum/internal/otel" "github.com/ethereum/go-ethereum/log" "github.com/ethereum/go-ethereum/node" "go.uber.org/automaxprocs/maxprocs" @@ -218,10 +217,8 @@ var ( } otelFlags = []cli.Flag{ - utils.OTELTracingFlag, + utils.OTELEnabledFlag, utils.OTELEndpointFlag, - utils.OTELProtocolFlag, - utils.OTELInsecureFlag, utils.OTELServiceNameFlag, } ) @@ -284,14 +281,10 @@ func init() { if err := debug.Setup(ctx); err != nil { return err } - if err := otel.Setup(ctx); err != nil { - return err - } flags.CheckEnvVars(ctx, app.Flags, "GETH") return nil } app.After = func(ctx *cli.Context) error { - otel.Exit() debug.Exit() prompt.Stdin.Close() // Resets terminal mode. return nil diff --git a/cmd/keeper/go.mod b/cmd/keeper/go.mod index cee1ce05a7..c193f4fc2d 100644 --- a/cmd/keeper/go.mod +++ b/cmd/keeper/go.mod @@ -32,8 +32,8 @@ require ( github.com/supranational/blst v0.3.16-0.20250831170142-f48500c1fdbe // indirect github.com/tklauser/go-sysconf v0.3.12 // indirect github.com/tklauser/numcpus v0.6.1 // indirect - golang.org/x/crypto v0.36.0 // indirect - golang.org/x/sync v0.12.0 // indirect + golang.org/x/crypto v0.44.0 // indirect + golang.org/x/sync v0.18.0 // indirect golang.org/x/sys v0.39.0 // indirect gopkg.in/yaml.v2 v2.4.0 // indirect ) diff --git a/cmd/keeper/go.sum b/cmd/keeper/go.sum index 62f10968e2..e9c081e605 100644 --- a/cmd/keeper/go.sum +++ b/cmd/keeper/go.sum @@ -108,22 +108,22 @@ github.com/tklauser/go-sysconf v0.3.12 h1:0QaGUFOdQaIVdPgfITYzaTegZvdCjmYO52cSFA github.com/tklauser/go-sysconf v0.3.12/go.mod h1:Ho14jnntGE1fpdOqQEEaiKRpvIavV0hSfmBq8nJbHYI= github.com/tklauser/numcpus v0.6.1 h1:ng9scYS7az0Bk4OZLvrNXNSAO2Pxr1XXRAPyjhIx+Fk= github.com/tklauser/numcpus v0.6.1/go.mod h1:1XfjsgE2zo8GVw7POkMbHENHzVg3GzmoZ9fESEdAacY= -golang.org/x/crypto v0.36.0 h1:AnAEvhDddvBdpY+uR+MyHmuZzzNqXSe/GvuDeob5L34= -golang.org/x/crypto v0.36.0/go.mod h1:Y4J0ReaxCR1IMaabaSMugxJES1EpwhBHhv2bDHklZvc= +golang.org/x/crypto v0.44.0 h1:A97SsFvM3AIwEEmTBiaxPPTYpDC47w720rdiiUvgoAU= +golang.org/x/crypto v0.44.0/go.mod h1:013i+Nw79BMiQiMsOPcVCB5ZIJbYkerPrGnOa00tvmc= golang.org/x/exp v0.0.0-20230626212559-97b1e661b5df h1:UA2aFVmmsIlefxMk29Dp2juaUSth8Pyn3Tq5Y5mJGME= golang.org/x/exp v0.0.0-20230626212559-97b1e661b5df/go.mod h1:FXUEEKJgO7OQYeo8N01OfiKP8RXMtf6e8aTskBGqWdc= -golang.org/x/sync v0.12.0 h1:MHc5BpPuC30uJk597Ri8TV3CNZcTLu6B6z4lJy+g6Jw= -golang.org/x/sync v0.12.0/go.mod h1:1dzgHSNfp02xaA81J2MS99Qcpr2w7fw1gpm99rleRqA= +golang.org/x/sync v0.18.0 h1:kr88TuHDroi+UVf+0hZnirlk8o8T+4MrK6mr60WkH/I= +golang.org/x/sync v0.18.0/go.mod h1:9KTHXmSnoGruLpwFjVSX0lNNA75CykiMECbovNTZqGI= golang.org/x/sys v0.0.0-20190916202348-b4ddaad3f8a3/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.1.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.8.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.11.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.39.0 h1:CvCKL8MeisomCi6qNZ+wbb0DN9E5AATixKsvNtMoMFk= golang.org/x/sys v0.39.0/go.mod h1:OgkHotnGiDImocRcuBABYBEXf8A9a87e/uXjp9XT3ks= -golang.org/x/text v0.23.0 h1:D71I7dUrlY+VX0gQShAThNGHFxZ13dGLBHQLVl1mJlY= -golang.org/x/text v0.23.0/go.mod h1:/BLNzu4aZCJ1+kcD0DNRotWKage4q2rGVAg4o22unh4= -google.golang.org/protobuf v1.34.2 h1:6xV6lTsCfpGD21XK49h7MhtcApnLqkfYgPcdHftf6hg= -google.golang.org/protobuf v1.34.2/go.mod h1:qYOHts0dSfpeUzUFpOMr/WGzszTmLH+DiWniOlNbLDw= +golang.org/x/text v0.31.0 h1:aC8ghyu4JhP8VojJ2lEHBnochRno1sgL6nEi9WGFGMM= +golang.org/x/text v0.31.0/go.mod h1:tKRAlv61yKIjGGHX/4tP1LTbc13YSec1pxVEWXzfoeM= +google.golang.org/protobuf v1.36.11 h1:fV6ZwhNocDyBLK0dj+fg8ektcVegBBuEolpbTQyBNVE= +google.golang.org/protobuf v1.36.11/go.mod h1:HTf+CrKn2C3g5S8VImy6tdcUvCska2kB7j23XfzDpco= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c h1:Hei/4ADfdWqJk1ZMxUNpqntNwaWcugrBjAiHlqqRiVk= gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c/go.mod h1:JHkPIbrfpd72SG/EVd6muEfDQjcINNoR0C8j2r3qZ4Q= diff --git a/cmd/utils/flags.go b/cmd/utils/flags.go index c96a97175e..6e822d7d16 100644 --- a/cmd/utils/flags.go +++ b/cmd/utils/flags.go @@ -1043,30 +1043,15 @@ Please note that --` + MetricsHTTPFlag.Name + ` must be set to start the server. Category: flags.MetricsCategory, } - OTELTracingFlag = &cli.BoolFlag{ - Name: "otel.tracing", + OTELEnabledFlag = &cli.BoolFlag{ + Name: "otel.enabled", Usage: "Enable OpenTelemetry tracing", Category: flags.LoggingCategory, } OTELEndpointFlag = &cli.StringFlag{ Name: "otel.endpoint", - Usage: "OpenTelemetry collector endpoint (e.g., localhost:4317)", - Value: "localhost:4317", - Category: flags.LoggingCategory, - } - - OTELProtocolFlag = &cli.StringFlag{ - Name: "otel.protocol", - Usage: "OpenTelemetry protocol: grpc or http", - Value: "grpc", - Category: flags.LoggingCategory, - } - - OTELInsecureFlag = &cli.BoolFlag{ - Name: "otel.insecure", - Usage: "Use insecure connection to OpenTelemetry collector", - Value: false, + Usage: "Defines where OpenTelemetry traces are sent (e.g., http://localhost:4318)", Category: flags.LoggingCategory, } diff --git a/cmd/utils/otel.go b/cmd/utils/otel.go new file mode 100644 index 0000000000..31abd883ba --- /dev/null +++ b/cmd/utils/otel.go @@ -0,0 +1,131 @@ +// Copyright 2025 The go-ethereum Authors +// This file is part of go-ethereum. +// +// go-ethereum is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// go-ethereum is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with go-ethereum. If not, see . + +package utils + +import ( + "context" + "fmt" + "net/url" + "time" + + "github.com/ethereum/go-ethereum/log" + "github.com/ethereum/go-ethereum/node" + "github.com/urfave/cli/v2" + "go.opentelemetry.io/otel" + "go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp" + "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.24.0" +) + +// otelService wraps the OpenTelemetry TracerProvider to implement node.Lifecycle. +type otelService struct { + tracerProvider *sdktrace.TracerProvider +} + +// Start implements node.Lifecycle. +func (o *otelService) Start() error { + return nil // Provider is already started during setup +} + +// Stop implements node.Lifecycle +func (o *otelService) Stop() error { + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + if err := o.tracerProvider.Shutdown(ctx); err != nil { + log.Error("Failed to stop otelService", "err", err) + return err + } + log.Info("otelService stopped") + return nil +} + +// SetupOTEL initializes OpenTelemetry tracing based on CLI flags. +func SetupOTEL(ctx *cli.Context) (*otelService, error) { + if !ctx.Bool(OTELEnabledFlag.Name) { + return nil, nil + } + + endpoint := ctx.String(OTELEndpointFlag.Name) + if endpoint == "" { + return nil, nil + } + + serviceName := ctx.String(OTELServiceNameFlag.Name) + setupCtx := ctx.Context + if setupCtx == nil { + setupCtx = context.Background() + } + + // Create exporter based on endpoint URL + u, err := url.Parse(endpoint) + if err != nil { + return nil, fmt.Errorf("invalid otel endpoint URL: %w", err) + } + + var exporter sdktrace.SpanExporter + switch u.Scheme { + case "http", "https": + opts := []otlptracehttp.Option{ + otlptracehttp.WithEndpoint(endpoint), + } + if u.Scheme == "http" { + opts = append(opts, otlptracehttp.WithInsecure()) + } + exporter, err = otlptracehttp.New(setupCtx, opts...) + default: + return nil, fmt.Errorf("unsupported otel url scheme: %s", u.Scheme) + } + + if err != nil { + return nil, fmt.Errorf("failed to create otel exporter: %w", err) + } + + // Create resource with service information + res, err := resource.New(setupCtx, + resource.WithAttributes( + semconv.ServiceName(serviceName), + ), + ) + if err != nil { + return nil, fmt.Errorf("failed to create otel resource: %w", err) + } + + // Create TracerProvider + tp := sdktrace.NewTracerProvider( + sdktrace.WithBatcher(exporter), + sdktrace.WithResource(res), + ) + + // Set as global provider + otel.SetTracerProvider(tp) + + // Set global propagator for context propagation + otel.SetTextMapPropagator(propagation.NewCompositeTextMapPropagator( + propagation.TraceContext{}, + propagation.Baggage{}, + )) + + log.Info("OpenTelemetry tracing enabled", "endpoint", endpoint, "service", serviceName) + return &otelService{tracerProvider: tp}, nil +} + +// RegisterOTELService registers the otelService with the node so its lifecycle is managed by the node. +func RegisterOTELService(otelService *otelService, stack *node.Node) { + stack.RegisterLifecycle(otelService) +} diff --git a/go.mod b/go.mod index 59b26c4e3c..faff5775ef 100644 --- a/go.mod +++ b/go.mod @@ -61,9 +61,9 @@ require ( github.com/syndtr/goleveldb v1.0.1-0.20210819022825-2ae1ddf74ef7 github.com/urfave/cli/v2 v2.27.5 go.opentelemetry.io/otel v1.39.0 + go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp v1.39.0 go.opentelemetry.io/otel/sdk v1.39.0 go.opentelemetry.io/otel/trace v1.39.0 - go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp v1.39.0 go.uber.org/automaxprocs v1.5.2 go.uber.org/goleak v1.3.0 golang.org/x/crypto v0.44.0 @@ -79,14 +79,13 @@ require ( ) require ( + github.com/cenkalti/backoff/v5 v5.0.3 // indirect github.com/go-logr/logr v1.4.3 // indirect github.com/go-logr/stdr v1.2.2 // indirect - go.opentelemetry.io/auto/sdk v1.2.1 // indirect - github.com/cenkalti/backoff/v5 v5.0.3 // indirect github.com/grpc-ecosystem/grpc-gateway/v2 v2.27.3 // indirect + go.opentelemetry.io/auto/sdk v1.2.1 // indirect go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.39.0 // indirect go.opentelemetry.io/otel/metric v1.39.0 // indirect - go.opentelemetry.io/otel/trace v1.39.0 // indirect go.opentelemetry.io/proto/otlp v1.9.0 // indirect google.golang.org/genproto/googleapis/api v0.0.0-20251222181119-0a764e51fe1b // indirect google.golang.org/genproto/googleapis/rpc v0.0.0-20251222181119-0a764e51fe1b // indirect diff --git a/go.sum b/go.sum index 3bbbd52e05..fe2a64eaec 100644 --- a/go.sum +++ b/go.sum @@ -384,8 +384,6 @@ go.opentelemetry.io/otel v1.39.0 h1:8yPrr/S0ND9QEfTfdP9V+SiwT4E0G7Y5MO7p85nis48= go.opentelemetry.io/otel v1.39.0/go.mod h1:kLlFTywNWrFyEdH0oj2xK0bFYZtHRYUdv1NklR/tgc8= go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.39.0 h1:f0cb2XPmrqn4XMy9PNliTgRKJgS5WcL/u0/WRYGz4t0= go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.39.0/go.mod h1:vnakAaFckOMiMtOIhFI2MNH4FYrZzXCYxmb1LlhoGz8= -go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.39.0 h1:in9O8ESIOlwJAEGTkkf34DesGRAc/Pn8qJ7k3r/42LM= -go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.39.0/go.mod h1:Rp0EXBm5tfnv0WL+ARyO/PHBEaEAT8UUHQ6AGJcSq6c= go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp v1.39.0 h1:Ckwye2FpXkYgiHX7fyVrN1uA/UYd9ounqqTuSNAv0k4= go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp v1.39.0/go.mod h1:teIFJh5pW2y+AN7riv6IBPX2DuesS3HgP39mwOspKwU= go.opentelemetry.io/otel/metric v1.39.0 h1:d1UzonvEZriVfpNKEVmHXbdf909uGTOQjA0HF0Ls5Q0= diff --git a/internal/otel/otel.go b/internal/otel/otel.go deleted file mode 100644 index f964750f65..0000000000 --- a/internal/otel/otel.go +++ /dev/null @@ -1,120 +0,0 @@ -// Copyright 2025 The go-ethereum Authors -// This file is part of the go-ethereum library. -// -// The go-ethereum library is free software: you can redistribute it and/or modify -// it under the terms of the GNU Lesser General Public License as published by -// the Free Software Foundation, either version 3 of the License, or -// (at your option) any later version. -// -// The go-ethereum library is distributed in the hope that it will be useful, -// but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU Lesser General Public License for more details. -// -// You should have received a copy of the GNU Lesser General Public License -// along with the go-ethereum library. If not, see . - -// Package otel provides OpenTelemetry tracing configuration for geth. -package otel - -import ( - "context" - "fmt" - "time" - - "github.com/ethereum/go-ethereum/cmd/utils" - "github.com/ethereum/go-ethereum/log" - "github.com/urfave/cli/v2" - "go.opentelemetry.io/otel" - "go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc" - "go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp" - "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.24.0" -) - -var ( - tracerProvider *sdktrace.TracerProvider -) - -// Setup initializes OpenTelemetry tracing based on CLI flags. -func Setup(ctx *cli.Context) error { - if !ctx.Bool(utils.OTELTracingFlag.Name) { - return nil - } - endpoint := ctx.String(utils.OTELEndpointFlag.Name) - protocol := ctx.String(utils.OTELProtocolFlag.Name) - insecure := ctx.Bool(utils.OTELInsecureFlag.Name) - serviceName := ctx.String(utils.OTELServiceNameFlag.Name) - - setupCtx := ctx.Context - if setupCtx == nil { - setupCtx = context.Background() - } - - // Create resource with service information - res, err := resource.New(setupCtx, - resource.WithAttributes( - semconv.ServiceName(serviceName), - ), - ) - if err != nil { - return fmt.Errorf("failed to create otel resource: %w", err) - } - - // Create exporter based on protocol - var exporter sdktrace.SpanExporter - switch protocol { - case "grpc": - opts := []otlptracegrpc.Option{ - otlptracegrpc.WithEndpoint(endpoint), - } - if insecure { - opts = append(opts, otlptracegrpc.WithInsecure()) - } - exporter, err = otlptracegrpc.New(setupCtx, opts...) - case "http": - opts := []otlptracehttp.Option{ - otlptracehttp.WithEndpoint(endpoint), - } - if insecure { - opts = append(opts, otlptracehttp.WithInsecure()) - } - exporter, err = otlptracehttp.New(setupCtx, opts...) - default: - return fmt.Errorf("unsupported otel protocol: %s (use 'grpc' or 'http')", protocol) - } - if err != nil { - return fmt.Errorf("failed to create otel exporter: %w", err) - } - - // Create TracerProvider - tracerProvider = sdktrace.NewTracerProvider( - sdktrace.WithBatcher(exporter), - sdktrace.WithResource(res), - ) - - // Set as global provider - otel.SetTracerProvider(tracerProvider) - - // Set global propagator for context propagation - otel.SetTextMapPropagator(propagation.NewCompositeTextMapPropagator( - propagation.TraceContext{}, - propagation.Baggage{}, - )) - - log.Info("OpenTelemetry tracing enabled", "endpoint", endpoint, "protocol", protocol, "service", serviceName) - return nil -} - -// Exit shuts down the OpenTelemetry TracerProvider, flushing any remaining spans. -func Exit() { - if tracerProvider != nil { - ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) - defer cancel() - if err := tracerProvider.Shutdown(ctx); err != nil { - log.Error("Failed to shutdown OpenTelemetry tracer provider", "err", err) - } - } -}