From 0afe63641291ed64e345e75d112ca4a352c6d5fb Mon Sep 17 00:00:00 2001 From: Cedric Cordenier Date: Thu, 22 Oct 2026 13:48:17 +0100 Subject: [PATCH] CRE-7119: Use common's loop/pyroscope for the profiler --- libs/capability/config.go | 2 +- libs/capability/doccomments_gen.go | 8 ---- libs/capability/internal/docgen/main.go | 1 - libs/capability/observability.go | 5 ++- libs/capability/observability_test.go | 5 ++- libs/capability/pyroscope.go | 56 ++++-------------------- libs/capability/pyroscope_compat_test.go | 47 ++++++++++++++++++++ libs/capability/runner.go | 2 +- libs/capability/telemetry_test.go | 20 ++++++--- libs/go.mod | 4 +- libs/go.sum | 4 +- 11 files changed, 81 insertions(+), 73 deletions(-) create mode 100644 libs/capability/pyroscope_compat_test.go diff --git a/libs/capability/config.go b/libs/capability/config.go index 6068891c6..d43c3330d 100644 --- a/libs/capability/config.go +++ b/libs/capability/config.go @@ -50,7 +50,7 @@ func (c *config) bind(root *cobra.Command) (*cli.Binder, error) { } return b, errors.Join( b.Register(root, &c.observability.TelemetryConfig), - b.RegisterInNamespace(root, pyroscopeNamespace, &c.observability.pyroscope), + b.Register(root, &c.observability.PyroscopeConfig), b.RegisterInNamespace(root, httpNamespace, &c.observability.http), b.RegisterInNamespace(root, capabilitiesNamespace, &c.capabilities), b.RegisterInNamespace(root, grpcNamespace, &c.grpc), diff --git a/libs/capability/doccomments_gen.go b/libs/capability/doccomments_gen.go index 7c6329da1..2cbc95a91 100644 --- a/libs/capability/doccomments_gen.go +++ b/libs/capability/doccomments_gen.go @@ -24,14 +24,6 @@ func (HTTPConfig) DocComments() map[string]commentparsing.FieldDoc { } } -func (PyroscopeConfig) DocComments() map[string]commentparsing.FieldDoc { - return map[string]commentparsing.FieldDoc{ - "AuthToken": {Comment: "AuthToken is the pyroscope auth token."}, - "Environment": {Comment: "Environment is the environment tag attached to profiles."}, - "ServerAddress": {Comment: "ServerAddress is the pyroscope server address. Profiling is disabled when unset."}, - } -} - func (SettingsConfig) DocComments() map[string]commentparsing.FieldDoc { return map[string]commentparsing.FieldDoc{ "URL": {Comment: "URL is the gRPC target of the host's CRE settings service (e.g. localhost:9001), which pushes\nsettings updates to this process. Unset leaves every limit at its compiled-in default."}, diff --git a/libs/capability/internal/docgen/main.go b/libs/capability/internal/docgen/main.go index fd654a969..06ae47b2e 100644 --- a/libs/capability/internal/docgen/main.go +++ b/libs/capability/internal/docgen/main.go @@ -15,7 +15,6 @@ func main() { if err := commentparsing.Run(commentparsing.RunArgs{ Markup: tomlmarkup.New(), Roots: []any{ - &capability.PyroscopeConfig{}, &capability.HTTPConfig{}, &capability.CapabilitiesConfig{}, &capability.GRPCConfig{}, diff --git a/libs/capability/observability.go b/libs/capability/observability.go index 441a3c1c9..d5506c183 100644 --- a/libs/capability/observability.go +++ b/libs/capability/observability.go @@ -4,6 +4,7 @@ import ( "go.opentelemetry.io/otel/sdk/metric" "github.com/smartcontractkit/chainlink-common/pkg/beholder" + "github.com/smartcontractkit/chainlink-common/pkg/loop/pyroscope" "github.com/smartcontractkit/chainlink-common/pkg/loop/telemetry" ) @@ -16,8 +17,8 @@ const ( // the command runs and they have been decoded. type observability struct { telemetry.TelemetryConfig - pyroscope PyroscopeConfig - http HTTPConfig + pyroscope.PyroscopeConfig + http HTTPConfig otelViews []metric.View // supplied through capability.Run } diff --git a/libs/capability/observability_test.go b/libs/capability/observability_test.go index 57e3fc182..c3c720c40 100644 --- a/libs/capability/observability_test.go +++ b/libs/capability/observability_test.go @@ -12,6 +12,7 @@ import ( "github.com/smartcontractkit/chainlink-common/pkg/beholder" "github.com/smartcontractkit/chainlink-common/pkg/logger" + "github.com/smartcontractkit/chainlink-common/pkg/loop/pyroscope" "github.com/smartcontractkit/chainlink-common/pkg/services" ) @@ -21,7 +22,7 @@ import ( // Nil rather than a no-op matters to the caller: the root would take a no-op and report on it, and // a health report mentioning profiling that is not happening is worse than not mentioning it. func TestProfilerWithoutAServer(t *testing.T) { - assert.Nil(t, newProfiler(logger.Test(t), "cron", PyroscopeConfig{})) + assert.Nil(t, newProfiler(logger.Test(t), "cron", pyroscope.PyroscopeConfig{})) } // TestProfilerWithAServer covers the other side: a configured one is a service the root can take, @@ -30,7 +31,7 @@ func TestProfilerWithoutAServer(t *testing.T) { // It is built rather than started. Starting one dials a pyroscope server, which a test has no // business standing up to prove a constructor returned something. func TestProfilerWithAServer(t *testing.T) { - p := newProfiler(logger.Test(t), "cron", PyroscopeConfig{ServerAddress: "pyro:4040"}) + p := newProfiler(logger.Test(t), "cron", pyroscope.PyroscopeConfig{PyroscopeServerAddress: "pyro:4040"}) require.NotNil(t, p) assert.Equal(t, "Profiler", p.Name()) diff --git a/libs/capability/pyroscope.go b/libs/capability/pyroscope.go index b1ff98229..d72791d4a 100644 --- a/libs/capability/pyroscope.go +++ b/libs/capability/pyroscope.go @@ -2,34 +2,20 @@ package capability import ( "context" - "fmt" - "runtime/debug" - "github.com/grafana/pyroscope-go" - - commonconfig "github.com/smartcontractkit/chainlink-common/pkg/config" "github.com/smartcontractkit/chainlink-common/pkg/logger" + "github.com/smartcontractkit/chainlink-common/pkg/loop/pyroscope" "github.com/smartcontractkit/chainlink-common/pkg/services" ) -// PyroscopeConfig configures continuous profiling. An empty ServerAddress leaves profiling off. -type PyroscopeConfig struct { - // ServerAddress is the pyroscope server address. Profiling is disabled when unset. - ServerAddress string - // AuthToken is the pyroscope auth token. - AuthToken commonconfig.SecretString - // Environment is the environment tag attached to profiles. - Environment string -} - // newProfiler returns continuous profiling as a service, or nil when no pyroscope server is // configured. -func newProfiler(lggr logger.Logger, appName string, cfg PyroscopeConfig) *profilerService { - if cfg.ServerAddress == "" { +func newProfiler(lggr logger.Logger, appName string, cfg pyroscope.PyroscopeConfig) *profilerService { + if cfg.PyroscopeServerAddress == "" { return nil } - p := &profilerService{appName: appName, cfg: cfg} + p := &profilerService{lggr: lggr, appName: appName, cfg: cfg} p.Service, _ = services.Config{ Name: "Profiler", Start: p.start, @@ -38,7 +24,7 @@ func newProfiler(lggr logger.Logger, appName string, cfg PyroscopeConfig) *profi return p } -func startProfiler(ctx context.Context, lggr logger.Logger, appName string, cfg PyroscopeConfig) (*profilerService, error) { +func startProfiler(ctx context.Context, lggr logger.Logger, appName string, cfg pyroscope.PyroscopeConfig) (*profilerService, error) { profiler := newProfiler(lggr, appName, cfg) if profiler == nil { return nil, nil @@ -50,8 +36,9 @@ func startProfiler(ctx context.Context, lggr logger.Logger, appName string, cfg type profilerService struct { services.Service + lggr logger.Logger appName string - cfg PyroscopeConfig + cfg pyroscope.PyroscopeConfig // profiler is what start made and close stops. Written by one hook and read by the other, which // the state machine's lock orders: a service cannot be closed unless it started. @@ -59,34 +46,9 @@ type profilerService struct { } func (p *profilerService) start(context.Context) error { - var ver, sha string - if bi, ok := debug.ReadBuildInfo(); ok { - ver = bi.Main.Version - sha = bi.Main.Sum - if len(sha) > 7 { - sha = sha[:7] - } - } - - profiler, err := pyroscope.Start(pyroscope.Config{ - ApplicationName: p.appName, - ServerAddress: p.cfg.ServerAddress, - AuthToken: string(p.cfg.AuthToken), - Tags: map[string]string{ - "version": ver, - "sha": sha, - "environment": p.cfg.Environment, - }, - ProfileTypes: []pyroscope.ProfileType{ - pyroscope.ProfileCPU, - pyroscope.ProfileAllocObjects, - pyroscope.ProfileAllocSpace, - pyroscope.ProfileInuseObjects, - pyroscope.ProfileInuseSpace, - }, - }) + profiler, err := pyroscope.Start(p.lggr, p.cfg, p.appName, false) if err != nil { - return fmt.Errorf("failed to start profiler: %w", err) + return err } p.profiler = profiler diff --git a/libs/capability/pyroscope_compat_test.go b/libs/capability/pyroscope_compat_test.go new file mode 100644 index 000000000..f2394bc28 --- /dev/null +++ b/libs/capability/pyroscope_compat_test.go @@ -0,0 +1,47 @@ +package capability + +import ( + "reflect" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/smartcontractkit/chainlink-common/pkg/loop/pyroscope" +) + +// TestPyroscopeConfigRoundTripsThroughLoopEnv is the contract with a LOOP host: the pyroscope env it +// sets with loop.EnvConfig.AsCmdEnv starts the same profiler here as loop.Server would. +func TestPyroscopeConfigRoundTripsThroughLoopEnv(t *testing.T) { + want := pyroscope.PyroscopeConfig{ + PyroscopeAuthToken: "token", + PyroscopeServerAddress: "pyroscope:4040", + PyroscopeEnvironment: "staging", + PyroscopeLinkTracesToProfiles: true, + PyroscopePPROFBlockProfileRate: 7, + PyroscopePPROFMutexProfileFraction: 9, + } + requireNoZeroPyroscopeFields(t, want) + + for _, kv := range want.AsCmdEnv() { + k, v, ok := strings.Cut(kv, "=") + require.True(t, ok, "malformed env entry %q", kv) + if strings.HasPrefix(k, "CL_PYROSCOPE_") { + t.Setenv(k, v) + } + } + + assert.Equal(t, want, decoded(t).observability.PyroscopeConfig) +} + +// requireNoZeroPyroscopeFields fails on any field but AppName left at its zero value, so a field +// added to pyroscope.Config can't go untested by defaulting through on both sides. +func requireNoZeroPyroscopeFields(t *testing.T, cfg pyroscope.PyroscopeConfig) { + t.Helper() + v := reflect.ValueOf(cfg) + for i := range v.NumField() { + name := v.Type().Field(i).Name + require.False(t, v.Field(i).IsZero(), "%s is unset; give it a non-default value", name) + } +} diff --git a/libs/capability/runner.go b/libs/capability/runner.go index 45235f867..37379db25 100644 --- a/libs/capability/runner.go +++ b/libs/capability/runner.go @@ -27,7 +27,7 @@ func run(ctx context.Context, lggr logger.Logger, name string, cfg *config, newC mux := http.NewServeMux() - profiler, err := startProfiler(ctx, lggr, name, cfg.observability.pyroscope) + profiler, err := startProfiler(ctx, lggr, name, cfg.observability.PyroscopeConfig) if err != nil { return fmt.Errorf("failed to start profiler: %w", err) } diff --git a/libs/capability/telemetry_test.go b/libs/capability/telemetry_test.go index c749f65a3..aafad36dd 100644 --- a/libs/capability/telemetry_test.go +++ b/libs/capability/telemetry_test.go @@ -53,6 +53,12 @@ var undocumentedLoopFlags = []string{ "chip-ingress-max-grpc-request-size", "chip-ingress-send-interval", "chip-ingress-send-timeout", + "pyroscope-auth-token", + "pyroscope-environment", + "pyroscope-link-traces-to-profiles", + "pyroscope-pprof-block-profile-rate", + "pyroscope-pprof-mutex-profile-fraction", + "pyroscope-server-address", "telemetry-attribute", "telemetry-attribute.", "telemetry-auth-header", @@ -128,9 +134,9 @@ func TestObservabilityFlags(t *testing.T) { "chip-ingress-endpoint", "chip-ingress-insecure-connection", - "pyroscope.server-address", - "pyroscope.auth-token", - "pyroscope.environment", + "pyroscope-server-address", + "pyroscope-auth-token", + "pyroscope-environment", "http.port", @@ -203,8 +209,8 @@ func TestObservabilityDecodesFromFlags(t *testing.T) { "--tracing-tls-cert-path", "/certs/trace.pem", "--chip-ingress-endpoint", "chip:9000", "--chip-ingress-insecure-connection", "true", - "--pyroscope.server-address", "pyro:4040", - "--pyroscope.environment", "staging", + "--pyroscope-server-address", "pyro:4040", + "--pyroscope-environment", "staging", "--telemetry-endpoint", "otel:4317", "--telemetry-insecure-connection", "true", "--telemetry-attribute", "env=staging", @@ -226,8 +232,8 @@ func TestObservabilityDecodesFromFlags(t *testing.T) { assert.Equal(t, "chip:9000", obs.ChipIngressEndpoint) assert.True(t, obs.ChipIngressInsecureConnection) - assert.Equal(t, "pyro:4040", obs.pyroscope.ServerAddress) - assert.Equal(t, "staging", obs.pyroscope.Environment) + assert.Equal(t, "pyro:4040", obs.PyroscopeServerAddress) + assert.Equal(t, "staging", obs.PyroscopeEnvironment) } // TestTelemetryDecodesFromEnv covers why the namespace and field names are what they are: a diff --git a/libs/go.mod b/libs/go.mod index 9f0646317..af2e44160 100644 --- a/libs/go.mod +++ b/libs/go.mod @@ -6,14 +6,13 @@ require ( github.com/cenkalti/backoff/v5 v5.0.3 github.com/google/go-cmp v0.7.0 github.com/google/uuid v1.6.0 - github.com/grafana/pyroscope-go v1.2.8 github.com/hashicorp/go-plugin v1.8.0 github.com/jonboulle/clockwork v0.5.0 github.com/prometheus/client_golang v1.23.2 github.com/samber/do/v2 v2.1.0 github.com/samber/go-type-to-string v1.8.0 github.com/shopspring/decimal v1.4.0 - github.com/smartcontractkit/chainlink-common v0.11.2-0.20261022105827-74f78e92fb70 + github.com/smartcontractkit/chainlink-common v0.11.2-0.20261022160347-6812ecc7b2db github.com/smartcontractkit/chainlink-common/x/config v0.0.0-20261014172408-e5560b3eeb33 github.com/smartcontractkit/chainlink-protos/cre/go v0.0.0-20261013212015-e622e8b83b6b github.com/smartcontractkit/libocr v0.0.0-20260810200708-618b5bf7f342 @@ -50,6 +49,7 @@ require ( github.com/go-viper/mapstructure/v2 v2.5.0 // indirect github.com/golang/protobuf v1.5.4 // indirect github.com/grafana/otel-profiling-go v0.5.1 // indirect + github.com/grafana/pyroscope-go v1.2.8 // indirect github.com/grafana/pyroscope-go/godeltaprof v0.1.9 // indirect github.com/grpc-ecosystem/go-grpc-middleware/providers/prometheus v1.0.1 // indirect github.com/grpc-ecosystem/go-grpc-middleware/v2 v2.3.2 // indirect diff --git a/libs/go.sum b/libs/go.sum index 7eb01ac9e..5eeb305e4 100644 --- a/libs/go.sum +++ b/libs/go.sum @@ -224,8 +224,8 @@ github.com/shopspring/decimal v1.4.0 h1:bxl37RwXBklmTi0C79JfXCEBD1cqqHt0bbgBAGFp github.com/shopspring/decimal v1.4.0/go.mod h1:gawqmDU56v4yIKSwfBSFip1HdCCXN8/+DMd9qYNcwME= github.com/smartcontractkit/chain-selectors v1.0.104 h1:/n9pPGM5W/+r1eHoWZv4VwX9LNS1af4+ICyhM8zKRNM= github.com/smartcontractkit/chain-selectors v1.0.104/go.mod h1:qy7whtgG5g+7z0jt0nRyii9bLND9m15NZTzuQPkMZ5w= -github.com/smartcontractkit/chainlink-common v0.11.2-0.20261022105827-74f78e92fb70 h1:onP1IHy0c3yG5x9DRJKcn0SlYZijM3hRRlX08AoFAHQ= -github.com/smartcontractkit/chainlink-common v0.11.2-0.20261022105827-74f78e92fb70/go.mod h1:ZRHl1hjJyTPrnv669e3thza3U7Z2as5zKCDZP0kJXuw= +github.com/smartcontractkit/chainlink-common v0.11.2-0.20261022160347-6812ecc7b2db h1:+XNlv+GFy2d5HQ7oRzAY5GrjTKaeiY88+OQLrd7+mts= +github.com/smartcontractkit/chainlink-common v0.11.2-0.20261022160347-6812ecc7b2db/go.mod h1:ZRHl1hjJyTPrnv669e3thza3U7Z2as5zKCDZP0kJXuw= github.com/smartcontractkit/chainlink-common/pkg/chipingress v0.0.11-0.20261005132539-b7d63d5fd549 h1:J4vYRJxkBO8Yf//17wScKjYWmS45VZaFWx0zt+4ETEU= github.com/smartcontractkit/chainlink-common/pkg/chipingress v0.0.11-0.20261005132539-b7d63d5fd549/go.mod h1:Scyk7O80fde5rJF3Ty3fEE3xiS6j84eWbu8/2gl3pIY= github.com/smartcontractkit/chainlink-common/x/config v0.0.0-20261014172408-e5560b3eeb33 h1:9aS43MBP2grzsxpppcqosVkaeKFEfqfcIG2puIec3y4=