feat: add upstream request/response logging for passthrough routes by pawbana · Pull Request #186 · coder/aibridge

pawbana · 2026-02-19T16:01:40Z

Added request/response dump functionality for passthrough routes.

Added RoundTripperMiddleware in the apidump package.
Updated the Provider interface to expose the APIDumpDir method

Drive by change

Improved error handling in the dumper code

pawbana · 2026-02-19T16:01:54Z

feat: add upstream request/response logging for passthrough routes #186 👈 (View in Graphite)
main

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

pawbana · 2026-02-20T11:26:42Z

intercept/apidump/apidump.go

+	if !bytes.HasSuffix(result, []byte("\n")) {
+		result = append(result, []byte("\n")...)
 	}
-	// Trim trailing newline added by pretty.Pretty.


Was there a reson for removing trailing new line?
Viewing dumps in terminal without it is annoying.

dannykopping · 2026-02-23T08:59:35Z

I'm thinking we should perhaps hold off on this change until #166 lands. If we have a session ID then we can correlate all intercepted and passthrough routes.

If you'd rather land this in the interim and then add session ID correlation later, that's fine too - in which case I'll give this a review.

pawbana · 2026-02-23T10:50:24Z

I'm thinking we should perhaps hold off on this change until #166 lands. If we have a session ID then we can correlate all intercepted and passthrough routes.

Will session ID be available in each request? Or only in requests from clients that set it?

dannykopping · 2026-02-23T11:09:27Z

I'm thinking we should perhaps hold off on this change until #166 lands. If we have a session ID then we can correlate all intercepted and passthrough routes.

Will session ID be available in each request? Or only in requests from clients that set it?

Only if clients set it

pawbana · 2026-02-23T11:23:37Z

I'm thinking we should perhaps hold off on this change until #166 lands. If we have a session ID then we can correlate all intercepted and passthrough routes.

Will session ID be available in each request? Or only in requests from clients that set it?

Only if clients set it

If session ID is not always set I'm not sure what do you mean by session ID correlation. Traffic dumping (both intercepted or passed though) will need to work without session ID.

I agree that dump file names are unfortunate for pass though routes (file names contain random UUIDs, not related to anything) but I wanted to keep the same format. Maybe different approach would be better.
Now that I think about it maybe it would be helpful to add this UUID to Passthrough trace and PassthroughCount metric? Although I'm not sure if it matches original intention behind interception ID.

ssncferreira

Nice addition 💪 this will be super nice for debugging. Just a few comments, but nothing blockin

ssncferreira · 2026-03-03T08:50:04Z

intercept/apidump/apidump.go

