[CORE-711] Always log market id when pair information is missing. #694
Conversation
WalkthroughThe changes primarily focus on improving the error handling and readability of the code. The Changes
TipsChat with CodeRabbit Bot (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (3)
- protocol/daemons/pricefeed/metrics/market_pairs.go (3 hunks)
- protocol/daemons/pricefeed/metrics/market_pairs_test.go (1 hunks)
- protocol/daemons/pricefeed/metrics/metrics_test.go (2 hunks)
Files skipped from review due to trivial changes (2)
- protocol/daemons/pricefeed/metrics/market_pairs_test.go
- protocol/daemons/pricefeed/metrics/metrics_test.go
Additional comments: 3
protocol/daemons/pricefeed/metrics/market_pairs.go (3)
4-4: The addition of the "fmt" package is necessary for the string formatting operation introduced in the
GetMarketPairForTelemetryfunction. This change is appropriate and does not introduce any issues.23-23: The comment has been updated to fix a minor spelling mistake. This change is appropriate and does not introduce any issues.
43-43: The return value of the
GetMarketPairForTelemetryfunction has been updated to return a formatted string that includes the market id when the market id does not exist in themarketToPairmap. This change improves the logging of market ids when pair information is not available. However, ensure that all calls to this function throughout the codebase can handle the new return value format.
| @@ -39,7 +40,7 @@ func GetMarketPairForTelemetry(marketId types.MarketId) string { | |||
|
|
|||
| marketPair, exists := marketToPair[marketId] | |||
| if !exists { | |||
| return INVALID | |||
| return fmt.Sprintf("id:%v", marketId) | |||
There was a problem hiding this comment.
| return fmt.Sprintf("id:%v", marketId) | |
| return fmt.Sprintf("invalid id:%v", marketId) |
There was a problem hiding this comment.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files selected for processing (3)
- protocol/daemons/pricefeed/metrics/market_pairs.go (3 hunks)
- protocol/daemons/pricefeed/metrics/market_pairs_test.go (1 hunks)
- protocol/daemons/pricefeed/metrics/metrics_test.go (2 hunks)
Files skipped from review due to trivial changes (1)
- protocol/daemons/pricefeed/metrics/market_pairs_test.go
Additional comments: 3
protocol/daemons/pricefeed/metrics/metrics_test.go (2)
4-4: The addition of the "fmt" package is appropriate for the new string formatting requirement in the test function
TestGetLabelForMarketIdFailure.31-31: The change in the expected value from
pricefeedmetrics.INVALIDtofmt.Sprintf("invalid_id:%d", INVALID_ID)is correct as it aligns with the updated behavior of theGetMarketPairForTelemetryfunction. Ensure that theINVALID_IDconstant is defined and accessible in this context.protocol/daemons/pricefeed/metrics/market_pairs.go (1)
- 4-4: The addition of the "fmt" package is appropriate for the new string formatting requirement in the
GetMarketPairForTelemetryfunction.
| @@ -39,7 +40,7 @@ func GetMarketPairForTelemetry(marketId types.MarketId) string { | |||
|
|
|||
| marketPair, exists := marketToPair[marketId] | |||
| if !exists { | |||
| return INVALID | |||
| return fmt.Sprintf("invalid_id:%v", marketId) | |||
There was a problem hiding this comment.
The change from returning a constant "INVALID" to a formatted string with the market id provides more information when the market pair is not found. This is a good improvement for logging and debugging purposes.
- return INVALID
+ return fmt.Sprintf("invalid_id:%v", marketId)Committable suggestion (Beta)
| return fmt.Sprintf("invalid_id:%v", marketId) | |
| marketPair, exists := marketToPair[marketId] | |
| if !exists { | |
| return fmt.Sprintf("invalid_id:%v", marketId) | |
| } | |
| return marketPair |
(cherry picked from commit eb7c6ff)
…) (#700) (cherry picked from commit eb7c6ff) Co-authored-by: Crystal Lemire <[email protected]>
Changelist
When getting market id labels, always return the printed market id if the pair string is not available.
Test Plan
Updated existing tests.
Author/Reviewer Checklist
state-breakinglabel.PrepareProposalorProcessProposal, manually add the labelproposal-breaking.feature:[feature-name].backport/[branch-name].refactor,chore,bug.