chore(orch): add more info for sandboxes#2174
Conversation
![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: 840b28ba3a
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
| telemetry.WithSandboxID(req.GetSandbox().GetSandboxId()), | ||
| attribute.String("client.id", s.info.ClientId), | ||
| attribute.String("envd.version", req.GetSandbox().GetEnvdVersion()), | ||
| telemetry.WithEnvdVersion(req.GetSandbox().GetEnvdVersion()), |
There was a problem hiding this comment.
Span attribute client.id accidentally removed from Create
Medium Severity
The attribute.String("client.id", s.info.ClientId) span attribute was dropped from the Create handler during this refactor. The Update (line 289) and Delete (line 414) handlers still set client.id on their spans, so this is an inconsistency. Since the PR intent is to add more info, this looks like an accidental removal.
There was a problem hiding this comment.
Not needed, it shouldn't be really used anyway
| } | ||
|
|
||
| func WithKernelVersion(kernelVersion string) zap.Field { | ||
| return zap.String("kernel.version", kernelVersion) |
There was a problem hiding this comment.
lets call these sandbox.kernel.version or sandbox.kernel_version
(similar for the FC version)
3 participants
Note
Low Risk
Mostly observability changes: adds consistent kernel/firecracker/envd version fields to spans and logs, with a small refactor around sandbox config construction that should not affect runtime behavior. Low risk aside from potential metric/log attribute key changes impacting dashboards.
Overview
This PR improves sandbox observability by standardizing kernel/firecracker version attributes in telemetry and adding envd/kernel/firecracker version fields to orchestrator sandbox lifecycle logs (including map insertion and sandbox-create failure paths). It also refactors sandbox creation to build a
config/runtimeobject before resuming the sandbox (so the resolved Firecracker version is recorded consistently), and updates tests to initialize the required sandbox config metadata.Written by Cursor Bugbot for commit 6f3f0b3. This will update automatically on new commits. Configure here.