+// NewRoundTripperMiddleware returns http.RoundTripper that dumps requests and responses to files.
+// If baseDir is empty, returns the original transport unchanged.
+// Used for logging passed through requests.
+func NewRoundTripperMiddleware(transport http.RoundTripper, baseDir string, provider string, logger slog.Logger, clk quartz.Clock) http.RoundTripper {


nit: Since this is just for Passthrough routes, I would suggest renaming it:

Suggested change

func NewRoundTripperMiddleware(transport http.RoundTripper, baseDir string, provider string, logger slog.Logger, clk quartz.Clock) http.RoundTripper {

func NewPassthroughMiddleware(transport http.RoundTripper, baseDir string, provider string, logger slog.Logger, clk quartz.Clock) http.RoundTripper {

Similarly, rename NewMiddleware to NewBridgeMiddleware

ssncferreira · 2026-03-03T09:07:02Z

intercept/apidump/apidump.go

+// A random UUID is generated for the filename. "passthrough" is used as the directory name
+// in place of the model.
+func passthroughDumpPath(baseDir string, provider string, clk quartz.Clock) string {
+	return filepath.Join(baseDir, provider, "passthrough", fmt.Sprintf("%d-%s", clk.Now().UTC().UnixMilli(), uuid.New()))


Using a randomly generated UUID here doesn't seem very useful 🤔 What if we use the sanitized route instead (e.g., /v1/models → v1-models) to make it easier to identify requests in the dump files. In order to avoid collisions, we can also adda short UUID suffix.

ssncferreira · 2026-03-03T09:22:14Z

intercept/apidump/apidump.go

+	result := body
+	if json.Valid(body) {
+		result = pretty.Pretty(body)
+	}


Why did this change? Since result is body, if json.Valid returns false we'll be appending a newline to it, couldn't this impact the caller's slice?

ssncferreira · 2026-03-03T09:25:07Z

intercept/apidump/apidump.go

-	fmt.Fprintf(&buf, "\r\n")
+	_, err = fmt.Fprintf(&buf, "\r\n")
+	if err != nil {
+		return fmt.Errorf("write request body: %w", err)


nit: not really the body, right?

Suggested change

return fmt.Errorf("write request body: %w", err)

return fmt.Errorf("write header terminator: %w", err)

ssncferreira · 2026-03-03T09:25:27Z

intercept/apidump/apidump.go

+	}
+	_, err = fmt.Fprintf(&headerBuf, "\r\n")
+	if err != nil {
+		return fmt.Errorf("write response body: %w", err)


nit: same as above

Suggested change

return fmt.Errorf("write response body: %w", err)

return fmt.Errorf("write header terminator: %w", err)

ssncferreira · 2026-03-03T09:27:01Z

intercept/apidump/apidump.go

-				fmt.Fprintf(w, "%s: %s\r\n", key, override)
+				_, err := fmt.Fprintf(w, "%s: %s\r\n", key, override)
+				if err != nil {
+					return fmt.Errorf("write response header override: %w", err)


This function is used for both request/responses, right? Consider removing response from here, to avoid confusion. Same below 👀

dannykopping

LGTM, thanks!

As discussed, we'll leave session indexing for later.

intercept/apidump/apidump.go

pawbana marked this pull request as ready for review February 19, 2026 16:21

pawbana force-pushed the pb/dump-passthrough branch from 5e86692 to 63d2dcb Compare February 19, 2026 16:25

pawbana requested a review from dannykopping February 19, 2026 16:35

pawbana commented Feb 20, 2026

View reviewed changes

pawbana requested a review from ssncferreira March 2, 2026 14:39

ssncferreira reviewed Mar 3, 2026

View reviewed changes

dannykopping approved these changes Mar 3, 2026

View reviewed changes

intercept/apidump/apidump.go Show resolved Hide resolved

intercept/apidump/apidump.go Show resolved Hide resolved

pawbana added 5 commits March 3, 2026 11:59

feat: add upstream request/response logging for passthrough routes

32e23b1

drive by fix: prettyPrintJSON non json edge case + new line

991d4a9

added missing error handling

1a03d34

Made NewRoundTripperMiddleware reusable

7b47019

review 1: pass though dump file name includes url path + renaming

153f439

pawbana force-pushed the pb/dump-passthrough branch from 36705e3 to 153f439 Compare March 3, 2026 13:26

pawbana merged commit b84b1c3 into main Mar 4, 2026
5 checks passed

pawbana linked an issue Mar 4, 2026 that may be closed by this pull request

Dump passthrough requests/responses #169

Closed

pawbana mentioned this pull request Mar 4, 2026

Dump passthrough requests/responses #169

Closed

	func NewRoundTripperMiddleware(transport http.RoundTripper, baseDir string, provider string, logger slog.Logger, clk quartz.Clock) http.RoundTripper {
	func NewPassthroughMiddleware(transport http.RoundTripper, baseDir string, provider string, logger slog.Logger, clk quartz.Clock) http.RoundTripper {

	return fmt.Errorf("write request body: %w", err)
	return fmt.Errorf("write header terminator: %w", err)

	return fmt.Errorf("write response body: %w", err)
	return fmt.Errorf("write header terminator: %w", err)

Conversation

pawbana commented Feb 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pawbana commented Feb 19, 2026

Uh oh!

pawbana Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

dannykopping commented Feb 23, 2026

Uh oh!

pawbana commented Feb 23, 2026

Uh oh!

dannykopping commented Feb 23, 2026

Uh oh!

pawbana commented Feb 23, 2026

Uh oh!

ssncferreira left a comment

Choose a reason for hiding this comment

Uh oh!

ssncferreira Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

ssncferreira Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

ssncferreira Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

ssncferreira Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

ssncferreira Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

ssncferreira Mar 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dannykopping left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

pawbana commented Feb 19, 2026 •

edited

Loading

ssncferreira Mar 3, 2026 •

edited

Loading