From 45d85123d35d5c933126a15ced60897d4621a8ec Mon Sep 17 00:00:00 2001 From: jsy1218 <91580504+jsy1218@users.noreply.github.com> Date: Fri, 22 Sep 2023 13:28:48 -0700 Subject: [PATCH] logging more around fot tax (#406) * logging more around fot tax * log token fee result existence dedicatedly * use the validationResult from resultWrapper * distinguish the token fee result metrics from cache hit vs cache miss * use explicit cache hit vs cache miss result metrics for token validator --- src/providers/token-fee-fetcher.ts | 8 ++++++- src/providers/token-properties-provider.ts | 26 ++++++++++++++++++++-- src/providers/token-validator-provider.ts | 12 ++++++++-- 3 files changed, 41 insertions(+), 5 deletions(-) diff --git a/src/providers/token-fee-fetcher.ts b/src/providers/token-fee-fetcher.ts index 029b279d3..7f652482c 100644 --- a/src/providers/token-fee-fetcher.ts +++ b/src/providers/token-fee-fetcher.ts @@ -4,7 +4,7 @@ import { ChainId } from '@uniswap/sdk-core'; import { TokenFeeDetector__factory } from '../types/other/factories/TokenFeeDetector__factory'; import { TokenFeeDetector } from '../types/other/TokenFeeDetector'; -import { log, WRAPPED_NATIVE_CURRENCY } from '../util'; +import { log, metric, MetricLoggerUnit, WRAPPED_NATIVE_CURRENCY } from '../util'; import { ProviderConfig } from './provider'; @@ -93,12 +93,18 @@ export class OnChainTokenFeeFetcher implements ITokenFeeFetcher { blockTag: providerConfig?.blockNumber, } ); + + metric.putMetric("TokenFeeFetcherFetchFeesSuccess", 1, MetricLoggerUnit.Count) + return { address, ...feeResult }; } catch (err) { log.error( { err }, `Error calling validate on-chain for token ${address}` ); + + metric.putMetric("TokenFeeFetcherFetchFeesFailure", 1, MetricLoggerUnit.Count) + // in case of FOT token fee fetch failure, we return null // so that they won't get returned from the token-fee-fetcher // and thus no fee will be applied, and the cache won't cache on FOT tokens with failed fee fetching diff --git a/src/providers/token-properties-provider.ts b/src/providers/token-properties-provider.ts index 1b8027355..87aa183e0 100644 --- a/src/providers/token-properties-provider.ts +++ b/src/providers/token-properties-provider.ts @@ -1,6 +1,6 @@ import { ChainId, Token } from '@uniswap/sdk-core'; -import { log } from '../util'; +import { log, metric, MetricLoggerUnit } from '../util'; import { ICache } from './cache'; import { ProviderConfig } from './provider'; import { @@ -14,6 +14,7 @@ import { ITokenValidatorProvider, TokenValidationResult, } from './token-validator-provider'; +import { BigNumber } from '@ethersproject/bignumber'; export const DEFAULT_TOKEN_PROPERTIES_RESULT: TokenPropertiesResult = { tokenFeeResult: DEFAULT_TOKEN_FEE_RESULT, @@ -89,6 +90,16 @@ export class TokenPropertiesProvider implements ITokenPropertiesProvider { for (const address of addressesRaw) { const cachedValue = tokenProperties[address]; if (cachedValue) { + metric.putMetric("TokenPropertiesProviderBatchGetCacheHit", 1, MetricLoggerUnit.Count) + const tokenFee = cachedValue.tokenFeeResult; + const tokenFeeResultExists: BigNumber | undefined = tokenFee && (tokenFee.buyFeeBps || tokenFee.sellFeeBps) + + if (tokenFeeResultExists) { + metric.putMetric(`TokenPropertiesProviderCacheHitTokenFeeResultExists${tokenFeeResultExists}`, 1, MetricLoggerUnit.Count) + } else { + metric.putMetric(`TokenPropertiesProviderCacheHitTokenFeeResultNotExists`, 1, MetricLoggerUnit.Count) + } + tokenToResult[address] = cachedValue; } else if ( tokenToResult[address]?.tokenValidationResult === @@ -116,13 +127,23 @@ export class TokenPropertiesProvider implements ITokenPropertiesProvider { await Promise.all( addressesToFetchFeesOnchain.map((address) => { const tokenFee = tokenFeeMap[address]; - if (tokenFee && (tokenFee.buyFeeBps || tokenFee.sellFeeBps)) { + const tokenFeeResultExists: BigNumber | undefined = tokenFee && (tokenFee.buyFeeBps || tokenFee.sellFeeBps) + + if (tokenFeeResultExists) { + // we will leverage the metric to log the token fee result, if it exists + // the idea is that the token fee should not differ by too much across tokens, + // so that we can accurately log the token fee for a particular quote request (without breaching metrics dimensionality limit) + // in the form of metrics. + // if we log as logging, given prod traffic volume, the logging volume will be high. + metric.putMetric(`TokenPropertiesProviderTokenFeeResultCacheMissExists${tokenFeeResultExists}`, 1, MetricLoggerUnit.Count) const tokenResultForAddress = tokenToResult[address]; if (tokenResultForAddress) { tokenResultForAddress.tokenFeeResult = tokenFee; } + metric.putMetric("TokenPropertiesProviderBatchGetCacheMiss", 1, MetricLoggerUnit.Count) + // update cache concurrently // at this point, we are confident that the tokens are FOT, so we can hardcode the validation result return this.tokenPropertiesCache.set( @@ -133,6 +154,7 @@ export class TokenPropertiesProvider implements ITokenPropertiesProvider { } ); } else { + metric.putMetric(`TokenPropertiesProviderTokenFeeResultCacheMissNotExists`, 1, MetricLoggerUnit.Count) return Promise.resolve(true); } }) diff --git a/src/providers/token-validator-provider.ts b/src/providers/token-validator-provider.ts index 446bd7171..66e85791c 100644 --- a/src/providers/token-validator-provider.ts +++ b/src/providers/token-validator-provider.ts @@ -2,7 +2,7 @@ import { ChainId, Token } from '@uniswap/sdk-core'; import _ from 'lodash'; import { ITokenValidator__factory } from '../types/other/factories/ITokenValidator__factory'; -import { log, WRAPPED_NATIVE_CURRENCY } from '../util'; +import { log, metric, MetricLoggerUnit, WRAPPED_NATIVE_CURRENCY } from '../util'; import { ICache } from './cache'; import { IMulticallProvider } from './multicall-provider'; @@ -89,6 +89,8 @@ export class TokenValidatorProvider implements ITokenValidatorProvider { (await this.tokenValidationCache.get( this.CACHE_KEY(this.chainId, address) ))!; + + metric.putMetric(`TokenValidatorProviderValidateCacheHitResult${tokenToResult[address.toLowerCase()]}`, 1, MetricLoggerUnit.Count) } else { addresses.push(address); } @@ -140,7 +142,9 @@ export class TokenValidatorProvider implements ITokenValidatorProvider { // Could happen if the tokens transfer consumes too much gas so we revert. Just // drop the token in that case. if (!resultWrapper.success) { - log.info( + metric.putMetric("TokenValidatorProviderValidateFailed", 1, MetricLoggerUnit.Count) + + log.error( { result: resultWrapper }, `Failed to validate token ${token.symbol}` ); @@ -148,6 +152,8 @@ export class TokenValidatorProvider implements ITokenValidatorProvider { continue; } + metric.putMetric("TokenValidatorProviderValidateSuccess", 1, MetricLoggerUnit.Count) + const validationResult = resultWrapper.result[0]!; tokenToResult[token.address.toLowerCase()] = @@ -157,6 +163,8 @@ export class TokenValidatorProvider implements ITokenValidatorProvider { this.CACHE_KEY(this.chainId, token.address.toLowerCase()), tokenToResult[token.address.toLowerCase()]! ); + + metric.putMetric(`TokenValidatorProviderValidateCacheMissResult${validationResult}`, 1, MetricLoggerUnit.Count) } return {