Repository navigation
Conversation
Add a :7300 ingress port to the enclaver manifest and launch op-batcher with metrics enabled on 0.0.0.0:7300 so the enclaver ingress can bridge Prometheus metrics out of the Nitro enclave to the parent. The parent-side bridge (tcp<->vsock) lives in EspressoSystems/tee-image-builder. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![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: 0433510f2b
ℹ️ 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".
| # localhost, or the ingress cannot reach the endpoint. Skip if the caller already set it. | ||
| metrics_already_set=false | ||
| for arg in "${all_args[@]}"; do | ||
| case "$arg" in --metrics.enabled*) metrics_already_set=true; break;; esac |
There was a problem hiding this comment.
When a caller supplies --metrics.enabled together with a custom --metrics.port or --metrics.addr, this check suppresses all of the ingress-compatible defaults. The manifest still exposes only port 7300, while initMetricsServer binds the caller-supplied address and port, so for example --metrics.enabled --metrics.port=9000 leaves the parent-side 7300 bridge with no listener; a loopback address has the same issue. Inject the address/port independently or reject configurations that cannot be reached through the fixed ingress.
Useful? React with 👍 / 👎.
| for arg in "${all_args[@]}"; do | ||
| case "$arg" in --metrics.enabled*) metrics_already_set=true; break;; esac | ||
| done | ||
| if ! $metrics_already_set; then | ||
| all_args+=(--metrics.enabled --metrics.addr=0.0.0.0 --metrics.port=7300) | ||
| fi |
There was a problem hiding this comment.
I think this can be confusing in some cases. If someone passes --metrics.enabled --metrics.port=9000, we skip the defaults and the batcher ends up on a port the manifest doesn't bridge, so scrapes just fail quietly. The other way round, --metrics.port=9000 on its own gets overridden by the appended 7300. Since the cli takes the last value, I think we can drop the loop and just order the args:
all_args=(--metrics.enabled --metrics.addr=0.0.0.0 "${filtered_args[@]}" "${url_args[@]}" --metrics.port=7300)Review on #536: the previous "skip if caller set --metrics.enabled" guard let a caller-supplied --metrics.port/addr bind somewhere the enclaver ingress (fixed at 7300) doesn't bridge, so scrapes failed silently. Drop the guard and append enabled/addr/port last so they win via urfave/cli's last-value-wins, guaranteeing the batcher always serves on 0.0.0.0:7300. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
This branch has not been deployed
3 participants
Follow-up to #527. Ports the "expose Prometheus :7300 via enclaver ingress" work into the op repo (Asana: Setup Prometheus Metrics / Chaos Testnet).
What
op-batcher/enclave-tools/enclaver.go: add aMetricsPort = 7300ingress entry to the enclaver manifest so enclaver bridges the metrics port out of the enclave.op-batcher/enclave-entrypoint.bash: launch op-batcher with--metrics.enabled --metrics.addr=0.0.0.0 --metrics.port=7300(skipped if the caller already set metrics). Binding to0.0.0.0is required — op-service has aTODO: Switch to 127.0.0.1default that would otherwise break ingress — and is guarded so it won't duplicate caller-provided flags.Not included (lives in tee-image-builder)
The parent-side tcp<->vsock bridge that exposes 7300 on the host, same split as the 8337/8338 ports.
Base
Stacked on
espresso/tee-images(#527) since that branch introduces the enclave tooling and isn't merged yet.🤖 Generated with Claude Code