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

[Observability] Foundation for load testing telemetry #832

Merged
merged 42 commits into from
Oct 31, 2024
Merged

Conversation

okdas
Copy link
Member

@okdas okdas commented Sep 24, 2024

Summary

Refactor the foundation for E2E tokenomics observability w/ lots of new data points.

Key changes include:

  • x/tokenomics telemetry
  • Begin/End blockers execution time management
  • Custom poktroll telemetry config in app.toml

Issue

@okdas okdas added the tooling Tooling - CLI, scripts, helpers, off-chain, etc... label Sep 24, 2024
@okdas okdas self-assigned this Sep 24, 2024
@okdas okdas changed the title measure begin/end blockers [Observability] measure begin/end blockers Sep 24, 2024
@okdas okdas changed the title [Observability] measure begin/end blockers [Observability] Tweak metrics and logs Sep 26, 2024
@okdas
Copy link
Member Author

okdas commented Oct 4, 2024

Current dashboard:
Screenshot 2024-10-03 at 5 28 53 PM

Unfortunately, unable to make more requests without breaking the network.

@Olshansk
Copy link
Member

Olshansk commented Oct 4, 2024

@okdas

unable to make more requests without breaking the network.

What breaks the network?

@okdas
Copy link
Member Author

okdas commented Oct 4, 2024

What breaks the network?

Same issue - #841

return
}

// CosmosSDK has a metric called `minted_tokens` (as a part of `mint` module), however it is wrongfully marked a `gauge`.
Copy link
Member Author

Choose a reason for hiding this comment

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

Going to open a cosmos-sdk pr to address this

Copy link
Member

Choose a reason for hiding this comment

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

@okdas Send the link and #PUC once you have it.

I'll help push it through (for review et al) on their end.

@okdas okdas marked this pull request as ready for review October 22, 2024 00:56
@Olshansk
Copy link
Member

I don't think Binary Builders or Informal Systems had a need for this kind of service. The exporter enables the exposure of on-chain data (such as wallet balances, validator info, parameters, etc.) into a Prometheus/Grafana setup.

I just want to understand this last point.

Informal Systems: CometBFT maintainers
Binary Builders: CosmosSDK maintainers

Why do you think they didn't have a need for the on-chain data you listed?

@okdas
Copy link
Member Author

okdas commented Oct 24, 2024

Why do you think they didn't have a need for the on-chain data you listed?

I'm saying I don't think they need an exporter that exposes on-chain data. There are multiples provided by the community.

Copy link

gitguardian bot commented Oct 24, 2024

⚠️ GitGuardian has uncovered 2 secrets following the scan of your pull request.

Please consider investigating the findings and remediating the incidents. Failure to do so may lead to compromising the associated services or software components.

🔎 Detected hardcoded secrets in your pull request
GitGuardian id GitGuardian status Secret Commit Filename
12819930 Triggered Generic Password d966241 localnet/kubernetes/values-pocketdex-postgres.yaml View secret
12819930 Triggered Generic Password ce77cc7 localnet/kubernetes/observability-prometheus-stack.yaml View secret
🛠 Guidelines to remediate hardcoded secrets
  1. Understand the implications of revoking this secret by investigating where it is used in your code.
  2. Replace and store your secrets safely. Learn here the best practices.
  3. Revoke and rotate these secrets.
  4. If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.

To avoid such incidents in the future consider


🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.

@okdas okdas mentioned this pull request Oct 25, 2024
15 tasks
@okdas okdas requested a review from Olshansk October 30, 2024 01:29
Tiltfile Outdated Show resolved Hide resolved
x/tokenomics/module/abci.go Outdated Show resolved Hide resolved
)

var once sync.Once

// PoktrollAdditionalConfig represents a poktroll-specific part of `app.toml` file.
// See the `customAppConfigTemplate()` for additional information about each setting.
type PoktrollAdditionalConfig struct {
Copy link
Member

Choose a reason for hiding this comment

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

I looked in Cosmos & Comet and the names for these are usually RPCConfig, CustomAppConfig, serverconfig.Config, etc...

Wdyt of PoktrollAppConfig?

Additional feels weird...

Copy link
Member Author

Choose a reason for hiding this comment

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

CN(aming)O at work

cmd/poktrolld/cmd/config.go Outdated Show resolved Hide resolved
cmd/poktrolld/cmd/config.go Show resolved Hide resolved
x/proof/keeper/msg_server_create_claim.go Outdated Show resolved Hide resolved
x/proof/keeper/msg_server_create_claim.go Outdated Show resolved Hide resolved
x/proof/keeper/msg_server_submit_proof.go Outdated Show resolved Hide resolved
x/tokenomics/keeper/settle_pending_claims.go Show resolved Hide resolved
telemetry/tokens.go Show resolved Hide resolved
@Olshansk Olshansk changed the title [Observability] Tweak metrics and logs [Observability] Foundation for load testing telemetry Oct 30, 2024
@okdas okdas requested a review from Olshansk October 30, 2024 20:46
@okdas
Copy link
Member Author

okdas commented Oct 30, 2024

@Olshansk ready for another pass!

// finalizeTelemetry logs telemetry metrics for a claim based on its stage (e.g., EXPIRED, SETTLED).
// Meant to run deferred.
func (k Keeper) finalizeTelemetry(
proofStage prooftypes.ClaimProofStage,
Copy link
Member

Choose a reason for hiding this comment

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

s/proofStage/claimProofStage

// MetricNameKeys constructs the full metric name by prefixing with a defined
// prefix and appending any additional metrics provided as variadic arguments.
// MetricNameKeys prefixes metrics with `poktroll` for easy identification.
// E.g., `("hodlers", "regret_level")` yields `poktroll_hodlers_regret_level` — great for tracking FOMO as hodlers rethink choices.
Copy link
Member

Choose a reason for hiding this comment

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

lol

@okdas
Copy link
Member Author

okdas commented Oct 31, 2024

e2e tests passed locally. Merging this in.

@okdas okdas merged commit bae452a into main Oct 31, 2024
9 of 10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
devnet devnet-test-e2e push-image CI related - pushes images to ghcr.io tooling Tooling - CLI, scripts, helpers, off-chain, etc...
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

2 participants