mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
otel: remove the last workspace crate feature (#16469)
## Why `codex-otel` still carried `disable-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: - our Bazel setup does not honor crate features today, which can let feature-gated issues go unnoticed - they create extra crate build permutations that we want to avoid 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_assertions` as 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 - removed `disable-default-metrics-exporter` from `codex-rs/otel/Cargo.toml` - removed the `codex-otel` dev-dependency feature activation from `codex-rs/core/Cargo.toml` - changed `codex-rs/otel/src/config.rs` so the built-in `OtelExporter::Statsig` default resolves to `None` when `debug_assertions` is enabled, with a focused unit test covering that behavior - removed the final feature exceptions from `.github/scripts/verify_cargo_workspace_manifests.py`, so workspace crate features are now hard-banned instead of temporarily allowlisted - expanded the verifier error message to explain the Bazel mismatch and build-permutation cost behind that policy ## How tested - `python3 .github/scripts/verify_cargo_workspace_manifests.py` - `cargo test -p codex-otel` - `cargo test -p codex-core metrics_exporter_defaults_to_statsig_when_missing` - `cargo test -p codex-app-server app_server_default_analytics_` - `just bazel-lock-check`
This commit is contained in:
committed by
GitHub
Unverified
parent
a99d4845e3
commit
323aa968c3
@@ -26,19 +26,9 @@ TOP_LEVEL_NAME_EXCEPTIONS = {
|
||||
UTILITY_NAME_EXCEPTIONS = {
|
||||
"path-utils": "codex-utils-path",
|
||||
}
|
||||
MANIFEST_FEATURE_EXCEPTIONS = {
|
||||
"codex-rs/otel/Cargo.toml": {
|
||||
"disable-default-metrics-exporter": (),
|
||||
},
|
||||
}
|
||||
MANIFEST_FEATURE_EXCEPTIONS = {}
|
||||
OPTIONAL_DEPENDENCY_EXCEPTIONS = set()
|
||||
INTERNAL_DEPENDENCY_FEATURE_EXCEPTIONS = {
|
||||
(
|
||||
"codex-rs/core/Cargo.toml",
|
||||
"dev-dependencies",
|
||||
"codex-otel",
|
||||
): ("disable-default-metrics-exporter",),
|
||||
}
|
||||
INTERNAL_DEPENDENCY_FEATURE_EXCEPTIONS = {}
|
||||
|
||||
|
||||
def main() -> int:
|
||||
@@ -73,6 +63,12 @@ def main() -> int:
|
||||
"opt into workspace lints, and avoid introducing new workspace crate "
|
||||
"features."
|
||||
)
|
||||
print(
|
||||
"Workspace crate features are disallowed because our Bazel build setup "
|
||||
"does not honor them today, which can let issues hidden behind feature "
|
||||
"gates go unnoticed, and because they add extra crate build "
|
||||
"permutations we want to avoid."
|
||||
)
|
||||
print(
|
||||
"Cargo only applies `codex-rs/Cargo.toml` `[workspace.lints.clippy]` "
|
||||
"entries to a crate when that crate declares:"
|
||||
@@ -91,8 +87,8 @@ def main() -> int:
|
||||
"`codex-rs/utils/<crate>/Cargo.toml`."
|
||||
)
|
||||
print(
|
||||
"Existing workspace crate features are temporarily allowlisted in this "
|
||||
"script and must shrink as they are removed."
|
||||
"Workspace crate features are forbidden; add a targeted exception here "
|
||||
"only if there is a deliberate temporary migration in flight."
|
||||
)
|
||||
print()
|
||||
for path in sorted(failures_by_path):
|
||||
|
||||
@@ -146,9 +146,7 @@ codex-shell-escalation = { workspace = true }
|
||||
assert_cmd = { workspace = true }
|
||||
assert_matches = { workspace = true }
|
||||
codex-arg0 = { workspace = true }
|
||||
codex-otel = { workspace = true, features = [
|
||||
"disable-default-metrics-exporter",
|
||||
] }
|
||||
codex-otel = { workspace = true }
|
||||
codex-utils-cargo-bin = { workspace = true }
|
||||
core_test_support = { workspace = true }
|
||||
ctor = { workspace = true }
|
||||
|
||||
@@ -12,13 +12,6 @@ path = "src/lib.rs"
|
||||
[lints]
|
||||
workspace = true
|
||||
|
||||
[features]
|
||||
## Disables the built-in default metrics exporter.
|
||||
##
|
||||
## Intended for use from `dev-dependencies` so unit/integration tests never
|
||||
## attempt to export metrics over the network.
|
||||
disable-default-metrics-exporter = []
|
||||
|
||||
[dependencies]
|
||||
chrono = { workspace = true }
|
||||
codex-utils-absolute-path = { workspace = true }
|
||||
|
||||
@@ -10,7 +10,11 @@ pub(crate) const STATSIG_API_KEY: &str = "client-MkRuleRQBd6qakfnDYqJVR9JuXcY57L
|
||||
pub(crate) fn resolve_exporter(exporter: &OtelExporter) -> OtelExporter {
|
||||
match exporter {
|
||||
OtelExporter::Statsig => {
|
||||
if cfg!(test) || cfg!(feature = "disable-default-metrics-exporter") {
|
||||
// Keep the built-in Statsig default off in debug builds so
|
||||
// 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) {
|
||||
return OtelExporter::None;
|
||||
}
|
||||
|
||||
@@ -74,3 +78,17 @@ pub enum OtelExporter {
|
||||
tls: Option<OtelTlsConfig>,
|
||||
},
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::OtelExporter;
|
||||
use super::resolve_exporter;
|
||||
|
||||
#[test]
|
||||
fn statsig_default_metrics_exporter_is_disabled_in_debug_builds() {
|
||||
assert!(matches!(
|
||||
resolve_exporter(&OtelExporter::Statsig),
|
||||
OtelExporter::None
|
||||
));
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user