Skip to content

Commit

Permalink
migrate more callsites to the new logging library + new unwrapSdkCont…
Browse files Browse the repository at this point in the history
…ext (#1009)

* migrate more callsites to the new logging library + new unwrapSdkContext

* context after defer from previous PR

* fix callsites for offchain update loggers

* fix log error with optional context tests and callsites

* lint
  • Loading branch information
jonfung-dydx authored Feb 5, 2024
1 parent 7ec713e commit e8c3ab1
Show file tree
Hide file tree
Showing 44 changed files with 280 additions and 272 deletions.
8 changes: 6 additions & 2 deletions protocol/app/middleware/middleware.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,12 @@ package middleware

import (
"fmt"
"github.com/cosmos/cosmos-sdk/types/module"
"os"
"runtime/debug"
"strings"

"github.com/cosmos/cosmos-sdk/types/module"

"cosmossdk.io/log"
"github.com/cosmos/cosmos-sdk/baseapp"
kitlog "github.com/go-kit/log"
Expand All @@ -28,8 +29,11 @@ func NewRunTxPanicLoggingMiddleware(moduleBasics module.BasicManager) baseapp.Re
keyvals = append(keyvals, fullModuleName, "true")
}
}
if err, isError := recoveryObj.(error); isError {
keyvals = append(keyvals, "error", err)
}

keyvals = append(keyvals, "stack trace", stack)
keyvals = append(keyvals, "stack_trace", stack)

