Skip to content

feat: log streaming body#5625

Merged
Flo4604 merged 1 commit intomainfrom
04-07-feat_log_streaming_body
Apr 13, 2026
Merged

feat: log streaming body#5625
Flo4604 merged 1 commit intomainfrom
04-07-feat_log_streaming_body

Conversation

@Flo4604
Copy link
Copy Markdown
Member

@Flo4604 Flo4604 commented Apr 7, 2026 •
edited
Loading

What does this PR do?

Adds streaming request/response body capture for logging in the proxy handlers.

Previously, streaming requests (gRPC, Connect) had their bodies skipped entirely to avoid buffering issues. This change uses TeeReader with a LimitedWriter to capture up to 1 MiB of streaming body content for logging purposes while allowing the stream to flow through uninterrupted.

The implementation includes:

  • A new LimitedWriter that caps captured bytes at a configurable limit (1 MiB) and silently discards excess data
  • Modified proxy handlers in both frontline and sentinel to use TeeReader for streaming body capture
  • A new SetResponseBody method on Session to allow proxy handlers to feed captured response bodies back for middleware logging
  • Exported IsStreamingContentType function for reuse across services
  • Added seeder methods for creating regions and instances in integration tests

Fixes # (issue)

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • Chore (refactoring code, technical debt, workflow improvements)
  • Enhancement (small improvements)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How should this be tested?

  • Run the new TestLimitedWriter_* tests to verify the limited writer behavior
  • Run the new TestHandler_StreamingBodyCapture test to verify streaming request/response body capture
  • Run the TestHandler_ServerStreamBodyCapture test to verify server-only streaming scenarios
  • Run the TestHandler_StreamingBodyCapture_RespectsLimit test to verify the 1 MiB capture limit is enforced
  • Test with actual gRPC/Connect streaming requests to ensure bodies are captured in logs without affecting stream performance

Checklist

Required

  • Filled out the "How to test" section in this PR
  • Read Contributing Guide
  • Self-reviewed my own code
  • Commented on my code in hard-to-understand areas
  • Ran pnpm build
  • Ran pnpm fmt
  • Ran make fmt on /go directory
  • Checked for warnings, there are none
  • Removed all console.logs
  • Merged the latest changes from main onto my branch with git pull origin main
  • My changes don't cause any responsiveness issues

Appreciated

  • If a UI change was made: Added a screen recording or screenshots to this PR
  • Updated the Unkey Docs if changes were necessary

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 7, 2026 •
edited
Loading

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 0cf791c6-7b4e-4f57-84e7-fde158a62d0f

📥 Commits

Reviewing files that changed from the base of the PR and between 96c4b62 and d99dc35.

📒 Files selected for processing (8)
  • pkg/zen/BUILD.bazel
  • pkg/zen/limited_writer_test.go
  • pkg/zen/session.go
  • svc/ctrl/integration/seed/seed.go
  • svc/frontline/services/proxy/forward.go
  • svc/sentinel/routes/proxy/BUILD.bazel
  • svc/sentinel/routes/proxy/handler.go
  • svc/sentinel/routes/proxy/handler_test.go
📝 Walkthrough

Walkthrough

The changes implement body capture functionality for streaming proxies by introducing a LimitedWriter utility to enforce a 1 MiB capture limit, updating proxy handlers to capture request/response bodies during streaming, adding test seeding helpers for database records, and exporting previously internal helper functions.

Changes

