fix: add future/fdb metrics #2377

MasterPtato · 2025-04-24T00:06:28Z

Changes

MasterPtato · 2025-04-24T00:06:45Z

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.

cloudflare-workers-and-pages · 2025-04-24T00:10:08Z

Deploying rivet with Cloudflare Pages

Latest commit:	`2ffbbd6`
Status:	✅ Deploy successful!
Preview URL:	https://1790567b.rivet.pages.dev
Branch Preview URL:	https://04-23-fix-add-future-fdb-met.rivet.pages.dev

View logs

greptile-apps

PR Summary

This PR adds comprehensive metrics and tracing capabilities across the Rivet codebase, focusing on FoundationDB operations and future durations.

Added new Grafana dashboard futures.json for monitoring instrumented future durations with heatmap visualization
Introduced CustomInstrumentExt trait in future.rs for tracking future durations with Prometheus metrics, though has potential safety issues with unsafe code
Added process-exporter installation script and configuration in /packages/core/services/cluster/src/workflows/server/install/install_scripts/ but lacks security measures like checksum verification
Replaced foundationdb::tuple::Subspace with fdb_util::Subspace across multiple files for better metrics tracking and consistency
Added custom instrumentation spans to various FDB transactions throughout the codebase for improved tracing and monitoring

_{28 file(s) reviewed, 10 comment(s)}
_{Edit PR Review Bot Settings | Greptile}

greptile-apps · 2025-04-24T22:43:52Z

docker/dev-full/grafana/dashboards/futures.json

+				"filterValues": {
+					"le": 1e-9
+				},


logic: filterValues.le is set to 1e-9 which is an extremely small value that may filter out relevant data points. Consider adjusting or removing this filter.

greptile-apps · 2025-04-24T22:43:53Z

docker/dev-full/grafana/dashboards/futures.json

+				"yAxis": {
+					"axisPlacement": "left",
+					"max": "60",
+					"min": 0,
+					"reverse": false,
+					"unit": "s"
+				}


style: yAxis max value is hardcoded to 60s which may not be suitable for all future durations. Consider making this dynamic or configurable via variables.

greptile-apps · 2025-04-24T22:45:25Z

packages/common/chirp-workflow/core/src/worker.rs

