feat: req/resp logging middleware #105
Conversation
dannykopping
force-pushed
the
dk/request-logging-redux
branch
from
beb1aca to
d021c47
Compare
|
|
||
| resp, err := next(req) | ||
| if err != nil { | ||
| return resp, err |
There was a problem hiding this comment.
Not sure about it but maybe it be useful to have "err" file with error?
There was a problem hiding this comment.
Let's leave this for a future enhancement, ok?
There was a problem hiding this comment.
sure, could you please create an issue about it?
bridge_integration_test.go
Outdated
| reqDumpData, err := os.ReadFile(reqDumpFile) | ||
| require.NoError(t, err, "request dump file should exist") | ||
| require.Contains(t, string(reqDumpData), "POST ") | ||
| require.Contains(t, string(reqDumpData), "Host:") |
There was a problem hiding this comment.
maybe add check like require.Contains(t, string(reqDumpData), string(reqBody))
and similar for response
There was a problem hiding this comment.
This forced me to confront the fact that map keys were not stably sorted. Thanks 👍
This stack of pull requests is managed by Graphite. Learn more about stacking. |
dannykopping
force-pushed
the
dk/request-logging-redux
branch
from
ec13727 to
db0f11e
Compare
|
|
||
| for _, key := range headerKeys { | ||
| // Skip Content-Length since we write it explicitly above with the pretty-printed body length. | ||
| if key == "Content-Length" { |
There was a problem hiding this comment.
nit: Host could also be skipped
| // Sort header keys for deterministic output. | ||
| headerKeys := make([]string, 0, len(resp.Header)) | ||
| for key := range resp.Header { | ||
| headerKeys = append(headerKeys, key) | ||
| } | ||
| slices.Sort(headerKeys) | ||
|
|
||
| for _, key := range headerKeys { | ||
| _, sensitive := sensitiveResponseHeaders[key] | ||
| for _, value := range resp.Header[key] { | ||
| if sensitive { | ||
| value = redactHeaderValue(value) | ||
| } | ||
| fmt.Fprintf(&headerBuf, "%s: %s\r\n", key, value) | ||
| } | ||
| } |
There was a problem hiding this comment.
Looks copy-pasty from lines 105-124.
| return []aibridge.Provider{provider.NewOpenAI(openaiCfgWithAPIDump(addr, apiKey, dumpDir))} | ||
| }, | ||
| createRequestFunc: createOpenAIChatCompletionsReq, | ||
| }, |
There was a problem hiding this comment.
Please add test case for responses
Signed-off-by: Danny Kopping <[email protected]>
Signed-off-by: Danny Kopping <[email protected]>
Signed-off-by: Danny Kopping <[email protected]>
Signed-off-by: Danny Kopping <[email protected]>
Signed-off-by: Danny Kopping <[email protected]>
Signed-off-by: Danny Kopping <[email protected]>
Signed-off-by: Danny Kopping <[email protected]>
dannykopping
force-pushed
the
dk/request-logging-redux
branch
from
a93b2ec to
e4c2a82
Compare
2 participants
Logs the raw HTTP requests to and responses from AI providers' APIs.