feat(observability): Enhance HTTP logging context and add sandbox startup failure detection#2466
Conversation
AdaAibaby
requested review from
ValentaTomas,
dobrac and
jakubno
as code owners
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
|
Closing this for. Could you please open the issue first, as mentioned in CONTRIBUTING.md? |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bb418ebc23
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| riverClient, err = backgroundworker.StartAuthUserSyncWorker( | ||
| ctx, | ||
| signalCtx, | ||
| supabaseDB, | ||
| authDB, | ||
| l, | ||
| ) | ||
| if err != nil { | ||
| l.Error(ctx, "failed to start auth user sync worker", zap.Error(err)) | ||
|
|
||
| return 1 | ||
| } | ||
| } | ||
|
|
||
| l.Info(ctx, "HTTP service starting", zap.Int("port", config.Port)) | ||
| runErr := waitForServiceStop(signalCtx, startHTTPServer(s), riverStoppedChan(riverClient)) | ||
| if runErr != nil { | ||
| l.Error(ctx, "dashboard-api runtime error", zap.Error(runErr)) | ||
| } else { | ||
| l.Info(ctx, "Shutting down dashboard-api service...") | ||
| } | ||
|
|
||
| shutdownCtx, shutdownCancel := context.WithTimeout(context.WithoutCancel(ctx), shutdownTimeout) | ||
| defer shutdownCancel() | ||
|
|
||
| if err := shutdownService(shutdownCtx, s, riverClient); err != nil { | ||
| l.Error(ctx, "dashboard-api shutdown error", zap.Error(err)) | ||
| r.Use( |
There was a problem hiding this comment.
In run(), the StartAuthUserSyncWorker call was replaced with r.Use(...) arguments, but r only exists inside newHTTPServer, so this commit makes packages/dashboard-api/main.go fail to compile (undefined: r) before the service can start. This also interrupts the original startup/shutdown flow that followed this block, so the regression is blocking for every build/deploy of dashboard-api.
Useful? React with 👍 / 👎.
Thanks for the context! I actually tracked down the root cause here: e2b-dev/E2B#1280 |
4 participants
Changes
1. Enhanced HTTP Logging
request_idandremote_ipfields to theLoggingMiddlewareContextfunction2. Sandbox Startup Failure Detection
GetSandboxesSandboxIDRecordendpointtemplate_alias,lifetime, etc.🔗 Dependency Notice
This PR depends on #2450 (feat(logger): add HTTP request/response logging fields).
The following functions used in this PR are defined in #2450:
logger.WithRequestID(r *http.Request)logger.WithRemoteIP(r *http.Request)Without #2450 merged first, this PR will fail to compile.
Merge Strategy
Recommended: Sequential Merge
Alternative: Merge as Draft
If sequential merging isn't feasible, this PR can remain in Draft status until #2450 is merged, then be marked ready for review.
Technical Details
main.go: Modified logging middleware configurationhandlers.go: Added sandbox lifecycle detection logicrequest_id: Request tracing identifier (requires feat(logger): add HTTP request/response logging fields #2450)remote_ip: Client IP address (requires feat(logger): add HTTP request/response logging fields #2450)template_alias: Template alias used by the sandboxlifetime: Sandbox lifetime durationValue and Impact