Cohort / File(s) Summary
Limited Writer Utility
pkg/zen/BUILD.bazel, pkg/zen/session.go, pkg/zen/limited_writer_test.go		 Added LimitedWriter type to cap body capture at 1 MiB; introduced MaxBodyCapture constant and SetResponseBody() method on Session; renamed isStreamingContentType to exported IsStreamingContentType; added four unit tests for LimitedWriter write behavior under various conditions.
Seed Helpers
svc/ctrl/integration/seed/seed.go		 Added CreateRegionRequest and CreateInstanceRequest structs alongside CreateRegion() and CreateInstance() methods on Seeder for creating test database records with auto-generated IDs and default field values.
Frontline Proxy Integration
svc/frontline/services/proxy/forward.go		 Modified reverse proxy response handler to capture response bodies via io.TeeReader and LimitedWriter, then pass captured data to session via SetResponseBody() for downstream logging.
Sentinel Proxy Handler
svc/sentinel/routes/proxy/handler.go		 Reworked request/response body capture to use io.TeeReader with LimitedWriter for streaming bodies; changed response timing to capture time-to-first-byte instead of full duration; deferred instance timing and response body assignment until after proxy completion.
Sentinel Proxy Tests
svc/sentinel/routes/proxy/BUILD.bazel, svc/sentinel/routes/proxy/handler_test.go		 Added new test file with helpers to construct fully wired Handler backed by MySQL; added three test cases validating request/response body capture, streaming content handling, and the 1 MiB capture limit; extended test target dependencies.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant Handler as Proxy Handler
    participant TeeReader
    participant LimitedWriter
    participant Buffer as Response Buffer
    participant Upstream
    participant Session
    
    Client->>Handler: HTTP Request (streaming)
    Handler->>Upstream: Forward request
    Upstream-->>Handler: Response stream
    Handler->>TeeReader: Wrap response body
    TeeReader->>Handler: Stream bytes to client
    TeeReader->>LimitedWriter: Tee bytes for capture
    LimitedWriter->>Buffer: Write (up to 1 MiB limit)
    Buffer-->>LimitedWriter: Confirm write
    LimitedWriter-->>TeeReader: Continue passthrough
    Handler->>Session: SetResponseBody(captured bytes)
    Handler-->>Client: Complete response
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch 04-07-feat_log_streaming_body

Comment @coderabbitai help to get the list of available commands and usage tips.

This was referenced Apr 7, 2026
Merged
Merged
@Flo4604 Graphite App
Copy link
Copy Markdown
Member Author

Flo4604 commented Apr 7, 2026 •
edited
Loading

This stack of pull requests is managed by Graphite. Learn more about stacking.

@Flo4604 Flo4604 mentioned this pull request Apr 7, 2026
Merged
19 tasks
@vercel
Copy link
Copy Markdown

vercel Bot commented Apr 7, 2026 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
dashboard Ready Ready Preview, Comment Apr 13, 2026 0:39am

Request Review

@Flo4604 Flo4604 force-pushed the 04-07-feat_log_streaming_body branch from 24214fd to b7a6a02 Compare April 7, 2026 16:02
@Flo4604 Flo4604 changed the base branch from 04-03-feat_upstream_protocol to graphite-base/5625 April 7, 2026 18:43
@Flo4604 Flo4604 force-pushed the graphite-base/5625 branch from adf10b1 to 95239d2 Compare April 8, 2026 10:45
@Flo4604 Flo4604 force-pushed the 04-07-feat_log_streaming_body branch from b7a6a02 to 48a45ff Compare April 8, 2026 10:45
@Flo4604 Flo4604 changed the base branch from graphite-base/5625 to 04-03-feat_upstream_protocol April 8, 2026 10:45
@Flo4604 Flo4604 force-pushed the 04-07-feat_log_streaming_body branch from 48a45ff to 84d845e Compare April 10, 2026 12:20
@Flo4604 Flo4604 force-pushed the 04-03-feat_upstream_protocol branch from 95239d2 to cc4c976 Compare April 10, 2026 12:20
@Flo4604 Flo4604 force-pushed the 04-03-feat_upstream_protocol branch from cc4c976 to deeb119 Compare April 10, 2026 12:32
@Flo4604 Flo4604 force-pushed the 04-07-feat_log_streaming_body branch from 84d845e to b017f28 Compare April 10, 2026 12:32
@Flo4604 Flo4604 force-pushed the 04-07-feat_log_streaming_body branch from b017f28 to 1b7941a Compare April 10, 2026 14:24
@Flo4604 Flo4604 force-pushed the 04-07-feat_log_streaming_body branch from 1b7941a to 829c80d Compare April 10, 2026 14:39
@Flo4604 Flo4604 marked this pull request as ready for review April 10, 2026 17:20
@Flo4604 Flo4604 force-pushed the 04-07-feat_log_streaming_body branch from 786e201 to 2c8fd2b Compare April 11, 2026 16:47
@chronark
Copy link
Copy Markdown
Collaborator