+				res = &mut gc_handle => {
+					tracing::error!(?res, "metrics task unexpectedly stopped");
+					break;


logic: Error message incorrectly states 'metrics task unexpectedly stopped' when it's actually the GC task that stopped

Suggested change

res = &mut gc_handle => {

tracing::error!(?res, "metrics task unexpectedly stopped");

break;

res = &mut gc_handle => {

tracing::error!(?res, "gc task unexpectedly stopped");

break;

greptile-apps · 2025-04-24T22:46:18Z

packages/common/util/core/src/future.rs

+	fn poll(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<Self::Output> {
+		let this = unsafe { self.get_unchecked_mut() };
+		let inner = unsafe { Pin::new_unchecked(&mut this.inner) };
+


logic: Unsafe code can be replaced with safe alternatives. Use Pin::as_mut().get_mut() and Pin::as_mut() instead of get_unchecked_mut and new_unchecked.

Suggested change

fn poll(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<Self::Output> {

let this = unsafe { self.get_unchecked_mut() };

let inner = unsafe { Pin::new_unchecked(&mut this.inner) };

fn poll(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<Self::Output> {

let this = self.as_mut().get_mut();

let inner = Pin::new(&mut this.inner);

greptile-apps · 2025-04-24T22:46:18Z

packages/common/util/core/src/future.rs

+		let this = unsafe { self.get_unchecked_mut() };
+		let inner = unsafe { Pin::new_unchecked(&mut this.inner) };
+
+		let metadata = inner.span().metadata().clone();


style: Cloning metadata on every poll is inefficient. Consider storing the formatted location string in the struct during construction.

greptile-apps · 2025-04-24T22:46:19Z

...core/services/cluster/src/workflows/server/install/install_scripts/files/process_exporter.sh

+User=process-exporter
+Group=process-exporter
+Type=simple
+ExecStart=/usr/bin/process-exporter --config.path /etc/process-exporter/config.yaml


style: Missing security-related systemd directives like ProtectSystem=strict and NoNewPrivileges=true

greptile-apps · 2025-04-24T22:46:55Z

packages/edge/api/traefik-provider/src/route/game_guard/actor.rs

logic: The entire file has been deleted. This file contains critical routing logic for actors and should not be removed. Please restore this file and add the metrics changes separately.

greptile-apps · 2025-04-24T22:46:58Z

packages/edge/api/intercom/src/route/pegboard.rs

@@ -44,6 +44,7 @@ pub async fn prewarm_image(
 				Ok(None)
 			}
 		})
+		.custom_instrument(tracing::info_span!("prewarm_fetch_tx"))


logic: The tracing span should be placed before the transaction starts (line 25) rather than after it completes to properly capture the full transaction duration

Suggested change

.custom_instrument(tracing::info_span!("prewarm_fetch_tx"))

.custom_instrument(tracing::info_span!("prewarm_fetch_tx"))

.run(|tx, _mc| async move {

greptile-apps · 2025-04-24T22:47:26Z

packages/edge/services/pegboard/src/ops/actor/get.rs

@@ -110,6 +110,7 @@ pub async fn pegboard_actor_get(ctx: &OperationCtx, input: &Input) -> GlobalResu
 				.try_collect::<Vec<_>>()
 				.await
 		})
+		.custom_instrument(tracing::info_span!("actor_list_wf_tx"))


style: Consider moving the custom_instrument call before the run() to capture the entire FDB operation including setup

greptile-apps · 2025-04-24T22:48:18Z

packages/edge/services/pegboard/src/workflows/actor/destroy.rs

@@ -161,6 +161,7 @@ pub async fn update_fdb(ctx: &ActivityCtx, input: &UpdateFdbInput) -> GlobalResu
 				.await
 			}
 		})
+		.custom_instrument(tracing::info_span!("actor_destroy_tx"))


logic: The span should be added before line 131 where the transaction begins, not after the transaction completes. This ensures the entire transaction is properly instrumented.

cloudflare-workers-and-pages · 2025-04-29T22:25:39Z

Deploying rivet-studio with Cloudflare Pages

Latest commit:	`2ffbbd6`
Status:	✅ Deploy successful!
Preview URL:	https://306c3dae.rivet-studio.pages.dev
Branch Preview URL:	https://04-23-fix-add-future-fdb-met.rivet-studio.pages.dev

View logs

cloudflare-workers-and-pages · 2025-04-29T22:30:11Z

Deploying rivet-hub with Cloudflare Pages

Latest commit:	`2ffbbd6`
Status:	🚫 Build failed.

View logs

graphite-app · 2025-05-02T01:57:25Z

Merge activity

May 1, 9:57 PM EDT: MasterPtato added this pull request to the Graphite merge queue.
May 1, 9:58 PM EDT: CI is running for this pull request on a draft pull request (#2422) due to your merge queue CI optimization settings.
May 1, 9:59 PM EDT: Merged by the Graphite merge queue via draft PR: #2422.

## Changes

MasterPtato force-pushed the 04-23-fix_add_future_fdb_metrics branch 2 times, most recently from 9c38e32 to 19edb7d Compare April 24, 2025 22:42

MasterPtato marked this pull request as ready for review April 24, 2025 22:42

MasterPtato requested a review from NathanFlurry April 24, 2025 22:42

greptile-apps bot reviewed Apr 24, 2025

View reviewed changes

MasterPtato force-pushed the 04-23-fix_add_future_fdb_metrics branch from 19edb7d to 2ffbbd6 Compare April 29, 2025 22:25

This was referenced Apr 29, 2025

fix(pegboard): fix prewarm logic #2409

Closed

fix: use real errors for actor logs #2410

Closed

fix: add otel collector tail sampler #2411

Closed

This was referenced Apr 29, 2025

fix: add logs to ws #2412

Closed

fix: fix metrics #2416

Closed

fix: add udp test to system test #2417

Closed

fix: fix fdb cli, wf pulling #2418

Closed

fix: add future/fdb metrics

8cd9d0a

MasterPtato changed the base branch from main to graphite-base/2377 May 1, 2025 20:12

MasterPtato force-pushed the 04-23-fix_add_future_fdb_metrics branch from 2ffbbd6 to 8cd9d0a Compare May 1, 2025 20:12

MasterPtato changed the base branch from graphite-base/2377 to 04-30-fix_allow_custom_project_for_status_monitor May 1, 2025 20:12

This was referenced May 1, 2025

fix: allow custom project for status monitor #2415

Closed

chore: fix broadcast req error logs #2419

Closed

fix: remove iptables filter after CNI #2421

Open

graphite-app bot pushed a commit that referenced this pull request May 2, 2025

fix: add future/fdb metrics (#2377)

37250d5

 ## Changes

graphite-app bot closed this May 2, 2025

graphite-app bot deleted the 04-23-fix_add_future_fdb_metrics branch May 2, 2025 01:59

github-actions bot mentioned this pull request May 2, 2025

chore(main): release 25.4.2 #2423

Open

MasterPtato mentioned this pull request May 2, 2025

fix(pb): fix actor reschedule with wrong image #2428

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: add future/fdb metrics #2377

fix: add future/fdb metrics #2377

MasterPtato commented Apr 24, 2025

MasterPtato commented Apr 24, 2025 •

edited

Loading

cloudflare-workers-and-pages bot commented Apr 24, 2025 •

edited

Loading

greptile-apps bot left a comment

greptile-apps bot Apr 24, 2025

greptile-apps bot Apr 24, 2025

greptile-apps bot Apr 24, 2025

greptile-apps bot Apr 24, 2025

greptile-apps bot Apr 24, 2025

greptile-apps bot Apr 24, 2025

greptile-apps bot Apr 24, 2025

greptile-apps bot Apr 24, 2025

greptile-apps bot Apr 24, 2025

greptile-apps bot Apr 24, 2025

cloudflare-workers-and-pages bot commented Apr 29, 2025 •

edited

Loading

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

graphite-app bot commented May 2, 2025 •

edited

Loading

	.custom_instrument(tracing::info_span!("prewarm_fetch_tx"))
	.custom_instrument(tracing::info_span!("prewarm_fetch_tx"))
	.run(\|tx, _mc\| async move {

fix: add future/fdb metrics #2377

fix: add future/fdb metrics #2377

Conversation

MasterPtato commented Apr 24, 2025

Changes

MasterPtato commented Apr 24, 2025 • edited Loading

How to use the Graphite Merge Queue

cloudflare-workers-and-pages bot commented Apr 24, 2025 • edited Loading

Deploying rivet with Cloudflare Pages

greptile-apps bot left a comment

Choose a reason for hiding this comment

PR Summary

greptile-apps bot Apr 24, 2025

Choose a reason for hiding this comment

greptile-apps bot Apr 24, 2025

Choose a reason for hiding this comment

greptile-apps bot Apr 24, 2025

Choose a reason for hiding this comment

greptile-apps bot Apr 24, 2025

Choose a reason for hiding this comment

greptile-apps bot Apr 24, 2025

Choose a reason for hiding this comment

greptile-apps bot Apr 24, 2025

Choose a reason for hiding this comment

greptile-apps bot Apr 24, 2025

Choose a reason for hiding this comment

greptile-apps bot Apr 24, 2025

Choose a reason for hiding this comment

greptile-apps bot Apr 24, 2025

Choose a reason for hiding this comment

greptile-apps bot Apr 24, 2025

Choose a reason for hiding this comment

cloudflare-workers-and-pages bot commented Apr 29, 2025 • edited Loading

Deploying rivet-studio with Cloudflare Pages

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

Deploying rivet-hub with Cloudflare Pages

graphite-app bot commented May 2, 2025 • edited Loading

Merge activity

MasterPtato commented Apr 24, 2025 •

edited

Loading

cloudflare-workers-and-pages bot commented Apr 24, 2025 •

edited

Loading

cloudflare-workers-and-pages bot commented Apr 29, 2025 •

edited

Loading

graphite-app bot commented May 2, 2025 •

edited

Loading