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: gateway tracking requested cids in database #1386

Closed

Conversation

vasco-santos
Copy link
Member

@vasco-santos vasco-santos commented Feb 16, 2022

This PR adds metrics tracking whether content was stored in NFT.storage and its Pinning status characteristics. It uses nft.storage client to check the state of the content. We could go through the route of directly connecting to the DB here, but as we do this in the waitUntil it seems cleaner to rely on the client (specially as we can't access replica directly).

Prometheus metrics to be consumed are as follows:

# HELP nftgateway_errored_responses_with_known_content_total errored requests with known content in NFT.storage.
# TYPE nftgateway_errored_responses_with_known_content_total counter
nftgateway_errored_responses_with_known_content_total{env="test"} 0
# HELP nftgateway_responses_by_content_status_total total of responses by content status. Either stored or not stored.
# TYPE nftgateway_responses_by_content_status_total counter
nftgateway_responses_by_content_status_total{env="test",status="Stored"} 0
nftgateway_responses_by_content_status_total{env="test",status="NotStored"} 0
# HELP nftgateway_responses_by_pin_status_total total of responses by pin status.
# TYPE nftgateway_responses_by_pin_status_total counter
nftgateway_responses_by_pin_status_total{env="test",status="PinError"} 0
nftgateway_responses_by_pin_status_total{env="test",status="Pinned"} 0
nftgateway_responses_by_pin_status_total{env="test",status="Pinning"} 0
nftgateway_responses_by_pin_status_total{env="test",status="PinQueued"} 0
# HELP nftgateway_responses_per_time_by_status_total total of responses by status per response time bucket
# TYPE nftgateway_responses_per_time_by_status_total histogram
nftgateway_responses_per_time_by_status_total{status="PinError",le="0.05",env="test"} 0
nftgateway_responses_per_time_by_status_total{status="Pinned",le="0.05",env="test"} 1
nftgateway_responses_per_time_by_status_total{status="Pinning",le="0.05",env="test"} 0
nftgateway_responses_per_time_by_status_total{status="PinQueued",le="0.05",env="test"} 0
nftgateway_responses_per_time_by_status_total{status="PinError",le="0.1",env="test"} 0
nftgateway_responses_per_time_by_status_total{status="Pinned",le="0.1",env="test"} 2
nftgateway_responses_per_time_by_status_total{status="Pinning",le="0.1",env="test"} 0
nftgateway_responses_per_time_by_status_total{status="PinQueued",le="0.1",env="test"} 0
(...)

Follow up work (will create issue):

  • improve testing by spinning up API locally with Miniflare

Should use #1391 cache in check endpoint

Closes nftstorage/nftstorage.link#14

@cloudflare-pages
Copy link

cloudflare-pages bot commented Feb 16, 2022

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: 2c437ae
Status: ✅  Deploy successful!
Preview URL: https://8b008d6b.nft-storage-1at.pages.dev

View logs

@vasco-santos vasco-santos force-pushed the feat/gateway-tracking-requested-cids-in-database branch 3 times, most recently from ed439c0 to cdcbf74 Compare February 16, 2022 17:38

/**
* Durable Object for keeping summary metrics of gateway.nft.storage
*/
export class SummaryMetrics0 {
constructor(state) {
this.state = state
// @ts-ignore we don't need token just for check
this.nftStorageClient = new NFTStorage({})
Copy link
Member Author

Choose a reason for hiding this comment

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

For just check, we don't need a token, while it is "required" in NFTstorage (but param called option)

@hugomrdias should we make it optional in the types?

Copy link
Contributor

Choose a reason for hiding this comment

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

yes we should, but here if you just use it for check call might be better to not include the client.

Copy link
Member Author

Choose a reason for hiding this comment

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

you mean, just HTTP the endpoint?

Copy link
Collaborator

Choose a reason for hiding this comment

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

yep, I agree, if we just want to hit the public check endpoint, maybe just add a helper function around a fetch

@vasco-santos vasco-santos marked this pull request as ready for review February 16, 2022 17:48
@olizilla
Copy link
Collaborator

notable that this PR is the only thing that ties the gateway impl to nftstorage. It would be neat to have a generic "racing gateway proxy" that anyone could deploy, and then have some mechinism for adding extra metrics for a given deployment.

We might want to reuse this for a web3.storage gateway

@vasco-santos
Copy link
Member Author

notable that this PR is the only thing that ties the gateway impl to nftstorage. It would be neat to have a generic "racing gateway proxy" that anyone could deploy, and then have some mechinism for adding extra metrics for a given deployment.

We can have a check endpoint ENV Var to have this generic

@vasco-santos
Copy link
Member Author

Closing this and we will track on issue nftstorage/nftstorage.link#14 how to gather this

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.

Gateway tracking whether requested content is in Database
3 participants