Skip to content

Commit

Permalink
feat!: Provide logger through depinject (#15818)
Browse files Browse the repository at this point in the history
Co-authored-by: Julien Robert <julien@rbrt.fr>
  • Loading branch information
facundomedica and julienrbrt authored Apr 24, 2023
1 parent 8e896f4 commit af3122a
Show file tree
Hide file tree
Showing 58 changed files with 503 additions and 176 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ Ref: https://keepachangelog.com/en/1.0.0/

### Features

* (runtime) [#15818](https://github.com/cosmos/cosmos-sdk/pull/15818) Provide logger through `depinject` instead of appBuilder.
* (client) [#15597](https://github.com/cosmos/cosmos-sdk/pull/15597) Add status endpoint for clients.
* (testutil/integration) [#15556](https://github.com/cosmos/cosmos-sdk/pull/15556) Introduce `testutil/integration` package for module integration testing.
* (types) [#15735](https://github.com/cosmos/cosmos-sdk/pull/15735) Make `ValidateBasic() error` method of `Msg` interface optional. Modules should validate messages directly in their message handlers ([RFC 001](https://docs.cosmos.network/main/rfc/rfc-001-tx-validation)).
Expand Down Expand Up @@ -110,6 +111,7 @@ Ref: https://keepachangelog.com/en/1.0.0/

### API Breaking Changes

* (x/bank) [#15818](https://github.com/cosmos/cosmos-sdk/issues/15818) `BaseViewKeeper`'s `Logger` method now doesn't require a context. `NewBaseKeeper`, `NewBaseSendKeeper` and `NewBaseViewKeeper` now also require a `log.Logger` to be passed in.
* (client) [#15597](https://github.com/cosmos/cosmos-sdk/pull/15597) `RegisterNodeService` now requires a config parameter.
* (x/*all*) [#15648](https://github.com/cosmos/cosmos-sdk/issues/15648) Make `SetParams` consistent across all modules and validate the params at the message handling instead of `SetParams` method.
* (x/genutil) [#15679](https://github.com/cosmos/cosmos-sdk/pull/15679) `MigrateGenesisCmd` now takes a `MigrationMap` instead of having the SDK genesis migration hardcoded.
Expand Down
24 changes: 24 additions & 0 deletions UPGRADING.md
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,30 @@ app.ConsensusParamsKeeper = consensusparamkeeper.NewKeeper(
)
```

The following modules `NewKeeper` function now also take a `log.Logger`:

* `x/bank`


### depinject

For `depinject` users, now the logger must be supplied through the main `depinject.Inject` function instead of passing it to `appBuilder.Build`.

```diff
appConfig = depinject.Configs(
AppConfig,
depinject.Supply(
// supply the application options
appOpts,
+ logger,
...
```

```diff
- app.App = appBuilder.Build(logger, db, traceStore, baseAppOptions...)
+ app.App = appBuilder.Build(db, traceStore, baseAppOptions...)
```

### Packages

#### Store
Expand Down
22 changes: 13 additions & 9 deletions baseapp/block_gas_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,14 +82,18 @@ func TestBaseApp_BlockGas(t *testing.T) {
err error
)

err = depinject.Inject(configurator.NewAppConfig(
configurator.AuthModule(),
configurator.TxModule(),
configurator.ParamsModule(),
configurator.ConsensusModule(),
configurator.BankModule(),
configurator.StakingModule(),
),
err = depinject.Inject(
depinject.Configs(
configurator.NewAppConfig(
configurator.AuthModule(),
configurator.TxModule(),
configurator.ParamsModule(),
configurator.ConsensusModule(),
configurator.BankModule(),
configurator.StakingModule(),
),
depinject.Supply(log.NewNopLogger()),
),
&bankKeeper,
&accountKeeper,
&interfaceRegistry,
Expand All @@ -99,7 +103,7 @@ func TestBaseApp_BlockGas(t *testing.T) {
&appBuilder)
require.NoError(t, err)

bapp := appBuilder.Build(log.NewNopLogger(), dbm.NewMemDB(), nil)
bapp := appBuilder.Build(dbm.NewMemDB(), nil)
err = bapp.Load(true)
require.NoError(t, err)

Expand Down
10 changes: 8 additions & 2 deletions baseapp/grpcrouter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"github.com/stretchr/testify/require"

"cosmossdk.io/depinject"

"github.com/cosmos/cosmos-sdk/baseapp"
"github.com/cosmos/cosmos-sdk/codec/types"
"github.com/cosmos/cosmos-sdk/runtime"
Expand Down Expand Up @@ -55,10 +56,15 @@ func TestGRPCQueryRouter(t *testing.T) {
func TestRegisterQueryServiceTwice(t *testing.T) {
// Setup baseapp.
var appBuilder *runtime.AppBuilder
err := depinject.Inject(makeMinimalConfig(), &appBuilder)
err := depinject.Inject(
depinject.Configs(
makeMinimalConfig(),
depinject.Supply(log.NewTestLogger(t)),
),
&appBuilder)
require.NoError(t, err)
db := dbm.NewMemDB()
app := appBuilder.Build(log.NewTestLogger(t), db, nil)
app := appBuilder.Build(db, nil)

// First time registering service shouldn't panic.
require.NotPanics(t, func() {
Expand Down
25 changes: 19 additions & 6 deletions baseapp/msg_service_router_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (

"cosmossdk.io/depinject"
"cosmossdk.io/log"

"github.com/cosmos/cosmos-sdk/client/tx"
"github.com/cosmos/cosmos-sdk/codec"
codectypes "github.com/cosmos/cosmos-sdk/codec/types"
Expand All @@ -27,9 +28,13 @@ func TestRegisterMsgService(t *testing.T) {
appBuilder *runtime.AppBuilder
registry codectypes.InterfaceRegistry
)
err := depinject.Inject(makeMinimalConfig(), &appBuilder, &registry)
err := depinject.Inject(
depinject.Configs(
makeMinimalConfig(),
depinject.Supply(log.NewTestLogger(t)),
), &appBuilder, &registry)
require.NoError(t, err)
app := appBuilder.Build(log.NewTestLogger(t), dbm.NewMemDB(), nil)
app := appBuilder.Build(dbm.NewMemDB(), nil)

require.Panics(t, func() {
testdata.RegisterMsgServer(
Expand All @@ -55,10 +60,14 @@ func TestRegisterMsgServiceTwice(t *testing.T) {
appBuilder *runtime.AppBuilder
registry codectypes.InterfaceRegistry
)
err := depinject.Inject(makeMinimalConfig(), &appBuilder, &registry)
err := depinject.Inject(
depinject.Configs(
makeMinimalConfig(),
depinject.Supply(log.NewTestLogger(t)),
), &appBuilder, &registry)
require.NoError(t, err)
db := dbm.NewMemDB()
app := appBuilder.Build(log.NewTestLogger(t), db, nil)
app := appBuilder.Build(db, nil)
testdata.RegisterInterfaces(registry)

// First time registering service shouldn't panic.
Expand Down Expand Up @@ -86,9 +95,13 @@ func TestMsgService(t *testing.T) {
cdc codec.ProtoCodecMarshaler
interfaceRegistry codectypes.InterfaceRegistry
)
err := depinject.Inject(makeMinimalConfig(), &appBuilder, &cdc, &interfaceRegistry)
err := depinject.Inject(
depinject.Configs(
makeMinimalConfig(),
depinject.Supply(log.NewNopLogger()),
), &appBuilder, &cdc, &interfaceRegistry)
require.NoError(t, err)
app := appBuilder.Build(log.NewNopLogger(), dbm.NewMemDB(), nil)
app := appBuilder.Build(dbm.NewMemDB(), nil)

// patch in TxConfig instead of using an output from x/auth/tx
txConfig := authtx.NewTxConfig(cdc, authtx.DefaultSignModes)
Expand Down
9 changes: 7 additions & 2 deletions client/grpc_query_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,15 @@ func (s *IntegrationTestSuite) SetupSuite() {
// TODO duplicated from testutils/sims/app_helpers.go
// need more composable startup options for simapp, this test needed a handle to the closed over genesis account
// to query balances
err := depinject.Inject(testutil.AppConfig, &interfaceRegistry, &bankKeeper, &appBuilder, &cdc)
err := depinject.Inject(
depinject.Configs(
testutil.AppConfig,
depinject.Supply(log.NewNopLogger()),
),
&interfaceRegistry, &bankKeeper, &appBuilder, &cdc)
s.NoError(err)

app := appBuilder.Build(log.NewNopLogger(), dbm.NewMemDB(), nil)
app := appBuilder.Build(dbm.NewMemDB(), nil)
err = app.Load(true)
s.NoError(err)

Expand Down
8 changes: 7 additions & 1 deletion crypto/armor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ import (
"github.com/stretchr/testify/require"

"cosmossdk.io/depinject"
"cosmossdk.io/log"

"github.com/cosmos/cosmos-sdk/codec"
"github.com/cosmos/cosmos-sdk/codec/legacy"
"github.com/cosmos/cosmos-sdk/crypto"
Expand Down Expand Up @@ -75,7 +77,11 @@ func TestArmorUnarmorPrivKey(t *testing.T) {
func TestArmorUnarmorPubKey(t *testing.T) {
// Select the encryption and storage for your cryptostore
var cdc codec.Codec
err := depinject.Inject(configurator.NewAppConfig(), &cdc)

err := depinject.Inject(depinject.Configs(
configurator.NewAppConfig(),
depinject.Supply(log.NewNopLogger()),
), &cdc)
require.NoError(t, err)

cstore := keyring.NewInMemory(cdc)
Expand Down
8 changes: 7 additions & 1 deletion crypto/keys/multisig/multisig_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ import (
"github.com/stretchr/testify/require"

"cosmossdk.io/depinject"
"cosmossdk.io/log"

"github.com/cosmos/cosmos-sdk/codec"
"github.com/cosmos/cosmos-sdk/codec/legacy"
"github.com/cosmos/cosmos-sdk/codec/types"
Expand Down Expand Up @@ -354,7 +356,11 @@ func TestDisplay(t *testing.T) {

require.NotEmpty(msig.String())
var cdc codec.Codec
err := depinject.Inject(configurator.NewAppConfig(), &cdc)
err := depinject.Inject(
depinject.Configs(
configurator.NewAppConfig(),
depinject.Supply(log.NewNopLogger()),
), &cdc)
require.NoError(err)
bz, err := cdc.MarshalInterfaceJSON(msig)
require.NoError(err)
Expand Down
28 changes: 28 additions & 0 deletions docs/architecture/adr-063-core-module-api.md
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,34 @@ the block or app hash is left to the runtime to specify).
Events emitted by `EmitKVEvent` and `EmitProtoEventNonConsensus` are not considered to be part of consensus and cannot be observed
by other modules. If there is a client-side need to add events in patch releases, these methods can be used.

#### Logger

A logger (`cosmossdk.io/log`) must be supplied using `depinject`, and will
be made available for modules to use via `depinject.In`.
Modules using it should follow the current pattern in the SDK by adding the module name before using it.

```go
type ModuleInputs struct {
depinject.In

Logger log.Logger
}

func ProvideModule(in ModuleInputs) ModuleOutputs {
keeper := keeper.NewKeeper(
in.logger,
)
}

func NewKeeper(logger log.Logger) Keeper {
return Keeper{
logger: logger.With(log.ModuleKey, "x/"+types.ModuleName),
}
}
```

```
### Core `AppModule` extension interfaces
Expand Down
3 changes: 3 additions & 0 deletions runtime/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import (

runtimev1alpha1 "cosmossdk.io/api/cosmos/app/runtime/v1alpha1"
appv1alpha1 "cosmossdk.io/api/cosmos/app/v1alpha1"
"cosmossdk.io/log"

storetypes "cosmossdk.io/store/types"
abci "github.com/cometbft/cometbft/abci/types"
"golang.org/x/exp/slices"
Expand Down Expand Up @@ -47,6 +49,7 @@ type App struct {
baseAppOptions []BaseAppOption
msgServiceRouter *baseapp.MsgServiceRouter
appConfig *appv1alpha1.Config
logger log.Logger
// initChainer is the init chainer function defined by the app config.
// this is only required if the chain wants to add special InitChainer logic.
initChainer sdk.InitChainer
Expand Down
4 changes: 1 addition & 3 deletions runtime/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"encoding/json"
"io"

"cosmossdk.io/log"
dbm "github.com/cosmos/cosmos-db"

"github.com/cosmos/cosmos-sdk/baseapp"
Expand All @@ -26,7 +25,6 @@ func (a *AppBuilder) DefaultGenesis() map[string]json.RawMessage {

// Build builds an *App instance.
func (a *AppBuilder) Build(
logger log.Logger,
db dbm.DB,
traceStore io.Writer,
baseAppOptions ...func(*baseapp.BaseApp),
Expand All @@ -35,7 +33,7 @@ func (a *AppBuilder) Build(
baseAppOptions = append(baseAppOptions, option)
}

bApp := baseapp.NewBaseApp(a.app.config.AppName, logger, db, nil, baseAppOptions...)
bApp := baseapp.NewBaseApp(a.app.config.AppName, a.app.logger, db, nil, baseAppOptions...)
bApp.SetMsgServiceRouter(a.app.msgServiceRouter)
bApp.SetCommitMultiStoreTracer(traceStore)
bApp.SetVersion(version.Version)
Expand Down
3 changes: 3 additions & 0 deletions runtime/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"os"

"cosmossdk.io/core/store"
"cosmossdk.io/log"
"github.com/cosmos/gogoproto/proto"
"google.golang.org/protobuf/reflect/protodesc"
"google.golang.org/protobuf/reflect/protoregistry"
Expand Down Expand Up @@ -127,6 +128,7 @@ type AppInputs struct {
BaseAppOptions []BaseAppOption
InterfaceRegistry codectypes.InterfaceRegistry
LegacyAmino *codec.LegacyAmino
Logger log.Logger
}

func SetupAppBuilder(inputs AppInputs) {
Expand All @@ -135,6 +137,7 @@ func SetupAppBuilder(inputs AppInputs) {
app.config = inputs.Config
app.ModuleManager = module.NewManagerFromMap(inputs.Modules)
app.appConfig = inputs.AppConfig
app.logger = inputs.Logger

for name, mod := range inputs.Modules {
if basicMod, ok := mod.(module.AppModuleBasic); ok {
Expand Down
1 change: 1 addition & 0 deletions simapp/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -291,6 +291,7 @@ func NewSimApp(
app.AccountKeeper,
BlockedAddresses(),
authtypes.NewModuleAddress(govtypes.ModuleName).String(),
logger,
)
app.StakingKeeper = stakingkeeper.NewKeeper(
appCodec, keys[stakingtypes.StoreKey], app.AccountKeeper, app.BankKeeper, authtypes.NewModuleAddress(govtypes.ModuleName).String(),
Expand Down
3 changes: 2 additions & 1 deletion simapp/app_v2.go
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,7 @@ func NewSimApp(
depinject.Supply(
// supply the application options
appOpts,
logger,

// ADVANCED CONFIGURATION

Expand Down Expand Up @@ -248,7 +249,7 @@ func NewSimApp(
// }
// baseAppOptions = append(baseAppOptions, prepareOpt)

app.App = appBuilder.Build(logger, db, traceStore, baseAppOptions...)
app.App = appBuilder.Build(db, traceStore, baseAppOptions...)

// register streaming services
if err := app.RegisterStreamingServices(appOpts, app.kvStoreKeys()); err != nil {
Expand Down
1 change: 1 addition & 0 deletions tests/e2e/bank/suite.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"github.com/stretchr/testify/suite"

"cosmossdk.io/math"

"github.com/cosmos/cosmos-sdk/client"
"github.com/cosmos/cosmos-sdk/testutil"

Expand Down
8 changes: 6 additions & 2 deletions tests/e2e/params/suite.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,12 @@ func (s *E2ETestSuite) SetupSuite() {
appBuilder *runtime.AppBuilder
paramsKeeper keeper.Keeper
)
if err := depinject.Inject(AppConfig, &appBuilder, &paramsKeeper); err != nil {
if err := depinject.Inject(
depinject.Configs(
AppConfig,
depinject.Supply(val.GetCtx().Logger),
),
&appBuilder, &paramsKeeper); err != nil {
panic(err)
}

Expand All @@ -66,7 +71,6 @@ func (s *E2ETestSuite) SetupSuite() {
subspace := paramsKeeper.Subspace(mySubspace).WithKeyTable(paramtypes.NewKeyTable().RegisterParamSet(&paramSet))

app := appBuilder.Build(
val.GetCtx().Logger,
dbm.NewMemDB(),
nil,
baseapp.SetPruning(pruningtypes.NewPruningOptionsFromString(val.GetAppConfig().Pruning)),
Expand Down
Loading

0 comments on commit af3122a

Please sign in to comment.