-
Notifications
You must be signed in to change notification settings - Fork 416
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
chore: more granular metrics for routes/quotes fetched #756
Conversation
Graphite Automations"Request reviewers once CI passes on smart-order-router repo" took an action on this PR • (10/30/24)1 reviewer was added and 1 assignee was added to this PR based on 's automation. |
|
||
metric.putMetric( | ||
`MixedRoutesFetched_Chain_${this.chainId}`, | ||
_(routesWithQuotes).sum() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
number of routes with quotes should just be routesWithQuotes.length
?
.sum() | ||
); | ||
|
||
metric.putMetric(`V2RoutesFetched`, _(routesWithQuotes).sum()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so this logs the routes fetched in case we have a cache routes miss. in case we have a cached routes hit, it will be within refreshRoutesThenGetQuotes.
I'm thinking (1) whether we should log routes and/or quotes fetched in case of cached routes hit (2) whether we should log both cached routes hit and cached routes miss
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
makes sense - lets chat
good stuff, let's sync on the logging details |
discussed offline - will add logging in a different place |
What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
Adds more granular metrics for routes/quotes fetched.
This will give us more visibility on onChain effort, total routes, total quotes fetched per chain.
What is the current behavior? (You can also link to an open issue here)
Only logging number of quotes/latency at the top level
What is the new behavior (if this is a feature change)?
Logging quotes/routes/latency at top + per chain level.
Other information: