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

feat: add minimum required version metric. #85

Conversation

qedgardo
Copy link
Contributor

@qedgardo qedgardo commented Jan 3, 2025

Summary

Add new metric solana_min_required_version to track the minimum required
Solana version for foundation delegation program across different clusters.

Details

  • New metric solana_min_required_version with version and cluster labels
  • New api package to handle non-RPC API calls
  • Caching mechanism to reduce API calls (6-hour cache)
  • Automatic cluster detection using genesis hash

Technical Details

  • The metric queries https://api.solana.org/api/validators/epoch-stats to fetch the minimum version requirement
  • Cluster is automatically determined from RPC endpoint's genesis hash:
    • Mainnet: 5eykt4UsFv8P8NJdTREpY1vzqKqZKvdpKuc147dw2N9d
    • Testnet: 4uhcVJyU9pJkvQyS88uRDiswHXSCkY3zQawwpjk2NsNY
    • Devnet: EtWTRABZaYq6iMfeYKouRu166VU2xqa1wcaWoxPkrZBG
  • API responses are cached for 6 hours to minimize external calls

Testing

  • Added unit tests for API client and caching behavior
  • Added integration tests for metric collection
  • Added mock clients for testing

Copy link
Contributor

@andreclaro andreclaro left a comment

Choose a reason for hiding this comment

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

solana-exporter only needs to query the SFDP API once a day or max every 6 or 12 hours (max).

cmd/solana-exporter/desc.go Outdated Show resolved Hide resolved
cmd/solana-exporter/collector.go Outdated Show resolved Hide resolved
cmd/solana-exporter/desc.go Outdated Show resolved Hide resolved
pkg/rpc/client.go Outdated Show resolved Hide resolved
pkg/rpc/types.go Outdated Show resolved Hide resolved
pkg/rpc/client.go Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@qedgardo qedgardo marked this pull request as draft January 3, 2025 19:27
@qedgardo qedgardo force-pushed the metrics/sfdp-min-version-required branch 2 times, most recently from c4bc531 to 6e4e8cf Compare January 3, 2025 19:36
@qedgardo qedgardo marked this pull request as ready for review January 3, 2025 19:43
@qedgardo qedgardo force-pushed the metrics/sfdp-min-version-required branch 2 times, most recently from 22c866f to 8fccfe2 Compare January 3, 2025 19:55
README.md Show resolved Hide resolved
pkg/api/client.go Outdated Show resolved Hide resolved
pkg/api/client.go Outdated Show resolved Hide resolved
@qedgardo qedgardo force-pushed the metrics/sfdp-min-version-required branch from 8fccfe2 to 7157893 Compare January 6, 2025 11:46
Copy link
Contributor

@andreclaro andreclaro left a comment

Choose a reason for hiding this comment

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

Add unit tests :)

@qedgardo qedgardo force-pushed the metrics/sfdp-min-version-required branch 2 times, most recently from 1868714 to 2fb14e6 Compare January 6, 2025 12:35
@qedgardo qedgardo force-pushed the metrics/sfdp-min-version-required branch from 2fb14e6 to 0c131a9 Compare January 6, 2025 15:38
@qedgardo qedgardo force-pushed the metrics/sfdp-min-version-required branch from 0c131a9 to 4b63c01 Compare January 10, 2025 19:23
Copy link

@Alex99y Alex99y left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Contributor

@johnstonematt johnstonematt left a comment

Choose a reason for hiding this comment

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

I know it looks like I left a lot of issues, but they are generally all minor things. All round, I think this is a nice PR and I think you guys did a great job of introducing a whole new dimension to the exporter.

Please just address my (albeit generally minor) concerns and then once we are on the same page about those things I'll be happy to approve and merge.

cmd/solana-exporter/collector.go Outdated Show resolved Hide resolved
cmd/solana-exporter/collector.go Outdated Show resolved Hide resolved
pkg/rpc/client.go Outdated Show resolved Hide resolved
pkg/rpc/client.go Outdated Show resolved Hide resolved
pkg/api/types.go Outdated Show resolved Hide resolved
pkg/rpc/mock.go Outdated Show resolved Hide resolved
pkg/api/mock.go Outdated Show resolved Hide resolved
pkg/rpc/client.go Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
cmd/solana-exporter/collector.go Outdated Show resolved Hide resolved
@qedgardo qedgardo force-pushed the metrics/sfdp-min-version-required branch 3 times, most recently from 1d5f046 to 910a973 Compare January 13, 2025 20:23
Copy link
Contributor

@johnstonematt johnstonematt left a comment

Choose a reason for hiding this comment

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

Hey, thanks for addressing the requests. I believe they were at all satisfied, but there are a few more concerns here. But overall, still nice stuff and thank you very much for the contribution so far already

cmd/solana-exporter/collector.go Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
pkg/api/mock.go Outdated Show resolved Hide resolved
cmd/solana-exporter/collector_test.go Outdated Show resolved Hide resolved
cmd/solana-exporter/config.go Outdated Show resolved Hide resolved
@qedgardo qedgardo force-pushed the metrics/sfdp-min-version-required branch from 910a973 to 6454cb6 Compare January 14, 2025 11:26
@qedgardo qedgardo force-pushed the metrics/sfdp-min-version-required branch from 6454cb6 to 2c6360e Compare January 14, 2025 15:47
@qedgardo qedgardo marked this pull request as draft January 15, 2025 18:28
@qedgardo qedgardo force-pushed the metrics/sfdp-min-version-required branch from 21bd1ef to c26d17a Compare January 15, 2025 19:18
@qedgardo qedgardo marked this pull request as ready for review January 15, 2025 19:31
@johnstonematt johnstonematt merged commit de2295f into asymmetric-research:master Jan 16, 2025
2 checks passed
@qedgardo qedgardo deleted the metrics/sfdp-min-version-required branch January 16, 2025 11:45
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.

6 participants