-
Notifications
You must be signed in to change notification settings - Fork 1
Add metrics allowed regions #45
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
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThe changes introduce environment-based gating for metric collection across several blockchain-related API modules. Each affected file now imports Changes
Sequence Diagram(s)sequenceDiagram
participant Environment
participant Module
participant Handler
Environment->>Module: Set VERCEL_REGION
Module->>Module: Check if VERCEL_REGION in ALLOWED_REGIONS
alt Region allowed
Module->>Handler: Assign METRICS = full metric list
else Region not allowed
Module->>Handler: Assign METRICS = []
end
Handler->>Client: Serve metrics (if available)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15–20 minutes Possibly related PRs
Poem
Note 🔌 MCP (Model Context Protocol) integration is now available in Early Access!Pro users can now connect to remote MCP servers under the Integrations page to get reviews and chat conversations that understand additional development context. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration 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: 1
♻️ Duplicate comments (3)
api/read/test_blockchain.py (1)
13-18: Same duplication as noted insolana.py– see earlier comment about centralisingALLOWED_REGIONS.api/read/ton.py (1)
29-30:VERCEL_REGIONcase-insensitive check – same note as previous files.api/read/ethereum.py (1)
17-23: Duplicate constant & potential driftSee earlier advice on centralising
ALLOWED_REGIONS; keeping one source avoids mismatched lists across chains.
🧹 Nitpick comments (12)
api/read/solana.py (2)
15-20: CentraliseALLOWED_REGIONSto avoid driftEach blockchain module now hard-codes the same region list. Moving this constant to a shared place (e.g.
config.defaults) or a helper likeget_allowed_regions()keeps the list in sync and eliminates five-way duplication.
22-33: Guard for case / missing env & make lookup O(1)
os.getenv("VERCEL_REGION")may returnNoneor an upper-case code ("FRA1").
Small tweaks improve robustness and micro-perf:-ALLOWED_REGIONS: list[str] = [...] +ALLOWED_REGIONS: frozenset[str] = {...} -region = os.getenv("VERCEL_REGION") -if region in ALLOWED_REGIONS: +region = (os.getenv("VERCEL_REGION") or "").lower() +if region in ALLOWED_REGIONS:api/read/test_blockchain.py (1)
27-28: Normalise env var caseAs above, consider
.lower()on the fetched region to avoid silent metric drop-outs in mixed-case deployments.api/read/ton.py (1)
15-20: Consider makingALLOWED_REGIONSafrozensetImmutability conveys intent (config, not data) and speeds membership checks.
api/read/ethereum.py (1)
35-36: Minor: evaluate at runtime, not import-timeIf you ever hot-reload or change env vars between requests, compute the
regioninside the handler constructor instead of at module import so the decision reflects current env.api/read/bnbsc.py (3)
1-1: Prefer a module docstring over per-line noqa for D100Add a minimal module docstring and drop the inline
# noqa: D100to satisfy pydocstyle cleanly.Apply:
-import os # noqa: D100 +"""BNB Smart Chain metrics read handler.""" +import os
18-24: ALLOWED_REGIONS consistency and centralizationThis per-file allowlist can drift across modules. Consider centralizing allowed regions (per-chain or global) in config (e.g., config.defaults) or an env var, and document the rationale for each region.
18-24: Use an immutable set for membership checksTiny nit: use a frozenset for clarity and O(1) membership.
-ALLOWED_REGIONS: list[str] = [ +ALLOWED_REGIONS: frozenset[str] = frozenset({ "fra1", # Frankfurt (EU) "sfo1", # San Francisco (US West) "sin1", # Singapore # "kix1", # Osaka (JP) -] +})api/read/arbitrum.py (2)
1-1: Prefer a module docstring over per-line noqa for D100Same nit as other modules: add a brief module docstring and remove inline
# noqa: D100.-import os # noqa: D100 +"""Arbitrum metrics read handler.""" +import os
18-24: Allowed regions differ from bnbsc (sin1 commented here, kix1 enabled there)If this divergence is intentional (provider availability, latency, cost), please add a short comment linking to the decision source, or centralize per-chain allowlists in config to avoid drift.
api/write/solana.py (2)
1-1: Prefer a module docstring over per-line noqa for D100Add a minimal docstring and remove the inline noqa.
-import os # noqa: D100 +"""Solana write metrics handler.""" +import os
7-8: Type/structure ALLOWED_REGIONS consistentlyFor consistency with other modules and to avoid accidental mutation, consider a typed, immutable allowlist.
-METRIC_NAME = f"{MetricsServiceConfig.METRIC_PREFIX}transaction_landing_latency" -ALLOWED_REGIONS = ["fra1"] +METRIC_NAME = f"{MetricsServiceConfig.METRIC_PREFIX}transaction_landing_latency" +ALLOWED_REGIONS: tuple[str, ...] = ("fra1",)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
api/read/arbitrum.py(2 hunks)api/read/base.py(2 hunks)api/read/bnbsc.py(2 hunks)api/read/ethereum.py(2 hunks)api/read/solana.py(2 hunks)api/read/test_blockchain.py(2 hunks)api/read/ton.py(2 hunks)api/write/solana.py(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (4)
api/read/test_blockchain.py (2)
config/defaults.py (1)
MetricsServiceConfig(6-42)metrics/solana.py (4)
HTTPGetRecentBlockhashLatencyMetric(23-33)HTTPSimulateTxLatencyMetric(6-20)HTTPGetBalanceLatencyMetric(57-67)HTTPGetProgramAccsLatencyMetric(96-109)
api/read/bnbsc.py (3)
config/defaults.py (1)
MetricsServiceConfig(6-42)metrics/ethereum.py (8)
WSBlockLatencyMetric(145-310)HTTPBlockNumberLatencyMetric(33-43)HTTPEthCallLatencyMetric(14-30)HTTPAccBalanceLatencyMetric(64-79)HTTPDebugTraceBlockByNumberLatencyMetric(82-92)HTTPDebugTraceTxLatencyMetric(95-110)HTTPTxReceiptLatencyMetric(46-61)HTTPGetLogsLatencyMetric(113-142)metrics/bnbsc.py (7)
HTTPBlockNumberLatencyMetric(92-102)HTTPEthCallLatencyMetric(6-22)HTTPAccBalanceLatencyMetric(43-58)HTTPDebugTraceBlockByNumberLatencyMetric(79-89)HTTPDebugTraceTxLatencyMetric(61-76)HTTPTxReceiptLatencyMetric(25-40)HTTPGetLogsLatencyMetric(105-134)
api/read/arbitrum.py (3)
config/defaults.py (1)
MetricsServiceConfig(6-42)metrics/ethereum.py (8)
WSBlockLatencyMetric(145-310)HTTPBlockNumberLatencyMetric(33-43)HTTPEthCallLatencyMetric(14-30)HTTPAccBalanceLatencyMetric(64-79)HTTPDebugTraceBlockByNumberLatencyMetric(82-92)HTTPDebugTraceTxLatencyMetric(95-110)HTTPTxReceiptLatencyMetric(46-61)HTTPGetLogsLatencyMetric(113-142)metrics/arbitrum.py (7)
HTTPBlockNumberLatencyMetric(92-102)HTTPEthCallLatencyMetric(6-22)HTTPAccBalanceLatencyMetric(43-58)HTTPDebugTraceBlockByNumberLatencyMetric(79-89)HTTPDebugTraceTxLatencyMetric(61-76)HTTPTxReceiptLatencyMetric(25-40)HTTPGetLogsLatencyMetric(105-134)
api/write/solana.py (2)
config/defaults.py (1)
MetricsServiceConfig(6-42)metrics/solana_landing_rate.py (1)
SolanaLandingMetric(49-186)
🔇 Additional comments (5)
api/read/base.py (1)
18-24: Inconsistent region set – deliberate?Other modules include
"sin1"but exclude"kix1"; this file does the opposite. Confirm this asymmetry is intentional, otherwise some regions will emit Base-chain metrics while others do not.api/read/bnbsc.py (2)
26-39: Region-gated metrics: LGTMConditional gating on VERCEL_REGION is clear and aligns with the PR goal.
26-39: No changes needed: MetricsHandler gracefully handles empty METRICSVerified in
common/metrics_handler.pythat:
handle()resetsself._instances = []whenMETRICSis emptyget_metrics_text()returns an empty string if there are no metric instanceshandle()then logs a warning (“Nothing to push to Grafana.”) and returns("done", "")without raising exceptionsAll good—no action required.
api/read/arbitrum.py (1)
26-39: Region-gated metrics: LGTMPattern matches other read handlers; conditional list keeps the handler wiring simple.
api/write/solana.py (1)
10-14: Region-gated metric: LGTMGating the SolanaLandingMetric by VERCEL_REGION aligns with the read-side pattern.
Summary by CodeRabbit