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 parsing of health check event #4

Merged
merged 16 commits into from
Jul 26, 2024
Merged

feat: add parsing of health check event #4

merged 16 commits into from
Jul 26, 2024

Conversation

nvtaveras
Copy link
Contributor

@nvtaveras nvtaveras commented Jul 23, 2024

This adds the parsing of a second event, MedianUpdated (from SortedOracles), which acts as a health check/heartbeat for the cloud function, as it's a frequently emitted event that can serve as an indicator of quicknode working properly.

The parsing of the event results in a single console.log of the action, which will then be used for creating a metric on Google Cloud Monitoring around how many heartcheck logs are present in a given timespan.

@nvtaveras nvtaveras changed the title WIP: health check feat: add parsing of health check event Jul 25, 2024
Copy link
Collaborator

@chapati23 chapati23 left a comment

Choose a reason for hiding this comment

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

looks great, just a few questions

oh, and CI is red. i merged main into this branch to get the CI checks working and it complained about a few linting things

package.json Outdated Show resolved Hide resolved
src/health-check.fixture.json Show resolved Hide resolved
src/index.ts Outdated
console.info(`[HealthCheck]: Block ${parsedEvent.block}`);
break;
default:
console.warn("Unknown event type:", parsedEvent.event);
Copy link
Collaborator

Choose a reason for hiding this comment

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

we could even throw here instead of just warn. if it's just a warning it might drown in the other logs and we may never know about it.

throwing would alert us much earlier if somebody were to try to DDoS this function, wdyt?

}
if (isProposalCreatedEvent(event)) {
result.push({
block: Number(receipt.blockNumber),
Copy link
Collaborator

Choose a reason for hiding this comment

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

why do we need the block number? if we have the tx hash we could look up the block number easily, no?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wanted to log it on every health check event to know which block is the event coming from, which is good to know in case quicknode acts weird in any way (for example sending events multiple times, they fall behind from the tip of the chain, etc).

We don't really need it for the proposal events though so I can remove it from those

Copy link
Collaborator

Choose a reason for hiding this comment

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

gotcha, i'd say let's keep it only on the healthcheck then

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated

@nvtaveras nvtaveras merged commit 16fd387 into main Jul 26, 2024
1 check passed
@nvtaveras nvtaveras deleted the feat/healthCheck branch July 26, 2024 15:49
chapati23 pushed a commit that referenced this pull request Aug 6, 2024
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.

2 participants