-
Notifications
You must be signed in to change notification settings - Fork 12
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
[Observability] Add claim relays counter #644
Conversation
WalkthroughThe recent updates primarily focus on standardizing string capitalizations within Grafana dashboards, refining telemetry functions to better handle error states and specific metrics (relays vs. compute units), and restructuring data management within tokenomics functionalities. These collective changes enhance data visualization consistency, improve telemetry accuracy, and streamline claim settlement processes in the blockchain system. Changes
Sequence Diagram(s)Before ChangessequenceDiagram
participant User
participant Grafana
participant Telemetry
participant Tokenomics
User->>Grafana: Request Dashboard Data
Grafana->>Telemetry: Fetch metrics (case-insensitive match)
Telemetry->>Tokenomics: Increment Counter
Tokenomics->>Telemetry: Counter Updated
Telemetry->>Grafana: Return metrics
Grafana->>User: Display Data
After ChangessequenceDiagram
participant User
participant Grafana
participant Telemetry
participant Tokenomics
User->>Grafana: Request Dashboard Data
Grafana->>Telemetry: Fetch metrics (case-sensitive match)
Telemetry->>Tokenomics: Increment Counter
Tokenomics->>Telemetry: Return with Errors if any
Telemetry->>Grafana: Return metrics and error labels
Grafana->>User: Display Data
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
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.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Commits
Files that changed from the base of the PR and between 8593170 and 93637d4f44ef17940794538aa688084e75bdb488.
Files selected for processing (8)
- localnet/grafana-dashboards/stress-test-dashboard.json (14 hunks)
- telemetry/event_counters.go (1 hunks)
- x/proof/keeper/msg_server_create_claim.go (1 hunks)
- x/proof/keeper/msg_server_submit_proof.go (1 hunks)
- x/tokenomics/keeper/keeper_settle_pending_claims_test.go (8 hunks)
- x/tokenomics/keeper/settle_pending_claims.go (10 hunks)
- x/tokenomics/module/abci.go (1 hunks)
- x/tokenomics/types/types.go (1 hunks)
Additional comments not posted (22)
x/tokenomics/types/types.go (2)
5-12
: LGTM! Struct definition is clear and appropriate.The struct
PendingClaimsResult
encapsulates the result of settling pending claims and is well-defined.
14-19
: LGTM! Function definition is clear and correct.The function
NewClaimSettlementResult
correctly initializes aPendingClaimsResult
with an empty map forRelaysPerServiceMap
.x/tokenomics/module/abci.go (1)
22-80
: LGTM! Ensure telemetry function calls are correct.The function
EndBlocker
is well-structured and the telemetry additions for tracking relays and compute units are appropriate.Verify that the telemetry function calls are correctly implemented with the right parameters.
Verification successful
Telemetry function calls are correctly implemented.
The telemetry functions
ClaimCounter
,ClaimRelaysCounter
, andClaimComputeUnitsCounter
are called with the correct parameters in theEndBlocker
function.
ClaimCounter
parameters:prooftypes.ClaimProofStage_SETTLED
,settledResult.NumClaims
,err
andprooftypes.ClaimProofStage_EXPIRED
,expiredResult.NumClaims
,err
.ClaimRelaysCounter
parameters:prooftypes.ClaimProofStage_SETTLED
,settledResult.NumRelays
,err
andprooftypes.ClaimProofStage_EXPIRED
,expiredResult.NumRelays
,err
.ClaimComputeUnitsCounter
parameters:prooftypes.ClaimProofStage_SETTLED
,settledResult.NumComputeUnits
,err
andprooftypes.ClaimProofStage_EXPIRED
,expiredResult.NumComputeUnits
,err
.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correctness of telemetry function calls in `EndBlocker`. # Test: Search for the telemetry function implementations. Expect: Correct parameters and consistent usage. rg --type go 'func ClaimCounter\(' rg --type go 'func ClaimRelaysCounter\(' rg --type go 'func ClaimComputeUnitsCounter\('Length of output: 287
Script:
#!/bin/bash # Extract the definitions of telemetry functions to verify their parameters. ast-grep --lang go --pattern 'func ClaimCounter($_, $_, $_) { $$$ }' ast-grep --lang go --pattern 'func ClaimRelaysCounter($_, $_, $_) { $$$ }' ast-grep --lang go --pattern 'func ClaimComputeUnitsCounter($_, $_, $_) { $$$ }'Length of output: 4147
x/proof/keeper/msg_server_create_claim.go (1)
35-36
: LGTM! Ensure telemetry function calls are correct.The function
CreateClaim
is well-structured and the telemetry additions for tracking relays and compute units are appropriate.Verify that the telemetry function calls are correctly implemented with the right parameters.
Verification successful
Telemetry function calls are correctly implemented.
The telemetry functions
ClaimRelaysCounter
andClaimComputeUnitsCounter
are called with the correct parameters:types.ClaimProofStage_CLAIMED
,numRelays
,err
andtypes.ClaimProofStage_CLAIMED
,numComputeUnits
,err
respectively.
ClaimRelaysCounter
parameters:claimProofStage prooftypes.ClaimProofStage
,numRelays uint64
,err error
ClaimComputeUnitsCounter
parameters:claimProofStage prooftypes.ClaimProofStage
,numComputeUnits uint64
,err error
The function calls in
msg_server_create_claim.go
match these signatures.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correctness of telemetry function calls in `CreateClaim`. # Test: Search for the telemetry function implementations. Expect: Correct parameters and consistent usage. rg --type go 'func ClaimRelaysCounter\(' rg --type go 'func ClaimComputeUnitsCounter\('Length of output: 203
Script:
#!/bin/bash # Extract function signatures for telemetry functions rg --type go 'func ClaimRelaysCounter\(' telemetry/event_counters.go rg --type go 'func ClaimComputeUnitsCounter\(' telemetry/event_counters.go # Extract calls to telemetry functions in msg_server_create_claim.go rg 'telemetry.ClaimRelaysCounter' x/proof/keeper/msg_server_create_claim.go rg 'telemetry.ClaimComputeUnitsCounter' x/proof/keeper/msg_server_create_claim.goLength of output: 531
Script:
#!/bin/bash # Extract complete function signatures for telemetry functions rg --context 5 'func ClaimRelaysCounter\(' telemetry/event_counters.go rg --context 5 'func ClaimComputeUnitsCounter\(' telemetry/event_counters.goLength of output: 1062
telemetry/event_counters.go (1)
101-127
: LGTM! Function definition is clear and appropriate.The function
ClaimRelaysCounter
correctly increments a counter for relays represented by on-chain claims and includes error handling.x/tokenomics/keeper/settle_pending_claims.go (3)
23-26
: Initialize results structs.The initialization of the new
settledResult
andexpiredResult
structures is correct and aligns with the refactoring objective.
38-40
: Initialize results structs.The initialization of the new
settledResult
andexpiredResult
structures is correct and aligns with the refactoring objective.
56-56
: Ensure consistent error handling and logging.The error handling and logging throughout the function are consistent and follow best practices. The updates to the
settledResult
andexpiredResult
structures are correctly implemented.Also applies to: 61-61, 71-71, 92-92, 101-103, 113-113, 124-124, 133-133, 149-152, 157-162, 164-164
x/tokenomics/keeper/keeper_settle_pending_claims_test.go (5)
122-122
: Update test assertions to use new result structures.The test assertions have been correctly updated to use the
settledResult
andexpiredResult
structures.Also applies to: 126-127, 145-145, 149-150
178-178
: Update test assertions to use new result structures.The test assertions have been correctly updated to use the
settledResult
andexpiredResult
structures.Also applies to: 182-184
226-226
: Update test assertions to use new result structures.The test assertions have been correctly updated to use the
settledResult
andexpiredResult
structures.Also applies to: 230-233
286-286
: Update test assertions to use new result structures.The test assertions have been correctly updated to use the
settledResult
andexpiredResult
structures.Also applies to: 290-292
339-339
: Update test assertions to use new result structures.The test assertions have been correctly updated to use the
settledResult
andexpiredResult
structures.Also applies to: 343-345
x/proof/keeper/msg_server_submit_proof.go (1)
78-79
: Add telemetry tracking for claim relays.The addition of telemetry tracking for claim relays is correct and aligns with the PR objectives.
localnet/grafana-dashboards/stress-test-dashboard.json (8)
918-918
: Change Approved: Updatedclaim_proof_stage
to uppercase.The change ensures consistency in the
claim_proof_stage
values used in Prometheus queries.
936-936
: Change Approved: Updatedclaim_proof_stage
to uppercase.The change ensures consistency in the
claim_proof_stage
values used in Prometheus queries.
1037-1037
: Change Approved: Updatedproof_required_reason
to uppercase.The change ensures consistency in the
proof_required_reason
values used in Prometheus queries.
1055-1055
: Change Approved: Updatedproof_required_reason
to uppercase.The change ensures consistency in the
proof_required_reason
values used in Prometheus queries.
1072-1072
: Change Approved: Updatedproof_required_reason
to uppercase.The change ensures consistency in the
proof_required_reason
values used in Prometheus queries.
1173-1173
: Change Approved: Updatedclaim_proof_stage
to uppercase.The change ensures consistency in the
claim_proof_stage
values used in Prometheus queries.
1191-1191
: Change Approved: Updatedclaim_proof_stage
to uppercase.The change ensures consistency in the
claim_proof_stage
values used in Prometheus queries.
1208-1208
: Change Approved: Updatedclaim_proof_stage
to uppercase.The change ensures consistency in the
claim_proof_stage
values used in Prometheus queries.Also applies to: 1225-1225, 1326-1326, 1344-1344, 1545-1545, 1563-1563, 1580-1580, 1597-1597
93637d4
to
beedf18
Compare
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.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Commits
Files that changed from the base of the PR and between 93637d4f44ef17940794538aa688084e75bdb488 and df546c6.
Files selected for processing (4)
- load-testing/tests/relays_stress_test.go (3 hunks)
- localnet/grafana-dashboards/stress-test-dashboard.json (31 hunks)
- x/tokenomics/keeper/keeper_settle_pending_claims_test.go (8 hunks)
- x/tokenomics/types/types.go (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- x/tokenomics/keeper/keeper_settle_pending_claims_test.go
- x/tokenomics/types/types.go
Additional comments not posted (10)
load-testing/tests/relays_stress_test.go (3)
412-412
: LGTM!The change from
plans
tos.plans
improves readability and consistency.
442-442
: LGTM!The change from
plans
tos.plans
improves readability and consistency.
474-474
: LGTM!The change from
plans
tos.plans
improves readability and consistency.localnet/grafana-dashboards/stress-test-dashboard.json (7)
309-309
: LGTM!The updated expression for tracking
Gateway Latency
is correct.
393-393
: LGTM!The updated expression for tracking
RelayMiner latency
is correct.
Line range hint
513-531
:
LGTM!The updated expressions for tracking
Claims & Proofs
are correct.
Line range hint
632-659
:
LGTM!The updated expressions for tracking
Claim Settlement & Expiration
are correct.
Line range hint
751-811
:
LGTM!The updated expressions for tracking
Relays Lifecycle
are correct.
Line range hint
904-964
:
LGTM!The updated expressions for tracking
Compute Units Lifecycle
are correct.
Line range hint
1057-1097
:
LGTM!The updated expressions for tracking
Proof Requirement
are correct.
// PendingClaimsResult encapsulates the result of settling pending claims. It is | ||
// intended to be used to represent settled and expired results as unique instances. | ||
type PendingClaimsResult struct { | ||
NumClaims uint64 |
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.
Thoughts on adding an iota on the result type that contained "Settled" or "Expired"?
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.
I'm not sure I follow. Can you elaborate?
} | ||
|
||
// NewClaimSettlementResult creates a new PendingClaimsResult. | ||
func NewClaimSettlementResult() PendingClaimsResult { |
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.
This would take in the iota introduced.
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.
🤔
Co-authored-by: Daniel Olshansky <[email protected]>
The image is going to be pushed after the next commit. You can use If you also want to run E2E tests, please add |
The CI will now also run the e2e tests on devnet, which increases the time it takes to complete all CI checks. You may need to run GCP workloads (requires changing the namespace to 644) |
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.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- telemetry/event_counters.go (1 hunks)
- x/proof/keeper/msg_server_create_claim.go (1 hunks)
- x/proof/keeper/msg_server_submit_proof.go (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- telemetry/event_counters.go
- x/proof/keeper/msg_server_create_claim.go
Additional comments not posted (2)
x/proof/keeper/msg_server_submit_proof.go (2)
78-78
: Ensure telemetry counter is updated correctly.The
ClaimRelaysCounter
now tracks the number of relays and includes error handling. Ensure thatnumRelays
is correctly calculated before this point.
79-79
: Ensure telemetry counter is updated correctly.The
ClaimComputeUnitsCounter
now tracks the number of compute units and includes error handling. Ensure thatnumComputeUnits
is correctly calculated before this point.
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.
LGTM
…session-manager * pokt/main: [Observables] chore: add `ReplayObservable#SubscribeFromLatestBufferedOffset()` (#647) [Observability] Add claim relays counter (#644) [Code Health] chore: log unused error when updating relay mining difficulty (#683) [Testing] chore: uncomment proof CLI query tests (#668)
…ent-balances * pokt/main: [TODOs] refactor: proof path calculation (#659) [Dependencies] bump go-getter and ibc-go (#691) [Relayminer] refactor: `relayerSessionsManager#waitForBlock()` (#648) [Observables] chore: add `ReplayObservable#SubscribeFromLatestBufferedOffset()` (#647) [Observability] Add claim relays counter (#644) [Code Health] chore: log unused error when updating relay mining difficulty (#683) [Testing] chore: uncomment proof CLI query tests (#668) build(deps): bump ws from 7.5.9 to 7.5.10 in /docusaurus (#686) build(deps): bump webpack-dev-middleware from 5.3.3 to 5.3.4 in /docusaurus (#688) build(deps): bump express from 4.18.2 to 4.19.2 in /docusaurus (#687) build(deps): bump follow-redirects from 1.15.3 to 1.15.6 in /docusaurus (#685) build(deps): bump braces from 3.0.2 to 3.0.3 in /docusaurus (#689) [CosmosSDK] Bump to v0.50.7 (#682)
…ation-overserviced * pokt/main: [TODOs] refactor: proof path calculation (#659) [Dependencies] bump go-getter and ibc-go (#691) [Relayminer] refactor: `relayerSessionsManager#waitForBlock()` (#648) [Observables] chore: add `ReplayObservable#SubscribeFromLatestBufferedOffset()` (#647) [Observability] Add claim relays counter (#644) [Code Health] chore: log unused error when updating relay mining difficulty (#683) [Testing] chore: uncomment proof CLI query tests (#668) build(deps): bump ws from 7.5.9 to 7.5.10 in /docusaurus (#686) build(deps): bump webpack-dev-middleware from 5.3.3 to 5.3.4 in /docusaurus (#688) build(deps): bump express from 4.18.2 to 4.19.2 in /docusaurus (#687) build(deps): bump follow-redirects from 1.15.3 to 1.15.6 in /docusaurus (#685) build(deps): bump braces from 3.0.2 to 3.0.3 in /docusaurus (#689) [CosmosSDK] Bump to v0.50.7 (#682)
…ation-use-index * pokt/main: [TODOs] refactor: proof path calculation (#659) [Dependencies] bump go-getter and ibc-go (#691) [Relayminer] refactor: `relayerSessionsManager#waitForBlock()` (#648) [Observables] chore: add `ReplayObservable#SubscribeFromLatestBufferedOffset()` (#647) [Observability] Add claim relays counter (#644) [Code Health] chore: log unused error when updating relay mining difficulty (#683) [Testing] chore: uncomment proof CLI query tests (#668) build(deps): bump ws from 7.5.9 to 7.5.10 in /docusaurus (#686) build(deps): bump webpack-dev-middleware from 5.3.3 to 5.3.4 in /docusaurus (#688) build(deps): bump express from 4.18.2 to 4.19.2 in /docusaurus (#687) build(deps): bump follow-redirects from 1.15.3 to 1.15.6 in /docusaurus (#685) build(deps): bump braces from 3.0.2 to 3.0.3 in /docusaurus (#689) [CosmosSDK] Bump to v0.50.7 (#682)
Co-authored-by: Daniel Olshansky <[email protected]>
Summary
Issue
Type of change
Select one or more:
Testing
Documentation changes (only if making doc changes)
make docusaurus_start
; only needed if you make doc changesLocal Testing (only if making code changes)
make go_develop_and_test
make test_e2e
PR Testing (only if making code changes)
devnet-test-e2e
label to the PR.make trigger_ci
if you want to re-trigger tests without any code changesSanity Checklist
Summary by CodeRabbit
New Features
Refactor
Bug Fixes
Tests