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: filecoin metrics for ucan stream prometheus and updates metrics endpoint #290

Merged
merged 3 commits into from
Dec 7, 2023

Conversation

vasco-santos
Copy link
Contributor

@vasco-santos vasco-santos commented Nov 30, 2023

Adds filecoin metrics for ucan stream in the same pattern we track other metrics in https://up.web3.storage/metrics

We want to rely on UCAN Stream metrics table to in a "prometheus style" increment metrics over time for totals. There are some metrics in Firehose that we can rely on for ad-hoc queries and business queries (like monthly usage). More than the price and efficiency in the long run for totals in the Firehose, there are some limitations that are explicit here:

  • we use PieceCids to encode Piece size, which we will never be able to handle in Athena level as we cannot decode the CID to get the size
  • UCAN invocations (like aggregate/offer) may rely on attached blocks to move attachments with the invocation, while only identifying the CID for them. E.g in aggregate/offer we encode the very large list of pieces to put into the aggregate in a CBOR block, and inline it in the CAR of the invocation, while providing pieces as CID of the block in the nb object.

We will need most measures I wanted to handle, except for tracking execution time between the invocations. We need storacha/w3up#970 to get the power to track execution time of a UCAN effect (e.g. time between receipt of aggregate/offer and aggregate/accept

Copy link

seed-deploy bot commented Nov 30, 2023

View stack outputs

@vasco-santos vasco-santos force-pushed the feat/firehose-filecoin-invocation branch from c954475 to 8e039cd Compare December 1, 2023 08:22
@vasco-santos vasco-santos changed the title feat: firehose filecoin invocation feat: firehose filecoin metrics Dec 1, 2023
@vasco-santos vasco-santos changed the title feat: firehose filecoin metrics feat: filecoin metrics for firehose and ucan stream prometheus based Dec 1, 2023
@vasco-santos vasco-santos force-pushed the feat/firehose-filecoin-invocation branch from 8e039cd to b19c1ef Compare December 4, 2023 13:20
@vasco-santos vasco-santos force-pushed the feat/firehose-filecoin-invocation branch from b19c1ef to 9e9c3b0 Compare December 5, 2023 21:05
@vasco-santos vasco-santos force-pushed the feat/firehose-filecoin-invocation branch from 9e9c3b0 to 3da3463 Compare December 5, 2023 21:15
@vasco-santos vasco-santos changed the title feat: filecoin metrics for firehose and ucan stream prometheus based feat: filecoin metrics for ucan stream prometheus and updates metrics endpoint Dec 5, 2023
@vasco-santos vasco-santos marked this pull request as ready for review December 5, 2023 21:28
@@ -48,16 +62,17 @@ export type UcanInvocationType = 'workflow' | 'receipt'

export interface UcanInvocation {
carCid: string
invocationCid: string
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

Yes but, only for receipts.

I ended up re-typing these definitions in billing to more accurately reflect what's there and not get errors for properties that don't always exist:

https://github.com/web3-storage/w3infra/blob/9897890c65968f7d47a4b3b520f736863343642b/billing/lib/api.ts#L210-L240

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is great @alanshaw thanks!
I will add this as follow up PR

Copy link
Member

@alanshaw alanshaw left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -48,16 +62,17 @@ export type UcanInvocationType = 'workflow' | 'receipt'

export interface UcanInvocation {
carCid: string
invocationCid: string
Copy link
Member

Choose a reason for hiding this comment

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

Yes but, only for receipts.

I ended up re-typing these definitions in billing to more accurately reflect what's there and not get errors for properties that don't always exist:

https://github.com/web3-storage/w3infra/blob/9897890c65968f7d47a4b3b520f736863343642b/billing/lib/api.ts#L210-L240

@vasco-santos vasco-santos merged commit fa37784 into main Dec 7, 2023
3 checks passed
@vasco-santos vasco-santos deleted the feat/firehose-filecoin-invocation branch December 7, 2023 08:57
Copy link

sentry-io bot commented Dec 7, 2023

Suspect Issues

This pull request was deployed and Sentry observed the following issues:

  • ‼️ DecodeBlockOperationError: missing block: [object Object] staging-w3infra-UcanInvoc-metricsaggregateoffer... View Issue

Did you find this useful? React with a 👍 or 👎

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.

3 participants