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

CF Metrics, Tests, and Fixes #33

Merged
merged 39 commits into from
Jul 18, 2024
Merged

CF Metrics, Tests, and Fixes #33

merged 39 commits into from
Jul 18, 2024

Conversation

neonphog
Copy link
Collaborator

@neonphog neonphog commented Jul 11, 2024

RESOLVES #29

  • Switches to the newly-recommended vitest-pool-workers wrangler test methodology
  • Adds basic bytes received (and connection count) metrics, and api-key based metrics reporting
  • Total rewrite of rate limit code, and tests
  • Dead-status early return check in signal Durable Object

@neonphog neonphog changed the title wip metrics CF Metrics, Tests, and Fixes Jul 15, 2024
@neonphog neonphog marked this pull request as ready for review July 15, 2024 20:22
@@ -0,0 +1,108 @@
import { fromUint8Array, toUint8Array } from 'js-base64';
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The contents are this file are just consolidated from other files, no code changes here.

Comment on lines +82 to +86
if (this.status !== 200) {
return new Response(JSON.stringify({ err: 'dead' }), {
status: this.status,
});
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

added these checks that short-circuit cpu time when the object has already errored / closed

#cpu_ms = 5000

kv_namespaces = [
{ binding = "SBD_COORDINATION", id = "cde857dd4ea745fea55f80daa52a6d5c" }
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This kv store is where metrics are stored.

const { pubKeyStr } = parsePubKey(url.pathname);
let pathParts = url.pathname.split('/');

if (pathParts.length === 4 && pathParts[1] === 'metrics') {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

here's the new metrics entrypoint

Copy link
Member

@ThetaSinner ThetaSinner left a comment

Choose a reason for hiding this comment

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

Looking good! and pointed a wind tunnel test at it this morning, seems to be working nicely :)

*Just a local one, not on HPs yet

```
# HELP client.count active client count
# TYPE client.count guage
client.count 4
Copy link
Member

Choose a reason for hiding this comment

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

Are these recorded somewhere? Otherwise it's just a point in time and it'd be useful to have this recorded over time

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The intention would be for these to be scraped by a grafana somewhere. It'd be too expensive to aggregate them in cloudflare itself.

ts/sbd-server/src/prom.ts Outdated Show resolved Hide resolved
@neonphog neonphog enabled auto-merge (squash) July 18, 2024 21:01
@neonphog neonphog merged commit 4de593c into main Jul 18, 2024
5 checks passed
@neonphog neonphog deleted the cf-metrics branch July 18, 2024 21:22
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.

(cf-sbd-server) publish to cloudflare and run sanity tests against production
3 participants