Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

deps!: Bumps go-header #2844

Merged
merged 8 commits into from
Oct 13, 2023
Merged
Prev Previous commit
Next Next commit
refactor(nodebuilder/header): remove config value and make it a var i…
…nstead
renaynay committed Oct 13, 2023

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.
commit 1bda27f371774ef1da20b79f25deeaa1e2ab3d5c
4 changes: 2 additions & 2 deletions cmd/flags_misc.go
Original file line number Diff line number Diff line change
@@ -122,7 +122,7 @@ and their lower-case forms`,
}

// ParseMiscFlags parses miscellaneous flags from the given cmd and applies values to Env.
func ParseMiscFlags(ctx context.Context, cmd *cobra.Command, cfg *nodebuilder.Config) (context.Context, error) {
func ParseMiscFlags(ctx context.Context, cmd *cobra.Command) (context.Context, error) {
logLevel := cmd.Flag(LogLevelFlag).Value.String()
if logLevel != "" {
level, err := logging.LevelFromString(logLevel)
@@ -238,7 +238,7 @@ func ParseMiscFlags(ctx context.Context, cmd *cobra.Command, cfg *nodebuilder.Co
opts = append(opts, otlpmetrichttp.WithInsecure())
}

ctx = WithNodeOptions(ctx, nodebuilder.WithMetrics(opts, NodeType(ctx), cfg))
ctx = WithNodeOptions(ctx, nodebuilder.WithMetrics(opts, NodeType(ctx)))
}

ok, err = cmd.Flags().GetBool(p2pMetrics)
2 changes: 1 addition & 1 deletion cmd/util.go
Original file line number Diff line number Diff line change
@@ -111,7 +111,7 @@ func PersistentPreRunEnv(cmd *cobra.Command, nodeType node.Type, _ []string) err
}
}

ctx, err = ParseMiscFlags(ctx, cmd, &cfg)
ctx, err = ParseMiscFlags(ctx, cmd)
if err != nil {
return err
}
5 changes: 2 additions & 3 deletions nodebuilder/header/config.go
Original file line number Diff line number Diff line change
@@ -16,6 +16,8 @@ import (
"github.com/celestiaorg/celestia-node/nodebuilder/p2p"
)

var MetricsEnabled = false

// Config contains configuration parameters for header retrieval and management.
type Config struct {
// TrustedHash is the Block/Header hash that Nodes use as starting point for header synchronization.
@@ -31,9 +33,6 @@ type Config struct {

Server p2p_exchange.ServerParameters
Client p2p_exchange.ClientParameters `toml:",omitempty"`

// enables metrics for the entire header module
MetricsEnabled bool
}

func DefaultConfig(tp node.Type) Config {
6 changes: 3 additions & 3 deletions nodebuilder/header/constructors.go
Original file line number Diff line number Diff line change
@@ -47,7 +47,7 @@ func newP2PExchange[H libhead.Header[H]](
p2p.WithChainID(network.String()),
p2p.WithPeerIDStore[p2p.ClientParameters](pidstore),
}
if cfg.MetricsEnabled {
if MetricsEnabled {
opts = append(opts, p2p.WithMetrics[p2p.ClientParameters]())
}

@@ -75,7 +75,7 @@ func newSyncer[H libhead.Header[H]](
cfg Config,
) (*sync.Syncer[H], *modfraud.ServiceBreaker[*sync.Syncer[H], H], error) {
opts := []sync.Option{sync.WithParams(cfg.Syncer), sync.WithBlockTime(modp2p.BlockTime)}
if cfg.MetricsEnabled {
if MetricsEnabled {
opts = append(opts, sync.WithMetrics())
}

@@ -104,7 +104,7 @@ func newInitStore[H libhead.Header[H]](
return nil, err
}

if cfg.MetricsEnabled {
if MetricsEnabled {
err = libhead.WithMetrics[H](s)
if err != nil {
return nil, err
6 changes: 3 additions & 3 deletions nodebuilder/header/module.go
Original file line number Diff line number Diff line change
@@ -50,9 +50,9 @@ func ConstructModule[H libhead.Header[H]](tp node.Type, cfg *Config) fx.Option {
}),
)),
fx.Provide(fx.Annotate(
func(cfg Config, ps *pubsub.PubSub, network modp2p.Network) (*p2p.Subscriber[H], error) {
func(ps *pubsub.PubSub, network modp2p.Network) (*p2p.Subscriber[H], error) {
opts := []p2p.SubscriberOption{p2p.WithSubscriberNetworkID(network.String())}
if cfg.MetricsEnabled {
if MetricsEnabled {
opts = append(opts, p2p.WithSubscriberMetrics())
}
return p2p.NewSubscriber[H](ps, header.MsgID, opts...)
@@ -75,7 +75,7 @@ func ConstructModule[H libhead.Header[H]](tp node.Type, cfg *Config) fx.Option {
p2p.WithParams(cfg.Server),
p2p.WithNetworkID[p2p.ServerParameters](network.String()),
}
if cfg.MetricsEnabled {
if MetricsEnabled {
opts = append(opts, p2p.WithMetrics[p2p.ServerParameters]())
}

5 changes: 1 addition & 4 deletions nodebuilder/node_test.go
Original file line number Diff line number Diff line change
@@ -67,18 +67,15 @@ func TestLifecycle_WithMetrics(t *testing.T) {

for i, tt := range test {
t.Run(strconv.Itoa(i), func(t *testing.T) {
cfg := DefaultConfig(tt.tp)
node := TestNodeWithConfig(
node := TestNode(
t,
tt.tp,
cfg,
WithMetrics(
[]otlpmetrichttp.Option{
otlpmetrichttp.WithEndpoint(otelCollectorURL),
otlpmetrichttp.WithInsecure(),
},
tt.tp,
cfg,
),
)
require.NotNil(t, node)
5 changes: 3 additions & 2 deletions nodebuilder/settings.go
Original file line number Diff line number Diff line change
@@ -24,6 +24,7 @@ import (

"github.com/celestiaorg/celestia-node/header"
"github.com/celestiaorg/celestia-node/nodebuilder/das"
modhead "github.com/celestiaorg/celestia-node/nodebuilder/header"
"github.com/celestiaorg/celestia-node/nodebuilder/node"
"github.com/celestiaorg/celestia-node/nodebuilder/p2p"
"github.com/celestiaorg/celestia-node/nodebuilder/share"
@@ -70,10 +71,10 @@ func WithPyroscope(endpoint string, nodeType node.Type) fx.Option {
}

// WithMetrics enables metrics exporting for the node.
func WithMetrics(metricOpts []otlpmetrichttp.Option, nodeType node.Type, config *Config) fx.Option {
func WithMetrics(metricOpts []otlpmetrichttp.Option, nodeType node.Type) fx.Option {
// TODO @renaynay: this will be refactored when there is more granular
// control over which module to enable metrics for
config.Header.MetricsEnabled = true
modhead.MetricsEnabled = true

baseComponents := fx.Options(
fx.Supply(metricOpts),