Conversation
🦋 Changeset detectedLatest commit: 19cd0b4 The changes in this PR will be included in the next version bump. This PR includes changesets to release 19 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub. 3 Skipped Deployments
|
|
@greptile review |
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughAdds PonderClient.health() and PonderClient.metrics(), implements Prometheus exposition-text parsing and Ponder indexing metrics deserialization with Zod validation, introduces new types and mocks/tests, renames several exported schema identifiers, and bundles the Prometheus parser dependency. Changes
Sequence DiagramsequenceDiagram
participant Client as PonderClient
participant Server as /metrics Endpoint
participant Parser as PrometheusMetrics
participant Validator as Zod Validator
participant Output as PonderIndexingMetrics
Client->>Server: GET /metrics
Server-->>Client: Prometheus exposition text
Client->>Parser: deserializePrometheusMetrics(text)
Parser-->>Client: PrometheusMetrics instance
Client->>Validator: deserializePonderIndexingMetrics(PrometheusMetrics)
Validator-->>Output: validated PonderIndexingMetrics
Output-->>Client: Promise<PonderIndexingMetrics>
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
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. Comment |
Greptile OverviewGreptile SummaryThis PR extends the Key Changes:
Architecture: Confidence Score: 5/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant Client as Consumer
participant PC as PonderClient
participant API as Ponder API
participant Parser as PrometheusMetrics
participant Deserializer as deserializePonderIndexingMetrics
participant Validator as Zod Schema
Note over Client,Validator: health() method flow
Client->>PC: health()
PC->>API: GET /health
alt response.ok
API-->>PC: 200 OK
PC-->>Client: void (success)
else !response.ok
API-->>PC: 503 Service Unavailable
PC-->>Client: throw Error
end
Note over Client,Validator: metrics() method flow
Client->>PC: metrics()
PC->>API: GET /metrics
alt response.ok
API-->>PC: 200 OK (Prometheus text format)
PC->>Deserializer: deserializePonderIndexingMetrics(responseText)
Deserializer->>Parser: PrometheusMetrics.parse(text)
Parser-->>Deserializer: PrometheusMetrics instance
Deserializer->>Validator: Validate required metrics exist
alt all required metrics present
Validator->>Deserializer: Build unvalidated metrics object
Deserializer->>Validator: Validate chains Map, appSettings
Validator->>Validator: Check invariants (1+ chains, no conflicting states)
alt validation passes
Validator-->>Deserializer: PonderIndexingMetrics
Deserializer-->>PC: PonderIndexingMetrics
PC-->>Client: PonderIndexingMetrics
else validation fails
Validator-->>Deserializer: ZodError
Deserializer-->>PC: throw Error (prettified)
PC-->>Client: throw Error
end
else missing required metrics
Validator-->>Deserializer: ZodError
Deserializer-->>PC: throw Error
PC-->>Client: throw Error
end
else !response.ok
API-->>PC: 500 Internal Server Error
PC-->>Client: throw Error
end
|
There was a problem hiding this comment.
Pull request overview
This PR extends the ponder-sdk package with new functionality to fetch and parse Ponder indexing metrics from a Ponder application's Prometheus /metrics endpoint.
Changes:
- Added
health()andmetrics()methods toPonderClientfor health checks and retrieving indexing metrics - Introduced
PonderIndexingMetricsdata model with application settings and per-chain indexing metrics - Implemented Prometheus text format parser to deserialize metrics into a structured format
Reviewed changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| pnpm-lock.yaml | Added parse-prometheus-text-format@1.1.1 dependency |
| packages/ponder-sdk/package.json | Added parse-prometheus-text-format to dependencies |
| packages/ponder-sdk/tsup.config.ts | Configured bundling to include parse-prometheus-text-format |
| packages/ponder-sdk/src/indexing-metrics.ts | Defined TypeScript interfaces for Ponder indexing metrics data model |
| packages/ponder-sdk/src/deserialize/prometheus-metrics-text.types.ts | Type definitions for external Prometheus parsing library |
| packages/ponder-sdk/src/deserialize/prometheus-metrics-text.ts | Prometheus metrics parser with query methods for extracting metric values and labels |
| packages/ponder-sdk/src/deserialize/prometheus-metrics.test.ts | Unit tests for Prometheus metrics parser |
| packages/ponder-sdk/src/deserialize/indexing-metrics.ts | Deserialization and validation logic for Ponder indexing metrics with Zod schemas |
| packages/ponder-sdk/src/deserialize/indexing-metrics.mock.ts | Mock data for testing valid and invalid indexing metrics responses |
| packages/ponder-sdk/src/deserialize/chains.ts | Chain ID deserialization helper |
| packages/ponder-sdk/src/client.ts | Added health() and metrics() methods to PonderClient |
| packages/ponder-sdk/src/client.test.ts | Tests for new client methods including validation and error handling |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@packages/ponder-sdk/src/deserialize/indexing-metrics.ts`:
- Around line 170-173: Update the function signature of
deserializePonderIndexingMetrics to accept a single unknown parameter (replace
`string | unknown` with `unknown`) and keep the existing runtime type check `if
(typeof data !== "string") { ... }` and error message intact; this simplifies
the type annotation while preserving the explicit validation inside the
function.
In `@packages/ponder-sdk/src/deserialize/prometheus-metrics-text.types.ts`:
- Around line 1-16: The current PrometheusMetric declaration is too narrow (only
{ value, labels? }) and misses Histogram and Summary shapes; introduce a
MetricEntry discriminated union and tighten PrometheusMetric.type to the literal
union ("GAUGE" | "COUNTER" | "HISTOGRAM" | "SUMMARY" | "UNTYPED"), then replace
PrometheusMetric.metrics: Array<{...}> with metrics: MetricEntry[] so histogram
entries ({ buckets, count, sum, labels? }) and summary entries ({ quantiles,
count, sum, labels? }) are represented alongside value-style entries; keep the
exported parsePrometheusTextFormat signature as-is but update the types
accordingly.
tk-o
left a comment
There was a problem hiding this comment.
Self-review completed.
| @@ -0,0 +1,204 @@ | |||
| /** | |||
There was a problem hiding this comment.
This file has been moved from packages/ponder-metadata/src/prometheus-metrics.ts.
| @@ -0,0 +1,82 @@ | |||
| import { describe, expect, it } from "vitest"; | |||
There was a problem hiding this comment.
This file has been moved from packages/ponder-metadata/src/prometheus-metrics.test.ts.
| @@ -0,0 +1,16 @@ | |||
| /** | |||
There was a problem hiding this comment.
This file has been moved from packages/ponder-metadata/src/types/parse-prometheus-text-format.ts.
| * @returns Unvalidated (possibly incomplete) Ponder Indexing Metrics | ||
| * to be validated with {@link schemaSerializedPonderIndexingMetrics}. | ||
| */ | ||
| function buildUnvalidatedPonderIndexingMetrics(metricsText: string): unknown { |
There was a problem hiding this comment.
We use unknown type to highlight that the returned object is unpredictable in terms of fields it's going to contain. The buildUnvalidatedPonderIndexingMetrics is implementation detail of schemaSerializedPonderIndexingMetrics, which ultimately validates the unvalidated Ponder Indexing Metrics object.
| .getLabels("ponder_sync_block", "chain") | ||
| .map((chainIdString) => deserializeChainIdString(chainIdString)); | ||
|
|
||
| const chains = new Map<ChainId, unknown>(); |
There was a problem hiding this comment.
Again, the unknown type let us avoid fighting TS compiler which wants us to declare a type for all possible partial outcomes.
| dts: true, | ||
| clean: true, | ||
| external: ["zod/*"], // Mark peer dependencies as external | ||
| noExternal: ["parse-prometheus-text-format"], |
There was a problem hiding this comment.
We need the parse-prometheus-text-format dependency to be included in the output bundle for the Ponder SDK package.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/ponder-sdk/src/deserialize/indexing-metrics.ts`:
- Around line 170-179: The deserializePonderIndexingMetrics function currently
calls schemaSerializedPonderIndexingMetrics.safeParse(data) but preprocessors
(PrometheusMetrics.parse and deserializeChainIdString) can throw outside
safeParse; wrap the safeParse invocation in a try-catch so any thrown exception
is caught, convert the caught error into a normalized message using
prettifyError (or include the error message) and rethrow a single Error with the
existing `Invalid serialized Ponder Indexing Metrics: ...` format; keep
references to deserializePonderIndexingMetrics,
schemaSerializedPonderIndexingMetrics, PrometheusMetrics.parse,
deserializeChainIdString, and prettifyError to locate the change.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 12 changed files in this pull request and generated 2 comments.
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
lightwalker-eth
left a comment
There was a problem hiding this comment.
@tk-o Nice work on this PR. Some small feedback. Please take the lead to merge when ready 👍
| /** | ||
| * Ordering strategy for onchain data used during indexing. | ||
| */ | ||
| ordering: "omnichain"; |
There was a problem hiding this comment.
Suggest adding a comment here about how support for other Ponder indexing strategies is planned for the future.
| /** | ||
| * Ponder Application Settings | ||
| * | ||
| * Represents the application-level settings for Ponder application. |
There was a problem hiding this comment.
| * Represents the application-level settings for Ponder application. | |
| * Represents the application-level settings for a Ponder app. |
| /** | ||
| * Chain Indexing Metrics | ||
| * | ||
| * Represents the indexing metrics for a specific chain indexed by Ponder application. |
There was a problem hiding this comment.
| * Represents the indexing metrics for a specific chain indexed by Ponder application. | |
| * Represents the indexing metrics for a specific chain indexed by a Ponder app. |
| */ | ||
| interface ChainIndexingMetrics { | ||
| /** | ||
| * Number of blocks required to be synced during backfill. |
There was a problem hiding this comment.
| * Number of blocks required to be synced during backfill. | |
| * Number of blocks required to be synced to complete the backfill phase of indexing. |
It would also help if you could clarify some things here more in comments. For example, as I understand the Ponder config defines the start block in the range that is used to calculate this value. For the end block in the range that is used to calculate this value: how / when is it calculated? Does it change across time for an active Ponder instance? What happens to this value after backfill is completed but then the Ponder app is restarted? When it restarts how then is this value calculated?
| /** | ||
| * Settings related to how the Ponder application is configured to index onchain data. | ||
| */ | ||
| application: PonderApplicationSettings; |
There was a problem hiding this comment.
| application: PonderApplicationSettings; | |
| appSettings: PonderApplicationSettings; |
| /** | ||
| * Schema representing settings of a Ponder app. | ||
| */ | ||
| const schemaSerializedApplicationSettings = z.object({ |
There was a problem hiding this comment.
Is there an easy way to avoid passing arrays of hardcoded strings into the z.enum calls here? Can we pass the existing definitions you created in a different file?
There was a problem hiding this comment.
Sure thing, will define "const enums" for those and import the values from business layer.
|
@greptile review |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@packages/ponder-sdk/src/client.test.ts`:
- Around line 101-158: The two tests ("should handle invalid Ponder application
settings in the response" and "should handle metrics using non-int chain names
in the response") nest assertions inside catch blocks so they silently pass if
PonderClient.metrics() does not throw; change each to use the same pattern as
the earlier test: call const ponderClient = new PonderClient(new
URL("http://localhost:3000")); then await
expect(ponderClient.metrics()).rejects.toThrowError() (and assert the specific
substrings/regexes you currently check for), or otherwise capture the thrown
error with await and assert its message outside a try/catch—apply this change to
both tests referencing PonderClient.metrics() and the existing expected error
messages.
In `@packages/ponder-sdk/src/deserialize/indexing-metrics.ts`:
- Around line 40-57: The backfillSyncBlocksTotal field in
schemaSerializedChainIndexingMetrics currently uses schemaPositiveInteger which
disallows zero; change that field to use a non-negative integer schema (e.g.,
makeNonNegativeIntegerSchema() or the existing schema that permits zero) so zero
values are accepted during deserialization, and add the corresponding import if
missing; update the schemaSerializedChainIndexingMetrics definition to reference
the non-negative integer validator for backfillSyncBlocksTotal.
In `@packages/ponder-sdk/src/indexing-metrics.ts`:
- Around line 58-62: Fix the doc typo in the backfill metric comment: locate the
comment that starts "Number of blocks required to be synced to complete the
backfill phase of indexing." and replace the phrase "calculated determined by
Ponder" with a single correct wording such as "calculated by Ponder" (or
"determined by Ponder") so the sentence reads "...This value is calculated by
Ponder at the time the backfill starts." Ensure the updated comment is applied
to the same metric/documentation block.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 19 changed files in this pull request and generated 3 comments.
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 19 changed files in this pull request and generated 2 comments.
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Lite PR
Tip: Review docs on the ENSNode PR process
Summary
PonderClientinponder-sdkwas extended with the following methods:health()allows running Ponder application healthcheckmetrics()allows fetching information about Ponder Indexing Metrics which include the following fieldsapplicationto capture relevant settings that Ponder application was initialized withchainsto describe indexing metrics for each indexed chain. Indexing metrics for each chain include:Why
Testing
Notes for Reviewer (Optional)
Pre-Review Checklist (Blocking)