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

Request TTFB metric for GetObject API #8556

Merged
merged 1 commit into from
Jan 27, 2025
Merged

Conversation

nopcoder
Copy link
Contributor

Closes #8503

Change Description

Background

Metric that will help us measure the GetObject API call's time during a request that does not involve the user's object content. This metric, GetObject Time to First Byte (TTFB), is crucial for understanding the initial latency of fetching an object and differentiating it from subsequent data transfer times.

New Feature

This Pull Request introduces a new metric: api_request_ttfb_duration_seconds. It measures the time elapsed from when a GetObject request is initiated until the first byte of data is sent back to the client.

Testing Details

The changes were tested by:

  1. Making numerous GetObject requests
  2. Analyzing /metrics to verify that the new TTFB metric is accurately recorded

Breaking Change?

No, this change does not break any existing functionality (API, CLI, Clients). The new metric is added without modifying any existing endpoints or behaviors.

@nopcoder nopcoder requested a review from arielshaqed January 27, 2025 13:48
@nopcoder nopcoder self-assigned this Jan 27, 2025
@nopcoder nopcoder added area/API Improvements or additions to the API include-changelog PR description should be included in next release changelog labels Jan 27, 2025
Copy link

E2E Test Results - Quickstart

11 passed

Copy link

E2E Test Results - DynamoDB Local - Local Block Adapter

13 passed

Copy link
Contributor

@arielshaqed arielshaqed left a comment

Choose a reason for hiding this comment

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

Neat, thanks! Great first issue 🤡 . Looking forward to seeing the metrics.

@@ -4564,6 +4565,11 @@ func (c *Controller) GetObject(w http.ResponseWriter, r *http.Request, repositor
w.Header().Set("Content-Length", fmt.Sprint(entry.Size))
}

// time to first byte - include out part of the processing without the actual data transfer
requestTTFBHistograms.
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a bit unfortunate: we will get GetObject TTFB on a different metric, possibly with different fields. But I also understand that if we report it on the same metric then summing that metric will be count GetObject TTFB twice - also unfortunate.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

True. Note that summing the underlaying block adapters ttfb can include other operations that under the hood perform GetObject that uses the same block adapter.

@nopcoder nopcoder merged commit f2b9264 into master Jan 27, 2025
40 of 42 checks passed
@nopcoder nopcoder deleted the feat/get-obj-api-ttfb branch January 27, 2025 14:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/API Improvements or additions to the API include-changelog PR description should be included in next release changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Report metric for GetObject TTFB
2 participants