Skip to content

Conversation

MasterPtato
Copy link
Contributor

Changes

Copy link
Contributor Author

MasterPtato commented May 28, 2025

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more


How to use the Graphite Merge Queue

Add the label merge-queue to this PR to add it to the merge queue.

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

This PR enhances Tokio runtime observability while simplifying guard service metrics to reduce cardinality.

  • Added TOKIO_TASK_TOTAL and TOKIO_ACTIVE_TASK_COUNT metrics in packages/common/runtime/src/metrics.rs for tracking task lifecycle
  • Removed actor_id, server_id, method, and path labels from PROXY_REQUEST_TOTAL and PROXY_REQUEST_PENDING in packages/edge/infra/guard/core/src/metrics.rs
  • Simplified PROXY_REQUEST_DURATION to only track status label
  • Added error tracking via PROXY_REQUEST_ERROR with error_type label
  • Configured Tokio runtime builder to collect new task metrics in packages/common/runtime/src/lib.rs

4 file(s) reviewed, 2 comment(s)
Edit PR Review Bot Settings | Greptile

Comment on lines +688 to +699
metrics::PROXY_REQUEST_ERROR
.with_label_values(&[&err.to_string()])
.inc();
Copy link

Choose a reason for hiding this comment

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

style: Consider using structured error types instead of error.to_string() to avoid high cardinality in error labels

Comment on lines 662 to 663
metrics::PROXY_REQUEST_PENDING.inc();

metrics::PROXY_REQUEST_TOTAL
.with_label_values(&[&actor_id_str, &server_id_str, method_str, &path])
.inc();
metrics::PROXY_REQUEST_TOTAL.inc();
Copy link

Choose a reason for hiding this comment

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

logic: Metrics incremented before error handling - could lead to inaccurate counts if subsequent operations fail

Copy link

cloudflare-workers-and-pages bot commented May 28, 2025

Deploying rivet with  Cloudflare Pages  Cloudflare Pages

Latest commit: 0bf9b0a
Status: ✅  Deploy successful!
Preview URL: https://f83682c1.rivet.pages.dev
Branch Preview URL: https://05-28-fix-guard-add-more-tok.rivet.pages.dev

View logs

@MasterPtato MasterPtato force-pushed the 05-28-fix_guard_add_more_tokio_runtime_metrics_remove_labels_from_metrics branch from fca464f to ce70387 Compare May 29, 2025 21:03
Copy link

cloudflare-workers-and-pages bot commented May 29, 2025

Deploying rivet-studio with  Cloudflare Pages  Cloudflare Pages

Latest commit: 0bf9b0a
Status: ✅  Deploy successful!
Preview URL: https://9b699cf3.rivet-studio.pages.dev
Branch Preview URL: https://05-28-fix-guard-add-more-tok.rivet-studio.pages.dev

View logs

Copy link

cloudflare-workers-and-pages bot commented May 29, 2025

Deploying rivet-hub with  Cloudflare Pages  Cloudflare Pages

Latest commit: 0bf9b0a
Status: ✅  Deploy successful!
Preview URL: https://255bbf9c.rivet-hub-7jb.pages.dev
Branch Preview URL: https://05-28-fix-guard-add-more-tok.rivet-hub-7jb.pages.dev

View logs

@NathanFlurry NathanFlurry force-pushed the 05-28-fix_guard_replace_internal_caches_with_moka branch from fcd1624 to d958baa Compare May 31, 2025 00:39
@NathanFlurry NathanFlurry force-pushed the 05-28-fix_guard_add_more_tokio_runtime_metrics_remove_labels_from_metrics branch from ce70387 to 08138ed Compare May 31, 2025 00:39
@MasterPtato MasterPtato force-pushed the 05-28-fix_guard_replace_internal_caches_with_moka branch from d958baa to 4134605 Compare May 31, 2025 00:54
@MasterPtato MasterPtato force-pushed the 05-28-fix_guard_add_more_tokio_runtime_metrics_remove_labels_from_metrics branch from 08138ed to f6d2ad5 Compare May 31, 2025 00:54
@NathanFlurry NathanFlurry force-pushed the 05-28-fix_guard_replace_internal_caches_with_moka branch from 4134605 to d958baa Compare May 31, 2025 01:01
@NathanFlurry NathanFlurry force-pushed the 05-28-fix_guard_add_more_tokio_runtime_metrics_remove_labels_from_metrics branch from f6d2ad5 to 08138ed Compare May 31, 2025 01:01
@MasterPtato MasterPtato force-pushed the 05-28-fix_guard_replace_internal_caches_with_moka branch from d958baa to 4134605 Compare May 31, 2025 01:04
@MasterPtato MasterPtato force-pushed the 05-28-fix_guard_add_more_tokio_runtime_metrics_remove_labels_from_metrics branch from 08138ed to f6d2ad5 Compare May 31, 2025 01:04
@NathanFlurry NathanFlurry force-pushed the 05-28-fix_guard_replace_internal_caches_with_moka branch from 4134605 to d958baa Compare May 31, 2025 01:52
@NathanFlurry NathanFlurry force-pushed the 05-28-fix_guard_add_more_tokio_runtime_metrics_remove_labels_from_metrics branch from f6d2ad5 to 08138ed Compare May 31, 2025 01:52
@MasterPtato MasterPtato force-pushed the 05-28-fix_guard_replace_internal_caches_with_moka branch from d958baa to 4134605 Compare May 31, 2025 02:05
@MasterPtato MasterPtato force-pushed the 05-28-fix_guard_add_more_tokio_runtime_metrics_remove_labels_from_metrics branch from 08138ed to f6d2ad5 Compare May 31, 2025 02:05
@MasterPtato MasterPtato force-pushed the 05-28-fix_guard_replace_internal_caches_with_moka branch from 4134605 to 6caa5bf Compare June 2, 2025 18:13
@MasterPtato MasterPtato force-pushed the 05-28-fix_guard_add_more_tokio_runtime_metrics_remove_labels_from_metrics branch from f6d2ad5 to 0bf9b0a Compare June 2, 2025 18:13
@graphite-app graphite-app bot closed this Jun 3, 2025
@graphite-app graphite-app bot deleted the 05-28-fix_guard_add_more_tokio_runtime_metrics_remove_labels_from_metrics branch June 3, 2025 07:03
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.

1 participant