-
Notifications
You must be signed in to change notification settings - Fork 10
feat(ensindexer): /api/indexing-status
API endpoint
#896
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
base: feat/ensindexer-api-config-endpoint
Are you sure you want to change the base?
feat(ensindexer): /api/indexing-status
API endpoint
#896
Conversation
Defines types to describe ENSIndexerIndexingStatus with serialize & deserialize funtionality.
…nfig` This will make it easier to read the ponder config object from other files.
Adds support for `ponder_sync_block_timestamp` metric
🦋 Changeset detectedLatest commit: 3512434 The changes in this PR will be included in the next version bump. This PR includes changesets to release 10 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 Git ↗︎
|
…ing serialized ENSIndexerIndexingStatus object.
import { ENSIndexerIndexingStatus } from "@ensnode/ensnode-sdk"; | ||
import { prettifyError } from "zod/v4"; | ||
|
||
export const indexedChainNames = Object.keys(ponderConfig.chains) as [string, ...string[]]; |
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.
why not just string[]
as a type?
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.
ah, i see that this perhaps ensures there's at least 1 value in the array... interesting...
const ponderStatusUrl = new URL("/status", ponderAppUrl); | ||
|
||
try { | ||
const metricsText = await fetch(ponderStatusUrl).then((r) => r.json()); |
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.
status
not metrics
also nit:
const response = await fetch(ponderStatusUrl);
if (!response.ok) throw new Error(...)
return await response.json();
// or...
const status = await response.json();
return status as PonderStatus;
and if we need a standard pattern for "fetch json and throw if error" then let's make a helper fetcher like
async function fetchJSON(url: string) {
try {
const response = await fetch(url);
if (!response.ok) throw new Error(`Invalid HTTP Status Response: ${response.statusCode}`)
return await response.json();
} catch (error) {
if (error instanceof Error) throw error;
throw new Error('Unknown Error')
}
}
and use it for metrics and status fetches
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.
could be
async function fetchJSON(...args: Parameters<typeof fetch>) {
// ...
return await fetch(...args);
// ...
}
blockNumber: BlockNumber, | ||
): Promise<BlockRef> { | ||
const block = await publicClient.getBlock({ blockNumber: BigInt(blockNumber) }); | ||
const blockCreatedAt = new Date(Date.parse(block.timestamp.toString())); |
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.
nit: should also be timestamp
not createdAt
|
||
export type ChainName = string; | ||
|
||
export type PonderBlockRef = { |
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.
see comments on previous prs but this should be the type of our BlockRef
and then PonderBlockRef
becomes unneccessary
@@ -0,0 +1,134 @@ | |||
import type { BlockRef, ChainId, Duration } from "../../shared"; |
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.
using the simpler BlockRef
type { number: number, timestamp: number }
also removes the need for all of the generics in this file
* The block refs must be fetched before the {@link DEFAULT_METRICS_FETCH_TIMEOUT} timeout occurs. | ||
* Otherwise, the ENSIndexer process must crash. | ||
*/ | ||
export const indexedChainsBlockRefs = fetchChainsBlockRefs( |
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.
is it necessary to fetch this information at start-time, or could it be deferred until the api handler for indexing-status is executed, and then cached? it would make sense to me to, instead of terminating the indexer if the metrics page isn't available, to just throw a 500 error in the api handler.
* Invariants: | ||
* - every backfillEnd value is a valid {@link BlockNumber}. | ||
*/ | ||
async function tryGettingBackfillEndBlocks( |
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.
let's replace the retry logic here with a simple implementation of fetch with retries in some helper file?
async function withRetries(fn, retries = 3, delay = 1000) {
try {
return await fn();
} catch (error) {
// bail early if explicitly aborted
if (error instanceof AbortError) throw error;
if (retries > 0) {
console.warn(`failed, retrying in ${delay}ms... (${retries} retries left)`);
await new Promise(resolve => setTimeout(resolve, delay));
return withRetries(fn, retries - 1, delay);
} else {
console.error('All retries failed.');
throw error;
}
}
}
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.
i don't think the timeout should be configurable, right? presumably we're the most informed about how long it takes ponder to serve the metrics page? if we defer this fetch until the api handler is executed, then we might not even need retry behavior? if the metrics page is unreachable, the api handler can 500 instead of terminating the process
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.
if it is necessary to fetch this information as soon as the indexer starts up, then the current idea of creating a promise at the top level and then awaiting it is acceptable (also document that the at-startup request is necessary), but i still think we can replace this timeout-based logic here with either a simpler retry logic as above OR, if timeouts are absolutely required, an AbortController
-based timeout where that abort controller is passed to each fetch used in the retry loop.
// fetchPonderMetricsWithTimeoutAndInterval(...)
const controller = new AbortController();
const timeout = setTimeout(() => controller.abort(), METRICS_FETCH_TIMEOUT);
return await withRetries(() => fetchPonderMetrics(controller), 4, METRICS_FETCH_INTERVAL)
const backfillEndBlock = chainsBackfillEndBlock[chainName]; | ||
const publicClient = publicClients[chainName]; | ||
|
||
if (typeof startBlock === "undefined") { |
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.
is this a use-case for a zod schema? also entirely acceptable as-is
} | ||
|
||
return { | ||
number: Number(block.number), |
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.
rename this to fetchBlockRef
to match return type
let's document here that viem returns bigint
typed block properties but we can safely cast them to number because of reality
This PR builds on top of #895. The major updates this PR brings:
Creates the
src/ensindexer/indexing-status
module in ENSNode SDK package which:ENSIndexerIndexingStatus
type and its serialized counterpart,isSubgraphCompatible
function useful for validation,Integrates the module from above into the ENSIndexer application:
GET /api/indexing-status
endpoint which returnsSerializedENSIndexerIndexingStatus
value.ponder.config.ts
into@/ponder/config
, so it's easy to import the Ponder config object in other application files (withimport ponderConfig from "@/ponder/config";
.@/indexing-status
module, including:@ensnode/ponder-metadata
package later on.buildIndexingStatus
function that is used to handle eachGET /api/indexing-status
requestWhen ENSIndexer application starts, it will attempt to fetch Ponder Metrics to learn about the backfill stats, and some ponder application settings. Later, still during the application start, RPC calls are made to get BlockRef data (number, timestamp) for relevant block numbers per chain (startBlock, endBlock, backfillEndBlock).
The timeout may occur if required Ponder Metrics could not be read, or the RPC calls could not be completed. In case of that timeout, we expect the ENSIndexer application to crash,.
Ponder Status and Ponder Metrics must always be fetched from ENSIndexer instance (which uses the
omnichain
ordering strategy) and never from ENSApi instance. That's why we introduced theensIndexerPrivateUrl
configuration param. This param allows the ENSApi instance to know which ENSIndexer instance it should call to fetch Ponder Status and Ponder Mertics.There's one new metric used to derive indexing status, it's called
ponder_sync_block_timestamp
. It was introduced in Ponder version>=0.11.40
. This PR updates Ponder to version0.11.43
.