Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e7e1e27f0a
ℹ️ 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".
| // incremental local development and test runs do not emit | ||
| // best-effort OTEL traffic unless a test or binary opts into an | ||
| // explicit exporter configuration. | ||
| if cfg!(debug_assertions) { |
There was a problem hiding this comment.
Preserve test-time exporter disable outside debug assertions
Switching the Statsig guard to cfg!(debug_assertions) means tests built with optimized profiles (cargo test --release or any profile with debug-assertions = false) will now resolve OtelExporter::Statsig to a real OTLP endpoint instead of None. Before this change, the workspace feature kept test runs from emitting network telemetry regardless of optimization level; with this condition, those test contexts can perform outbound export unexpectedly and become flaky/fail in network-restricted environments.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
We don't claim that cargo test --release works in this project.
Why
codex-otelstill carrieddisable-default-metrics-exporter, which was the last remaining workspace crate feature.We are removing workspace crate features because they do not fit our current build model well:
For this case, the feature was only being used to keep the built-in Statsig metrics exporter off in test and debug-oriented contexts. This repo already treats
debug_assertionsas the practical proxy for that class of behavior, so OTEL should follow the same convention instead of keeping a dedicated crate feature alive.What changed
disable-default-metrics-exporterfromcodex-rs/otel/Cargo.tomlcodex-oteldev-dependency feature activation fromcodex-rs/core/Cargo.tomlcodex-rs/otel/src/config.rsso the built-inOtelExporter::Statsigdefault resolves toNonewhendebug_assertionsis enabled, with a focused unit test covering that behavior.github/scripts/verify_cargo_workspace_manifests.py, so workspace crate features are now hard-banned instead of temporarily allowlistedHow tested
python3 .github/scripts/verify_cargo_workspace_manifests.pycargo test -p codex-otelcargo test -p codex-core metrics_exporter_defaults_to_statsig_when_missingcargo test -p codex-app-server app_server_default_analytics_just bazel-lock-check