Skip to content

feat(lfs): record handler outcomes and honest byte metrics - #57

Merged
yordis merged 1 commit into
mainfrom
yordis/feat-lfs-handler-metrics
Oct 31, 2026
Merged

yordis merged 1 commit into
mainfrom
yordis/feat-lfs-handler-metrics

Conversation

@yordis

@yordis yordis commented Oct 31, 2026

Copy link
Copy Markdown
Member

@cursor

cursor Bot commented Oct 31, 2026 •

Copy link
Copy Markdown

PR Summary

Low Risk
Observability-only refactor with no auth or storage logic changes; minor overhead from metric recording on the LFS hot path.

Overview
Adds Prometheus-style metrics around the LFS batch, get, put, and verify handlers without changing transfer behavior. Each public handler now wraps the existing logic in an *_inner function, records walgit_lfs_handler_seconds{op} and walgit_lfs_requests_total{op,result} (mapping HTTP and ApiError outcomes to ok, not_found, or error), and returns the same response as before.

GET responses additionally feed walgit_lfs_response_bytes_offered_total from Content-Length on successful GET/206 only—explicitly skipping HEAD, errors, 304, and x-accel-redirect edge offload so dashboards reflect offered body size, not completed downloads. PUT increments walgit_lfs_upload_bytes_validated_total and walgit_lfs_upload_object_bytes after the upload body is fully read and SHA-256 checks pass, even if the subsequent store write fails.

docs/LFS.md documents what these series mean (handler completion vs streaming, batch per-object errors in 200 responses). A unit test locks in the download-offer filters and error outcome labeling.

Reviewed by Cursor Bugbot for commit 1c9c775. Bugbot is set up for automated code reviews on this repo. Configure here.

@coderabbitai

coderabbitai Bot commented Oct 31, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 59 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: b690c1c2-1e8c-4d94-988b-9b2ac09dd85f
Commits

Reviewing files that changed from the base of the PR and between 7ea667a and 1c9c775.

Files selected for processing: 2
  • crates/walgit-server/src/lfs.rs
  • docs/LFS.md
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

@yordis yordis closed this Oct 31, 2026
@yordis yordis reopened this Oct 31, 2026
Adapt PR #51 with explicit offered-download and validated-upload counters. Exclude HEAD/304/error/offloaded responses; count HTTP errors even when returned as Ok(Response). A recorder regression covers range and dropped-response semantics.

(cherry picked from commit e21a078)
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
@yordis
yordis force-pushed the yordis/feat-lfs-handler-metrics branch from 6d26fde to 1c9c775 Compare October 31, 2026 16:41
@yordis
yordis merged commit bc97170 into main Oct 31, 2026
7 checks passed
@yordis
yordis deleted the yordis/feat-lfs-handler-metrics branch October 31, 2026 16:52
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