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

feat!: Provide logger through depinject #15818

Merged
merged 41 commits into from
Apr 24, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
41 commits
Select commit Hold shift + click to select a range
99ef23e
init
facundomedica Apr 12, 2023
07ad3de
implement + adr
facundomedica Apr 12, 2023
4fd390a
Merge branch 'main' of https://github.com/cosmos/cosmos-sdk into facu…
facundomedica Apr 12, 2023
f709e1a
update
facundomedica Apr 12, 2023
55dfec4
use cosmossdk.io\/log
facundomedica Apr 12, 2023
1b72a66
progress
facundomedica Apr 12, 2023
e64afbf
Merge branch 'main' of https://github.com/cosmos/cosmos-sdk into facu…
facundomedica Apr 12, 2023
c9cafd3
fix
facundomedica Apr 12, 2023
0262fcd
small fixes
facundomedica Apr 12, 2023
65b6a86
small fixes
facundomedica Apr 12, 2023
0998943
small fixes
facundomedica Apr 12, 2023
5e1ca87
small fixes
facundomedica Apr 12, 2023
f7e41da
small fixes
facundomedica Apr 12, 2023
db7b086
cl++
facundomedica Apr 13, 2023
e3b7073
fix app v1
facundomedica Apr 13, 2023
e58ea40
fix more tests
facundomedica Apr 13, 2023
1f9c0bf
more tests
facundomedica Apr 13, 2023
db2be7f
merge
facundomedica Apr 17, 2023
c7b25a3
tests
facundomedica Apr 18, 2023
c277e3a
update bank
facundomedica Apr 18, 2023
54bb8f6
update bank
facundomedica Apr 18, 2023
39c6030
Merge branch 'main' of https://github.com/cosmos/cosmos-sdk into facu…
facundomedica Apr 18, 2023
87bc493
fix more test
facundomedica Apr 18, 2023
9c4c448
fix tests
facundomedica Apr 18, 2023
63bcdad
Merge branch 'main' into facu/coreapi-logger
facundomedica Apr 18, 2023
601e89f
fix
facundomedica Apr 18, 2023
7024218
Merge branch 'facu/coreapi-logger' of https://github.com/cosmos/cosmo…
facundomedica Apr 18, 2023
7312bda
fix
facundomedica Apr 18, 2023
607401a
fix conflicts
facundomedica Apr 19, 2023
cf98c07
fix some more tests
facundomedica Apr 19, 2023
046c918
im tired
facundomedica Apr 19, 2023
089e775
hopefully the last
facundomedica Apr 19, 2023
ef11a15
Merge branch 'main' into facu/coreapi-logger
facundomedica Apr 19, 2023
6261eeb
Merge branch 'main' into facu/coreapi-logger
facundomedica Apr 19, 2023
8c2cf2b
lint
facundomedica Apr 20, 2023
5b90bf4
Merge branch 'main' into facu/coreapi-logger
facundomedica Apr 20, 2023
f14a89d
Update x/bank/keeper/keeper.go
facundomedica Apr 20, 2023
c038a61
adr++ and upgrading++
facundomedica Apr 20, 2023
3a856a5
cl++
facundomedica Apr 20, 2023
f462163
Merge branch 'main' into facu/coreapi-logger
facundomedica Apr 20, 2023
a4c1aca
Merge branch 'main' into facu/coreapi-logger
facundomedica Apr 21, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -109,6 +110,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 @@ -82,6 +82,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,
facundomedica marked this conversation as resolved.
Show resolved Hide resolved

// 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