chronark commented Apr 13, 2026

hmmm, I'm not sure if we want this tbh. We still need to find a good way to let the user configure logging and especially masking and streaming can produce really really large bodies, which we probably don't want in clickhouse

what was the motivation here? the PR description is shit

chronark
chronark requested changes Apr 13, 2026
View reviewed changes
Copy link
Copy Markdown
Collaborator

@chronark chronark left a comment

Choose a reason for hiding this comment

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

needs more context

@Flo4604
Copy link
Copy Markdown
Member Author

Flo4604 commented Apr 13, 2026

hmmm, I'm not sure if we want this tbh. We still need to find a good way to let the user configure logging and especially masking and streaming can produce really really large bodies, which we probably don't want in clickhouse

what was the motivation here? the PR description is shit

That we can actually support streaming, otherwise zen / the logging tries to read the fully body and locks up til the end of response

Which buffers the whole stream and we only return it once the stream gets closed

@Flo4604 Flo4604 requested a review from chronark April 13, 2026 10:50
@Flo4604 Flo4604 force-pushed the 04-03-feat_upstream_protocol_frontend branch from 0099993 to f565e5c Compare April 13, 2026 11:13
@Flo4604 Flo4604 force-pushed the 04-07-feat_log_streaming_body branch from 2c8fd2b to 7682b11 Compare April 13, 2026 11:13
ogzhanolguncu
ogzhanolguncu reviewed Apr 13, 2026
View reviewed changes
Comment thread pkg/zen/session.go
@Flo4604 Flo4604 force-pushed the 04-07-feat_log_streaming_body branch from 7682b11 to 5d3b32e Compare April 13, 2026 11:49
@Flo4604 Flo4604 force-pushed the 04-03-feat_upstream_protocol_frontend branch from f565e5c to a3139bc Compare April 13, 2026 11:49
@Flo4604 Flo4604 changed the base branch from 04-03-feat_upstream_protocol_frontend to graphite-base/5625 April 13, 2026 12:05
@Flo4604 Graphite App
Copy link
Copy Markdown
Member Author

Flo4604 commented Apr 13, 2026 •
edited
Loading

Merge activity

@Flo4604 Flo4604 force-pushed the 04-07-feat_log_streaming_body branch from 5d3b32e to 5070c7f Compare April 13, 2026 12:26
@Flo4604 Flo4604 force-pushed the graphite-base/5625 branch from a3139bc to 5c2ca7c Compare April 13, 2026 12:26
@Flo4604 Flo4604 changed the base branch from graphite-base/5625 to 04-03-feat_upstream_protocol_frontend April 13, 2026 12:26
Base automatically changed from 04-03-feat_upstream_protocol_frontend to main April 13, 2026 12:36
@Flo4604 Flo4604 force-pushed the 04-07-feat_log_streaming_body branch from 5070c7f to d99dc35 Compare April 13, 2026 12:37
@Flo4604 Flo4604 enabled auto-merge April 13, 2026 12:39
@Flo4604 Flo4604 added this pull request to the merge queue Apr 13, 2026
Merged via the queue into main with commit c183955 Apr 13, 2026
10 of 12 checks passed
@Flo4604 Flo4604 deleted the 04-07-feat_log_streaming_body branch April 13, 2026 12:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@chronark chronark chronark approved these changes

@ogzhanolguncu ogzhanolguncu ogzhanolguncu approved these changes

@perkinsjr perkinsjr Awaiting requested review from perkinsjr

@mcstepp mcstepp Awaiting requested review from mcstepp

@MichaelUnkey MichaelUnkey Awaiting requested review from MichaelUnkey

@imeyer imeyer Awaiting requested review from imeyer

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Flo4604 @chronark @ogzhanolguncu