Logger.Error(
fmt.Sprintf(
Expand Down
16 changes: 10 additions & 6 deletions protocol/app/process/process_proposal.go
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
package process

import (
"cosmossdk.io/log"
"time"

"github.com/dydxprotocol/v4-chain/protocol/lib"
error_lib "github.com/dydxprotocol/v4-chain/protocol/lib/error"
"time"
"github.com/dydxprotocol/v4-chain/protocol/lib/log"

abci "github.com/cometbft/cometbft/abci/types"
"github.com/cosmos/cosmos-sdk/client"
Expand Down Expand Up @@ -58,26 +59,29 @@ func ProcessProposalHandler(
currentConsensusRound += 1
}
ctx = ctx.WithValue(ConsensusRound, currentConsensusRound)
logger := ctx.Logger().With(log.ModuleKey, ModuleName)
ctx = log.AddPersistentTagsToLogger(
ctx,
log.Module, ModuleName,
)

// Perform the update of smoothed prices here to ensure that smoothed prices are updated even if a block is later
// rejected by consensus. We want smoothed prices to be updated on fixed cadence, and we are piggybacking on
// consensus round to do so.
if err := pricesKeeper.UpdateSmoothedPrices(ctx, lib.Uint64LinearInterpolate); err != nil {
recordErrorMetricsWithLabel(metrics.UpdateSmoothedPrices)
error_lib.LogErrorWithOptionalContext(logger, "UpdateSmoothedPrices failed", err)
error_lib.LogErrorWithOptionalContext(ctx, "UpdateSmoothedPrices failed", err)
}

txs, err := DecodeProcessProposalTxs(ctx, txConfig.TxDecoder(), req, bridgeKeeper, pricesKeeper)
if err != nil {
error_lib.LogErrorWithOptionalContext(logger, "DecodeProcessProposalTxs failed", err)
error_lib.LogErrorWithOptionalContext(ctx, "DecodeProcessProposalTxs failed", err)
recordErrorMetricsWithLabel(metrics.Decode)
return &abci.ResponseProcessProposal{Status: abci.ResponseProcessProposal_REJECT}, nil
}

err = txs.Validate()
if err != nil {
error_lib.LogErrorWithOptionalContext(logger, "DecodeProcessProposalTxs.Validate failed", err)
error_lib.LogErrorWithOptionalContext(ctx, "DecodeProcessProposalTxs.Validate failed", err)
recordErrorMetricsWithLabel(metrics.Validate)
return &abci.ResponseProcessProposal{Status: abci.ResponseProcessProposal_REJECT}, nil
}
Expand Down
3 changes: 2 additions & 1 deletion protocol/app/upgrades/v3.0.0/upgrade.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
icahosttypes "github.com/cosmos/ibc-go/v8/modules/apps/27-interchain-accounts/host/types"
icatypes "github.com/cosmos/ibc-go/v8/modules/apps/27-interchain-accounts/types"
ibctransfertypes "github.com/cosmos/ibc-go/v8/modules/apps/transfer/types"
"github.com/dydxprotocol/v4-chain/protocol/lib"
bridgemoduletypes "github.com/dydxprotocol/v4-chain/protocol/x/bridge/types"
clobmoduletypes "github.com/dydxprotocol/v4-chain/protocol/x/clob/types"
rewardsmoduletypes "github.com/dydxprotocol/v4-chain/protocol/x/rewards/types"
Expand Down Expand Up @@ -156,7 +157,7 @@ func CreateUpgradeHandler(
ak authkeeper.AccountKeeper,
) upgradetypes.UpgradeHandler {
return func(ctx context.Context, plan upgradetypes.Plan, vm module.VersionMap) (module.VersionMap, error) {
sdkCtx := sdk.UnwrapSDKContext(ctx)
sdkCtx := lib.UnwrapSDKContext(ctx, "app/upgrades")
sdkCtx.Logger().Info("Running %s Upgrade...", UpgradeName)
InitializeModuleAccs(sdkCtx, ak)

Expand Down
4 changes: 2 additions & 2 deletions protocol/app/upgrades/v4.0.0/upgrade.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ import (
"fmt"

upgradetypes "cosmossdk.io/x/upgrade/types"
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/types/module"
"github.com/dydxprotocol/v4-chain/protocol/lib"

ratelimitkeeper "github.com/dydxprotocol/v4-chain/protocol/x/ratelimit/keeper"
ratelimittypes "github.com/dydxprotocol/v4-chain/protocol/x/ratelimit/types"
Expand All @@ -18,7 +18,7 @@ func CreateUpgradeHandler(
rateLimitKeepr ratelimitkeeper.Keeper,
) upgradetypes.UpgradeHandler {
return func(ctx context.Context, plan upgradetypes.Plan, vm module.VersionMap) (module.VersionMap, error) {
sdkCtx := sdk.UnwrapSDKContext(ctx)
sdkCtx := lib.UnwrapSDKContext(ctx, "app/upgrades")
sdkCtx.Logger().Info(fmt.Sprintf("Running %s Upgrade...", UpgradeName))

if err := rateLimitKeepr.SetLimitParams(
Expand Down
133 changes: 72 additions & 61 deletions protocol/indexer/off_chain_updates/off_chain_updates.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,26 +5,22 @@ import (
"errors"
"fmt"

"cosmossdk.io/log"
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/dydxprotocol/v4-chain/protocol/indexer/common"
"github.com/dydxprotocol/v4-chain/protocol/indexer/msgsender"
v1 "github.com/dydxprotocol/v4-chain/protocol/indexer/protocol/v1"
"github.com/dydxprotocol/v4-chain/protocol/indexer/shared"
"github.com/dydxprotocol/v4-chain/protocol/lib/log"
clobtypes "github.com/dydxprotocol/v4-chain/protocol/x/clob/types"
satypes "github.com/dydxprotocol/v4-chain/protocol/x/subaccounts/types"
)

const (
hashErrMsg = "Cannot hash order id."
createErrMsg = "Cannot create message."
)

// MustCreateOrderPlaceMessage invokes CreateOrderPlaceMessage and panics if creation was unsuccessful.
func MustCreateOrderPlaceMessage(
logger log.Logger,
ctx sdk.Context,
order clobtypes.Order,
) msgsender.Message {
msg, ok := CreateOrderPlaceMessage(logger, order)
msg, ok := CreateOrderPlaceMessage(ctx, order)
if !ok {
panic(fmt.Errorf("Unable to create place order message for order %+v", order))
}
Expand All @@ -33,21 +29,30 @@ func MustCreateOrderPlaceMessage(

// CreateOrderPlaceMessage creates an off-chain update message for an order.
func CreateOrderPlaceMessage(
logger log.Logger,
ctx sdk.Context,
order clobtypes.Order,
) (message msgsender.Message, success bool) {
errMessage := "Error creating off-chain update message for placing order."
errDetails := fmt.Sprintf("Order: %+v", order)

orderIdHash, err := GetOrderIdHash(order.OrderId)
if err != nil {
logger.Error(fmt.Sprintf("%s %s Err: %+v %s\n", errMessage, hashErrMsg, err, errDetails))
log.ErrorLogWithError(
ctx,
errMessage,
err,
log.Order, order,
)
return msgsender.Message{}, false
}

update, err := newOrderPlaceMessage(order)
if err != nil {
logger.Error(fmt.Sprintf("%s %s Err: %+v %s\n", errMessage, createErrMsg, err, errDetails))
log.ErrorLogWithError(
ctx,
errMessage,
err,
log.Order, order,
)
return msgsender.Message{}, false
}

Expand All @@ -56,36 +61,46 @@ func CreateOrderPlaceMessage(

// MustCreateOrderUpdateMessage invokes CreateOrderUpdateMessage and panics if creation was unsuccessful.
func MustCreateOrderUpdateMessage(
logger log.Logger,
ctx sdk.Context,
orderId clobtypes.OrderId,
totalFilled satypes.BaseQuantums,
) msgsender.Message {
msg, ok := CreateOrderUpdateMessage(logger, orderId, totalFilled)
msg, ok := CreateOrderUpdateMessage(ctx, orderId, totalFilled)
if !ok {
panic(fmt.Errorf("Unable to create place order message for order id %+v", orderId))
}
return msg
}

// CreateOrderUpdateMessage creates an off-chain update message for an order being updated.
// TODO(CLOB-1051) take in ctx, not logger
func CreateOrderUpdateMessage(
logger log.Logger,
ctx sdk.Context,
orderId clobtypes.OrderId,
totalFilled satypes.BaseQuantums,
) (message msgsender.Message, success bool) {
errMessage := "Error creating off-chain update message for updating order."
errDetails := fmt.Sprintf("OrderId: %+v, TotalFilled %+v", orderId, totalFilled)

orderIdHash, err := GetOrderIdHash(orderId)
if err != nil {
logger.Error(fmt.Sprintf("%s %s Err: %+v %s\n", errMessage, hashErrMsg, err, errDetails))
log.ErrorLogWithError(
ctx,
errMessage,
err,
log.OrderId, orderId,
log.TotalFilled, totalFilled,
)
return msgsender.Message{}, false
}

update, err := newOrderUpdateMessage(orderId, totalFilled)
if err != nil {
logger.Error(fmt.Sprintf("%s %s Err: %+v %s\n", errMessage, createErrMsg, err, errDetails))
log.ErrorLogWithError(
ctx,
errMessage,
err,
log.OrderId, orderId,
log.TotalFilled, totalFilled,
)
return msgsender.Message{}, false
}

Expand All @@ -95,12 +110,12 @@ func CreateOrderUpdateMessage(
// MustCreateOrderRemoveMessageWithReason invokes CreateOrderRemoveMessageWithReason and panics if creation was
// unsuccessful.
func MustCreateOrderRemoveMessageWithReason(
logger log.Logger,
ctx sdk.Context,
orderId clobtypes.OrderId,
reason shared.OrderRemovalReason,
removalStatus OrderRemoveV1_OrderRemovalStatus,
) msgsender.Message {
msg, ok := CreateOrderRemoveMessageWithReason(logger, orderId, reason, removalStatus)
msg, ok := CreateOrderRemoveMessageWithReason(ctx, orderId, reason, removalStatus)
if !ok {
panic(fmt.Errorf("Unable to create remove order message with reason for order id %+v", orderId))
}
Expand All @@ -110,42 +125,51 @@ func MustCreateOrderRemoveMessageWithReason(
// CreateOrderRemoveMessageWithReason creates an off-chain update message for an order being removed
// with a specific reason for the removal and the resulting removal status of the removed order.
func CreateOrderRemoveMessageWithReason(
logger log.Logger,
ctx sdk.Context,
orderId clobtypes.OrderId,
reason shared.OrderRemovalReason,
removalStatus OrderRemoveV1_OrderRemovalStatus,
) (message msgsender.Message, success bool) {
errMessage := "Error creating off-chain update message for removing order."
errDetails := fmt.Sprintf(
"OrderId: %+v, Reason %d, Removal status %d",
orderId,
reason,
removalStatus,
)

orderIdHash, err := GetOrderIdHash(orderId)
if err != nil {
logger.Error(fmt.Sprintf("%s %s Err: %+v %s\n", errMessage, hashErrMsg, err, errDetails))
log.ErrorLogWithError(
ctx,
errMessage,
err,
log.OrderId, orderId,
log.Reason, reason,
log.RemovalStatus, removalStatus,
)
return msgsender.Message{}, false
}

update, err := newOrderRemoveMessage(orderId, reason, removalStatus)
if err != nil {
logger.Error(fmt.Sprintf("%s %s Err: %+v %s\n", errMessage, createErrMsg, err, errDetails))
log.ErrorLogWithError(
ctx,
errMessage,
err,
log.OrderId, orderId,
log.Reason, reason,
log.RemovalStatus, removalStatus,
)
return msgsender.Message{}, false
}

return msgsender.Message{Key: orderIdHash, Value: update}, true
}

// MustCreateOrderRemoveMessage invokes CreateOrderRemoveMessage and panics if creation was unsuccessful.
func MustCreateOrderRemoveMessage(logger log.Logger,
func MustCreateOrderRemoveMessage(
ctx sdk.Context,
orderId clobtypes.OrderId,
orderStatus clobtypes.OrderStatus,
orderError error,
removalStatus OrderRemoveV1_OrderRemovalStatus,
) msgsender.Message {
msg, ok := CreateOrderRemoveMessage(logger, orderId, orderStatus, orderError, removalStatus)
msg, ok := CreateOrderRemoveMessage(ctx, orderId, orderStatus, orderError, removalStatus)
if !ok {
panic(fmt.Errorf("Unable to create remove order message for order id %+v", orderId))
}
Expand All @@ -155,32 +179,25 @@ func MustCreateOrderRemoveMessage(logger log.Logger,
// CreateOrderRemoveMessage creates an off-chain update message for an order being removed, with the
// order's status and the resulting removal status of the removed order.
func CreateOrderRemoveMessage(
logger log.Logger,
ctx sdk.Context,
orderId clobtypes.OrderId,
orderStatus clobtypes.OrderStatus,
orderError error,
removalStatus OrderRemoveV1_OrderRemovalStatus,
) (message msgsender.Message, success bool) {
errDetails := fmt.Sprintf(
"OrderId: %+v, Removal status %d",
orderId,
removalStatus,
)

reason, err := shared.GetOrderRemovalReason(orderStatus, orderError)
if err != nil {
logger.Error(
fmt.Sprintf(
"Error creating off-chain update message for removing order. Invalid order removal "+
"reason. Error: %+v %s\n",
err,
errDetails,
),
log.ErrorLogWithError(
ctx,
"Error creating off-chain update message for removing order. Invalid order removal reason.",
err,
log.OrderId, orderId,
log.RemovalStatus, removalStatus,
)
return msgsender.Message{}, false
}

return CreateOrderRemoveMessageWithReason(logger, orderId, reason, removalStatus)
return CreateOrderRemoveMessageWithReason(ctx, orderId, reason, removalStatus)
}

// CreateOrderRemoveMessageWithDefaultReason creates an off-chain update message for an order being
Expand All @@ -189,7 +206,7 @@ func CreateOrderRemoveMessage(
// and falls back to the defaultRemovalReason. If defaultRemovalReason is ...UNSPECIFIED, it panics.
// TODO(CLOB-1051) take in ctx, not logger
func CreateOrderRemoveMessageWithDefaultReason(
logger log.Logger,
ctx sdk.Context,
orderId clobtypes.OrderId,
orderStatus clobtypes.OrderStatus,
orderError error,
Expand All @@ -204,26 +221,20 @@ func CreateOrderRemoveMessageWithDefaultReason(
),
)
}
errDetails := fmt.Sprintf(
"OrderId: %+v, Removal status %d",
orderId,
removalStatus,
)

reason, err := shared.GetOrderRemovalReason(orderStatus, orderError)
if err != nil {
logger.Error(
fmt.Sprintf(
"Error creating off-chain update message for removing order. Invalid order removal "+
"reason. Error: %+v %s\n",
err,
errDetails,
),
log.ErrorLogWithError(
ctx,
"Error creating off-chain update message for removing order. Invalid order removal reason.",
err,
log.OrderId, orderId,
log.RemovalStatus, removalStatus,
)
reason = defaultRemovalReason
}

return CreateOrderRemoveMessageWithReason(logger, orderId, reason, removalStatus)
return CreateOrderRemoveMessageWithReason(ctx, orderId, reason, removalStatus)
}

// newOrderPlaceMessage returns an `OffChainUpdate` struct populated with an `OrderPlace` struct
Expand Down
Loading

0 comments on commit e8c3ab1

Please sign in to comment.