Skip to content
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

MTG 1289 Fix redis_total_accounts_parsed metric #398

Merged
merged 2 commits into from
Jan 31, 2025
Merged

MTG 1289 Fix redis_total_accounts_parsed metric #398

merged 2 commits into from
Jan 31, 2025

Conversation

n00m4d
Copy link
Contributor

@n00m4d n00m4d commented Jan 30, 2025

What

This PR fixes counting redis_total_accounts_parsed metric. The issue is that value is incremented by result.len() but if self.message_parser.parse_account(item.data, false) returns empty vector we don't write anything to the result vec, but message was processed by fact. Parse_account method may return empty vector if it received account with program owner which we don't support. So as a result of this issue we can see not very informative metric on Grafana - Unparsed Transactions & Accounts, which looks like ingester cannot parse all the data, but it's wrong, we see it only because values in metric were not calculated correctly. It didn't happen before because previous Solana snapshot ETL was using Geyser plugin which has accounts filter inside and it sent only accounts which Aura supports. But with PR metaplex-foundation/digital-asset-validator-plugin#88 now ETL sends all the accounts from the snapshot.

Copy link
Contributor

@armyhaylenko armyhaylenko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LBTM, i generally agree
the metric i did originally would probably better called redis_total_accounts_written and not processed.

@n00m4d n00m4d merged commit ad0074f into main Jan 31, 2025
2 checks passed
@n00m4d n00m4d deleted the feat/mtg-1289 branch January 31, 2025 15:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants