Skip to content

feat(cogs): objectstore.cogs.usage datadog metric#492

Merged
matt-codecov merged 2 commits into
mainfrom
matt/objectstore-cogs-metric
Jun 9, 2026
Merged

feat(cogs): objectstore.cogs.usage datadog metric#492
matt-codecov merged 2 commits into
mainfrom
matt/objectstore-cogs-metric

Conversation

@matt-codecov

Copy link
Copy Markdown
Contributor

imitating how relay does compute COGS, except we are using request counts rather than request timings because we're network-bound and CPU-light

every request incrs the counter by 1, except batch requests incr it for each operation in the batch

need to update usage-accountant config to consume the counter but then compute COGS is finished

Ref FS-195

@matt-codecov matt-codecov requested a review from a team as a code owner June 5, 2026 23:59
@linear-code

linear-code Bot commented Jun 5, 2026

Copy link
Copy Markdown

FS-195

@codecov

This comment has been minimized.

Comment thread objectstore-server/src/endpoints/batch.rs Outdated
Comment thread objectstore-server/src/web/middleware.rs Outdated
Comment thread objectstore-server/src/cogs.rs Outdated

@lcian lcian left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please address comment about single vs batch and rejected vs accepted requests.

@matt-codecov matt-codecov force-pushed the matt/objectstore-cogs-metric branch from bdefa80 to dcbd9ed Compare June 8, 2026 19:46
Comment thread objectstore-server/src/endpoints/batch.rs Outdated
@matt-codecov matt-codecov force-pushed the matt/objectstore-cogs-metric branch from dcbd9ed to de841d6 Compare June 9, 2026 00:58
@matt-codecov matt-codecov requested a review from lcian June 9, 2026 00:58
@matt-codecov matt-codecov force-pushed the matt/objectstore-cogs-metric branch 2 times, most recently from dcf127c to a933c60 Compare June 9, 2026 01:02
Comment on lines +58 to +70
/// [`CountingBackend`]'s implementation clashes with how the [`MultipartUploadBackend`] trait is
/// connected to the [`Backend`] trait. The workaround is to give `CountingBackend` (up to) two
/// `Arc`s that point to the inner backend:
/// - `inner: Arc<dyn Backend>`
/// - `inner_multipart: Option<Arc<dyn MultipartUploadBackend>>` if `inner` supports it
///
/// [`CountingBackend`]'s implementation clashes with how the [`MultipartUploadBackend`] trait is
/// connected to the [`Backend`] trait: [`CountingBackend::as_multipart_upload_backend`] can't apply
/// the cast to `self.inner` which remains an `Arc<dyn Backend>`. There are two workarounds:
/// - (Rejected) Every multipart method must call `self.inner.clone().as_multipart_upload_backend()?`
/// which clones every time
/// - (Chosen) `CountingBackend` must call `self.inner.clone().as_multipart_upload_backend().ok()`
/// and hang onto it

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is a different way to work around the problem with a minor disadvantage, but IMO preferable to this approach: #494

@lcian lcian left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It seems sensible we do it by wrapping the Service, so that we count every operation that goes through there.
I would prefer if we work around the multipart thing using #494 instead of holding two Backends.

@lcian lcian force-pushed the matt/objectstore-cogs-metric branch from a933c60 to 98f3aa7 Compare June 9, 2026 09:09

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 98f3aa7. Configure here.

Comment thread objectstore-service/src/backend/counting.rs
@matt-codecov matt-codecov force-pushed the matt/objectstore-cogs-metric branch from 98f3aa7 to 2d61ae5 Compare June 9, 2026 17:12
@matt-codecov matt-codecov merged commit 1d66810 into main Jun 9, 2026
25 checks passed
@matt-codecov matt-codecov deleted the matt/objectstore-cogs-metric branch June 9, 2026 17:17
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.

2 participants