observability: per-route response-bytes histogram#1210
Conversation
istio byte metrics carry no path label and HyperDX spans report response size 0, so per-route egress is currently unobservable. Add an http_response_bytes histogram labeled by http.route, tallied via a pass-through stream on the response body (responses are chunked, so content-length is absent).
Tagging OptionsShould a new tag be published when this PR is merged?
|
📝 WalkthroughWalkthroughAdds per-route HTTP response body size observability. A new ChangesHTTP Response Byte Observability
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@observability/http.ts`:
- Around line 10-14: The httpResponseBytes histogram uses default OpenTelemetry
bucket boundaries that are designed for small-value metrics like latency,
causing all byte-scale observations to fall into the +Inf bucket. To fix this,
create a View with byte-appropriate bucket boundaries (such as 100, 1000, 10000,
100000, 1000000, 10000000) that matches the "By" unit. Reference the existing
View patterns in metrics.ts to implement this View for the "http_response_bytes"
metric, ensuring the boundaries align with typical HTTP response body sizes
ranging from hundreds of bytes to megabytes.
In `@runtime/middleware.ts`:
- Around line 312-320: The flush() callback in the TransformStream counter can
fail if recordResponseBytes throws an error, which will cause the transform
stream to error and truncate the client response. Wrap the recordResponseBytes
call inside the flush() method in a try-catch block to catch and swallow any
errors, ensuring that recording failures cannot interrupt the response delivery
to the client.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 930775dd-d882-4867-834c-ec9433f66d36
📒 Files selected for processing (2)
observability/http.tsruntime/middleware.ts
| const httpResponseBytes = meter.createHistogram("http_response_bytes", { | ||
| description: "http response body size in bytes (per route)", | ||
| unit: "By", | ||
| valueType: ValueType.INT, | ||
| }); |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major
🧩 Analysis chain
🌐 Web query:
What are the default explicit bucket boundaries for an OpenTelemetry histogram instrument, and how do you override them via a View?
💡 Result:
The default explicit bucket boundaries for an OpenTelemetry Histogram instrument, as defined in the OpenTelemetry specification, are [1][2] [1]. These boundaries are designed to be cumulative and are meant to handle a broad range of latency values [3][4][1]. To override these default boundaries, you must use an OpenTelemetry View [5][2]. A View allows you to customize the aggregation applied to specific metric instruments by matching them based on criteria such as instrument name or type [6][5][2]. The process generally involves the following steps: 1. Define a View that targets the specific Histogram instrument (e.g., using an InstrumentSelector based on the instrument's name or type) [5][2]. 2. Configure the View with an ExplicitBucketHistogramAggregation that specifies your custom bucket boundaries [6][5][2]. 3. Register this View with your MeterProvider during the SDK initialization [7][5][2]. For example, in Python, you can configure a View as follows: meter_provider = MeterProvider( metric_readers=[reader], views=[ View( instrument_type=metrics.Histogram, instrument_name="your_histogram_name", aggregation=ExplicitBucketHistogramAggregation( boundaries=[2]),) ],) [7] When using a View, ensure the custom bucket boundaries are in the same units as the recorded measurement values (e.g., if recording seconds, use boundaries in seconds) [5][2].
Citations:
- 1: Default bucket boundaries for explicit bucket histogram are different from specification open-telemetry/opentelemetry-java#4665
- 2: https://typelevel.org/otel4s/how-to-metrics/customize-histogram-buckets.html
- 3: Change the default buckets for Explicit Bucket Histogram open-telemetry/opentelemetry-specification#2770
- 4: Default boundaries in explicit buckets histograms are not optmized for Latency open-telemetry/opentelemetry-specification#2759
- 5: https://typelevel.org/otel4s/explanations/histogram-bucket-customization-with-views.html
- 6: https://opentelemetry-python.readthedocs.io/en/stable/sdk/metrics.view.html
- 7: Is it possible to set the histogram boundaries? open-telemetry/opentelemetry-python#3926
🏁 Script executed:
# Check the codebase to understand the OTEL setup and see if there's any existing histogram configuration
fd -t f 'http\.ts' --exec head -50 {} \;Repository: deco-cx/deco
Length of output: 2993
🏁 Script executed:
# Check the OTEL metrics configuration in the codebase
fd -t f 'metrics.ts' --exec cat -n {} \;Repository: deco-cx/deco
Length of output: 2449
🏁 Script executed:
# Check the deps.ts to see which OTEL version is being used
fd -t f 'deps.ts' | head -5 | xargs grep -A 5 "ExplicitBucketHistogramAggregation\|otel"Repository: deco-cx/deco
Length of output: 343
Add a View with byte-appropriate bucket boundaries for http_response_bytes.
The default OpenTelemetry explicit-bucket histogram boundaries are [0.005, 0.01, 0.025, ..., 10000] and are designed for latency/small-count scales. Response bodies typically range from hundreds of bytes to megabytes, so nearly all observations will fall into the +Inf bucket, making percentile and distribution analysis unusable. The _sum-based egress query works regardless, but distribution queries won't.
The codebase already uses Views for custom boundaries (see metrics.ts). Add a View for the "By" unit with byte-appropriate boundaries—for example, [100, 1000, 10000, 100000, 1000000, 10000000]—so distribution queries become meaningful.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@observability/http.ts` around lines 10 - 14, The httpResponseBytes histogram
uses default OpenTelemetry bucket boundaries that are designed for small-value
metrics like latency, causing all byte-scale observations to fall into the +Inf
bucket. To fix this, create a View with byte-appropriate bucket boundaries (such
as 100, 1000, 10000, 100000, 1000000, 10000000) that matches the "By" unit.
Reference the existing View patterns in metrics.ts to implement this View for
the "http_response_bytes" metric, ensuring the boundaries align with typical
HTTP response body sizes ranging from hundreds of bytes to megabytes.
| const counter = new TransformStream<Uint8Array, Uint8Array>({ | ||
| transform(chunk, controller) { | ||
| bytes += chunk.byteLength; | ||
| controller.enqueue(chunk); | ||
| }, | ||
| flush() { | ||
| recordResponseBytes(bytes, method, pathTemplate, res.status); | ||
| }, | ||
| }); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Guard flush() so a recording failure can't truncate the client response.
recordResponseBytes runs inside the TransformStream.flush() callback on the response hot path. If it throws (meter/exporter error, etc.), the transform stream errors, which propagates to the readable side and the client receives a truncated/aborted body. Observability must never be able to break response delivery — wrap the record call so failures are swallowed.
Separately, note the undercount documented in the PR: flush() only runs on normal stream completion, so client aborts mid-stream silently drop the byte tally. That's an accepted limitation, but worth confirming there's no expectation of accuracy on those responses.
🛡️ Proposed fix: isolate the recording from the stream
flush() {
- recordResponseBytes(bytes, method, pathTemplate, res.status);
+ try {
+ recordResponseBytes(bytes, method, pathTemplate, res.status);
+ } catch (_e) {
+ // never let observability break response delivery
+ }
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const counter = new TransformStream<Uint8Array, Uint8Array>({ | |
| transform(chunk, controller) { | |
| bytes += chunk.byteLength; | |
| controller.enqueue(chunk); | |
| }, | |
| flush() { | |
| recordResponseBytes(bytes, method, pathTemplate, res.status); | |
| }, | |
| }); | |
| const counter = new TransformStream<Uint8Array, Uint8Array>({ | |
| transform(chunk, controller) { | |
| bytes += chunk.byteLength; | |
| controller.enqueue(chunk); | |
| }, | |
| flush() { | |
| try { | |
| recordResponseBytes(bytes, method, pathTemplate, res.status); | |
| } catch (_e) { | |
| // never let observability break response delivery | |
| } | |
| }, | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@runtime/middleware.ts` around lines 312 - 320, The flush() callback in the
TransformStream counter can fail if recordResponseBytes throws an error, which
will cause the transform stream to error and truncate the client response. Wrap
the recordResponseBytes call inside the flush() method in a try-catch block to
catch and swallow any errors, ensuring that recording failures cannot interrupt
the response delivery to the client.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 2 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="observability/http.ts">
<violation number="1" location="observability/http.ts:10">
P2: The default histogram bucket boundaries (`[0, 5, 10, 25, 50, ..., 10000]`) are designed for latency/count scales and are inappropriate for byte measurements. Response bodies typically range from hundreds of bytes to megabytes, so nearly all observations will land in the `+Inf` bucket, making distribution and percentile queries useless. Register a View with byte-appropriate boundaries (e.g., `[100, 1_000, 10_000, 100_000, 1_000_000, 10_000_000]`) — the codebase already does this for other histograms in `metrics.ts`. The `_sum`-based egress query still works, but distribution analysis is broken without this.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| valueType: ValueType.DOUBLE, | ||
| }); | ||
|
|
||
| const httpResponseBytes = meter.createHistogram("http_response_bytes", { |
There was a problem hiding this comment.
P2: The default histogram bucket boundaries ([0, 5, 10, 25, 50, ..., 10000]) are designed for latency/count scales and are inappropriate for byte measurements. Response bodies typically range from hundreds of bytes to megabytes, so nearly all observations will land in the +Inf bucket, making distribution and percentile queries useless. Register a View with byte-appropriate boundaries (e.g., [100, 1_000, 10_000, 100_000, 1_000_000, 10_000_000]) — the codebase already does this for other histograms in metrics.ts. The _sum-based egress query still works, but distribution analysis is broken without this.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At observability/http.ts, line 10:
<comment>The default histogram bucket boundaries (`[0, 5, 10, 25, 50, ..., 10000]`) are designed for latency/count scales and are inappropriate for byte measurements. Response bodies typically range from hundreds of bytes to megabytes, so nearly all observations will land in the `+Inf` bucket, making distribution and percentile queries useless. Register a View with byte-appropriate boundaries (e.g., `[100, 1_000, 10_000, 100_000, 1_000_000, 10_000_000]`) — the codebase already does this for other histograms in `metrics.ts`. The `_sum`-based egress query still works, but distribution analysis is broken without this.</comment>
<file context>
@@ -6,6 +6,30 @@ const httpDuration = meter.createHistogram("http_request_duration", {
valueType: ValueType.DOUBLE,
});
+
+const httpResponseBytes = meter.createHistogram("http_response_bytes", {
+ description: "http response body size in bytes (per route)",
+ unit: "By",
</file context>
|
Closing in favor of a site-local approach. Per discussion: this is fleet-wide hot-path instrumentation that every site would only get via a version bump (dragging in all other engine changes), which is too heavy a vehicle just to attribute one site's egress. Moving the byte counter into the site's own |
1 participant
Why
Per-route egress is currently unobservable. istio byte metrics carry no path label, and HyperDX spans report response size
0. So when a site's egress dominates its cost (e.g. FARM Rio: ~13 TiB/mo, ~40% of the cluster's total egress, ~4× its compute), there's no way to attribute the bytes to a route and decide what to trim.What
Adds an
http_response_byteshistogram labeled byhttp.route(low cardinality — it'spathTemplate, not the full URL), sitting next to the existinghttp_request_durationhistogram and flowing through the same OTEL meter/exporter.Responses are
transfer-encoding: chunked, socontent-lengthis absent — bytes are tallied through a pass-throughTransformStreamon the body and recorded onflush()(after the body finishes streaming). Scoped to requests that already resolved apathTemplate.Unlocks:
Tested
ctx.resreassignment in the observability middleware'sfinally{}propagates the wrapped body through the nested middleware onion (verified against the same Hono 4.x major).deno checkintroduces no new type errors.Caveats
SHOULD_USE_ASYNC_RENDER_SINGLE_FLIGHT) if preferred.🤖 Generated with Claude Code
Summary by cubic
Add an
http_response_byteshistogram labeled byhttp.routeto make per-route egress observable. Bytes are counted for streamed responses and recorded after the body finishes.New Features
http_response_byteshistogram (unit: bytes) withhttp.method,http.route, andhttp.response.statuslabels.pathTemplateis resolved to keep label cardinality low.http_request_duration.Caveats
Written for commit 10681fc. Summary will update on new commits.
Summary by CodeRabbit