Skip to content

Commit

Permalink
[SKI-41] Separate healthcheck for Slinky Client and reduce logs (back…
Browse files Browse the repository at this point in the history
…port #1534) (#1542)

Co-authored-by: Christopher-Li <Christopher-Li@users.noreply.github.com>
  • Loading branch information
mergify[bot] and Christopher-Li authored May 23, 2024
1 parent ae691d2 commit 07f18c3
Show file tree
Hide file tree
Showing 8 changed files with 52 additions and 27 deletions.
3 changes: 2 additions & 1 deletion protocol/app/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -824,7 +824,8 @@ func New(
appFlags,
logger,
)
app.RegisterDaemonWithHealthMonitor(app.SlinkyClient, maxDaemonUnhealthyDuration)
app.RegisterDaemonWithHealthMonitor(app.SlinkyClient.GetMarketPairHC(), maxDaemonUnhealthyDuration)
app.RegisterDaemonWithHealthMonitor(app.SlinkyClient.GetPriceHC(), maxDaemonUnhealthyDuration)
}
}

Expand Down
1 change: 1 addition & 0 deletions protocol/daemons/pricefeed/client/constants/logger.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ const (
ErrorLogKey = "error"
ExchangeIdLogKey = "exchangeId"
MarketIdLogKey = "marketId"
MarketIdsLogKey = "marketIds"
PriceLogKey = "Price"
ReasonLogKey = "reason"

Expand Down
34 changes: 23 additions & 11 deletions protocol/daemons/slinky/client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,11 @@ package client

import (
"context"
"cosmossdk.io/errors"
"sync"
"time"

"cosmossdk.io/errors"

"cosmossdk.io/log"

oracleclient "github.com/skip-mev/slinky/service/clients/oracle"
Expand All @@ -17,25 +18,28 @@ import (
libtime "github.com/dydxprotocol/v4-chain/protocol/lib/time"
)

// Ensure Client is HealthCheckable
var _ daemontypes.HealthCheckable = (*Client)(nil)

// Client is the daemon implementation for pulling price data from the slinky sidecar.
type Client struct {
daemontypes.HealthCheckable
ctx context.Context
cf context.CancelFunc
marketPairFetcher MarketPairFetcher
marketPairHC daemontypes.HealthCheckable
priceFetcher PriceFetcher
priceHC daemontypes.HealthCheckable
wg sync.WaitGroup
logger log.Logger
}

func newClient(ctx context.Context, logger log.Logger) *Client {
logger = logger.With(log.ModuleKey, SlinkyClientDaemonModuleName)
client := &Client{
HealthCheckable: daemontypes.NewTimeBoundedHealthCheckable(
SlinkyClientDaemonModuleName,
marketPairHC: daemontypes.NewTimeBoundedHealthCheckable(
SlinkyClientMarketPairFetcherDaemonModuleName,
&libtime.TimeProviderImpl{},
logger,
),
priceHC: daemontypes.NewTimeBoundedHealthCheckable(
SlinkyClientPriceFetcherDaemonModuleName,
&libtime.TimeProviderImpl{},
logger,
),
Expand All @@ -45,6 +49,14 @@ func newClient(ctx context.Context, logger log.Logger) *Client {
return client
}

func (c *Client) GetMarketPairHC() daemontypes.HealthCheckable {
return c.marketPairHC
}

func (c *Client) GetPriceHC() daemontypes.HealthCheckable {
return c.priceHC
}

// start creates the main goroutines of the Client.
func (c *Client) start(
slinky oracleclient.OracleClient,
Expand Down Expand Up @@ -90,9 +102,9 @@ func (c *Client) RunPriceFetcher(ctx context.Context) {
err := c.priceFetcher.FetchPrices(ctx)
if err != nil {
c.logger.Error("Failed to run fetch prices for slinky daemon", "error", err)
c.ReportFailure(errors.Wrap(err, "failed to run PriceFetcher for slinky daemon"))
c.priceHC.ReportFailure(errors.Wrap(err, "failed to run PriceFetcher for slinky daemon"))
} else {
c.ReportSuccess()
c.priceHC.ReportSuccess()
}
case <-ctx.Done():
return
Expand Down Expand Up @@ -124,9 +136,9 @@ func (c *Client) RunMarketPairFetcher(ctx context.Context, appFlags appflags.Fla
err = c.marketPairFetcher.FetchIdMappings(ctx)
if err != nil {
c.logger.Error("Failed to run fetch id mappings for slinky daemon", "error", err)
c.ReportFailure(errors.Wrap(err, "failed to run FetchIdMappings for slinky daemon"))
c.marketPairHC.ReportFailure(errors.Wrap(err, "failed to run FetchIdMappings for slinky daemon"))
} else {
c.ReportSuccess()
c.marketPairHC.ReportSuccess()
}
case <-ctx.Done():
return
Expand Down
2 changes: 1 addition & 1 deletion protocol/daemons/slinky/client/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ func TestClient(t *testing.T) {
)
waitTime := time.Second * 5
require.Eventually(t, func() bool {
return cli.HealthCheck() == nil
return cli.GetMarketPairHC().HealthCheck() == nil && cli.GetPriceHC().HealthCheck() == nil
}, waitTime, time.Millisecond*500, "Slinky daemon failed to become healthy within %s", waitTime)
cli.Stop()
}
4 changes: 3 additions & 1 deletion protocol/daemons/slinky/client/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,5 +16,7 @@ var (

const (
// SlinkyClientDaemonModuleName is the module name used in logging.
SlinkyClientDaemonModuleName = "slinky-client-daemon"
SlinkyClientDaemonModuleName = "slinky-client-daemon"
SlinkyClientPriceFetcherDaemonModuleName = "slinky-client-price-fetcher-daemon"
SlinkyClientMarketPairFetcherDaemonModuleName = "slinky-client-market-pair-fetcher-daemon"
)
5 changes: 3 additions & 2 deletions protocol/daemons/types/health_checkable.go
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
package types

import (
"cosmossdk.io/log"
"fmt"
libtime "github.com/dydxprotocol/v4-chain/protocol/lib/time"
"sync"
"time"

"cosmossdk.io/log"
libtime "github.com/dydxprotocol/v4-chain/protocol/lib/time"
)

const (
Expand Down
14 changes: 9 additions & 5 deletions protocol/x/perpetuals/keeper/perpetual.go
Original file line number Diff line number Diff line change
Expand Up @@ -523,17 +523,14 @@ func (k Keeper) sampleAllPerpetuals(ctx sdk.Context) (

marketIdToIndexPrice := k.pricesKeeper.GetMarketIdToValidIndexPrice(ctx)

invalidPerpetualIndexPrices := []uint32{}
for _, perp := range allPerpetuals {
indexPrice, exists := marketIdToIndexPrice[perp.Params.MarketId]
// Valid index price is missing
if !exists {
// Only log and increment stats if height is passed initialization period.
if ctx.BlockHeight() > pricestypes.PriceDaemonInitializationBlocks {
log.ErrorLog(
ctx,
"Perpetual does not have valid index price. Skipping premium",
constants.MarketIdLogKey, perp.Params.MarketId,
)
invalidPerpetualIndexPrices = append(invalidPerpetualIndexPrices, perp.Params.MarketId)
telemetry.IncrCounterWithLabels(
[]string{
types.ModuleName,
Expand Down Expand Up @@ -601,6 +598,13 @@ func (k Keeper) sampleAllPerpetuals(ctx sdk.Context) (
),
)
}
if len(invalidPerpetualIndexPrices) > 0 {
log.ErrorLog(
ctx,
"Perpetuals do not have valid index price. Skipping premium",
constants.MarketIdsLogKey, invalidPerpetualIndexPrices,
)
}
return samples, nil
}

Expand Down
16 changes: 10 additions & 6 deletions protocol/x/prices/keeper/update_price.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ func (k Keeper) GetValidMarketPriceUpdates(

// 3. Collect all "valid" price updates.
updates := make([]*types.MsgUpdateMarketPrices_MarketPrice, 0, len(allMarketParamPrices))
nonExistentMarkets := []uint32{}
for _, marketParamPrice := range allMarketParamPrices {
marketId := marketParamPrice.Param.Id
indexPrice, indexPriceExists := allIndexPrices[marketId]
Expand All @@ -73,15 +74,18 @@ func (k Keeper) GetValidMarketPriceUpdates(
// there will be a delay in populating index prices after network genesis or a network restart, or when a
// market is created, it takes the daemon some time to warm up.
if !k.IsRecentlyAvailable(ctx, marketId) {
log.ErrorLog(
ctx,
"Index price for market does not exist",
constants.MarketIdLogKey,
marketId,
)
nonExistentMarkets = append(nonExistentMarkets, marketId)
}
continue
}
if len(nonExistentMarkets) > 0 {
log.ErrorLog(
ctx,
"Index price for markets does not exist",
constants.MarketIdsLogKey,
nonExistentMarkets,
)
}

// Index prices of 0 are unexpected. In this scenario, we skip the proposal logic for the market and report an
// error.
Expand Down

0 comments on commit 07f18c3

Please sign in to comment.