-
Notifications
You must be signed in to change notification settings - Fork 1.2k
feat(gateway): identify gateways in exported traces #2647
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,7 +7,7 @@ use clap::parser::ValueSource; | |
| use clap::{ArgAction, ArgMatches, Command, CommandFactory, FromArgMatches, Parser}; | ||
| use miette::{IntoDiagnostic, Result}; | ||
| use openshell_core::ComputeDriverKind; | ||
| use openshell_core::config::DEFAULT_SERVER_PORT; | ||
| use openshell_core::config::{DEFAULT_GATEWAY_NAME, DEFAULT_SERVER_PORT}; | ||
| use std::net::{IpAddr, SocketAddr}; | ||
| use std::path::PathBuf; | ||
| use tracing::{error, info, warn}; | ||
|
|
@@ -52,6 +52,14 @@ struct RunArgs { | |
| #[arg(long, env = "OPENSHELL_GATEWAY_CONFIG")] | ||
| config: Option<PathBuf>, | ||
|
|
||
| /// Operator-assigned name for this gateway installation. | ||
| #[arg( | ||
| long = "name", | ||
| default_value = DEFAULT_GATEWAY_NAME, | ||
| env = "OPENSHELL_GATEWAY_NAME" | ||
| )] | ||
| name: String, | ||
|
|
||
| /// IP address to bind the server, health, and metrics listeners to. | ||
| #[arg(long, default_value = "127.0.0.1", env = "OPENSHELL_BIND_ADDRESS")] | ||
| bind_address: IpAddr, | ||
|
|
@@ -307,7 +315,13 @@ fn prepare_server_config(args: &mut RunArgs, matches: &ArgMatches) -> Result<Ser | |
| .clone() | ||
| .expect("runtime defaults populate db_url"); | ||
|
|
||
| let name = args.name.trim(); | ||
| if name.is_empty() { | ||
| return Err(miette::miette!("gateway name must not be empty")); | ||
| } | ||
|
|
||
| let mut config = openshell_core::Config::new(tls) | ||
| .with_name(name) | ||
| .with_bind_address(bind) | ||
| .with_log_level(&args.log_level); | ||
| if let Some(auth) = file.as_ref().and_then(|f| f.openshell.gateway.auth.clone()) { | ||
|
|
@@ -460,11 +474,16 @@ async fn run_from_args(mut args: RunArgs, matches: ArgMatches) -> Result<()> { | |
| .config_file | ||
| .as_ref() | ||
| .and_then(|f| f.openshell.gateway.otlp.as_ref()); | ||
| let gateway_resource = crate::otel_tracing::GatewayResourceAttributes::new( | ||
| Some(prepared.config.name.as_str()), | ||
| prepared.config.compute_drivers.first().map(String::as_str), | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Warning — GATOR-f870a4f7-01 Invariant: Gateway OTLP resources must identify the actual supported compute driver, including the documented auto-detection path, rather than only an explicitly configured driver. Prerequisite: An operator enables Entry point → sink: Base → head: Base exported no compute-driver resource attribute. Head promises and exports Impact: Common default/package-managed deployments emit incomplete trace identity, making the newly documented compute-driver dimension materially false for a supported startup mode. Reproducer: Configure only PR ownership: This PR introduces both the attribute and its public documentation, while this new line sources it before the existing auto-detection path runs. Requested change: Populate the resource from the same effective driver resolution used by startup—either resolve the default driver before tracing installation or share/refactor the existing resolution so tracing receives the actual selected driver without duplicating divergent selection rules. |
||
| ); | ||
| let (tracing_handle, setup_error) = crate::tracing_setup::install( | ||
| EnvFilter::try_from_default_env() | ||
| .unwrap_or_else(|_| EnvFilter::new(&prepared.config.log_level)), | ||
| &tracing_log_bus, | ||
| otlp_config, | ||
| gateway_resource, | ||
| ); | ||
|
|
||
| let has_client_ca = prepared | ||
|
|
@@ -615,6 +634,11 @@ fn resolve_aux_listener( | |
| /// The function intentionally does not touch `database_url` — that secret is | ||
| /// env-only and the loader already rejected it when it appears in the file. | ||
| fn merge_file_into_args(args: &mut RunArgs, file: &GatewayFileSection, matches: &ArgMatches) { | ||
| if let Some(name) = &file.name | ||
| && arg_defaulted(matches, "name") | ||
| { | ||
| args.name.clone_from(name); | ||
| } | ||
| if let Some(addr) = file.bind_address { | ||
| if arg_defaulted(matches, "bind_address") { | ||
| args.bind_address = addr.ip(); | ||
|
|
@@ -1344,12 +1368,14 @@ enabled = false | |
| let _g1 = EnvVarGuard::remove("OPENSHELL_BIND_ADDRESS"); | ||
| let _g2 = EnvVarGuard::remove("OPENSHELL_SERVER_PORT"); | ||
| let _g3 = EnvVarGuard::remove("OPENSHELL_LOG_LEVEL"); | ||
| let _g4 = EnvVarGuard::remove("OPENSHELL_GATEWAY_NAME"); | ||
|
|
||
| let (mut args, matches) = | ||
| parse_with_args(&["openshell-gateway", "--db-url", "sqlite::memory:"]); | ||
| let file = config_file_from_toml( | ||
| r#" | ||
| [openshell.gateway] | ||
| name = "production-us-west" | ||
| bind_address = "0.0.0.0:9090" | ||
| log_level = "debug" | ||
| "#, | ||
|
|
@@ -1359,6 +1385,7 @@ log_level = "debug" | |
| assert_eq!(args.bind_address, IpAddr::V4(Ipv4Addr::UNSPECIFIED)); | ||
| assert_eq!(args.port, 9090); | ||
| assert_eq!(args.log_level, "debug"); | ||
| assert_eq!(args.name, "production-us-west"); | ||
| } | ||
|
|
||
| #[test] | ||
|
|
@@ -1368,23 +1395,28 @@ log_level = "debug" | |
| .unwrap_or_else(std::sync::PoisonError::into_inner); | ||
| let _g1 = EnvVarGuard::remove("OPENSHELL_BIND_ADDRESS"); | ||
| let _g2 = EnvVarGuard::remove("OPENSHELL_LOG_LEVEL"); | ||
| let _g3 = EnvVarGuard::remove("OPENSHELL_GATEWAY_NAME"); | ||
|
|
||
| let (mut args, matches) = parse_with_args(&[ | ||
| "openshell-gateway", | ||
| "--db-url", | ||
| "sqlite::memory:", | ||
| "--log-level", | ||
| "warn", | ||
| "--name", | ||
| "cli-gateway", | ||
| ]); | ||
| let file = config_file_from_toml( | ||
| r#" | ||
| [openshell.gateway] | ||
| name = "file-gateway" | ||
| log_level = "debug" | ||
| "#, | ||
| ); | ||
| merge_file_into_args(&mut args, &file.openshell.gateway, &matches); | ||
|
|
||
| assert_eq!(args.log_level, "warn", "CLI flag must win over file"); | ||
| assert_eq!(args.name, "cli-gateway"); | ||
| } | ||
|
|
||
| #[test] | ||
|
|
@@ -1393,18 +1425,21 @@ log_level = "debug" | |
| .lock() | ||
| .unwrap_or_else(std::sync::PoisonError::into_inner); | ||
| let _g = EnvVarGuard::set("OPENSHELL_LOG_LEVEL", "trace"); | ||
| let _g2 = EnvVarGuard::set("OPENSHELL_GATEWAY_NAME", "env-gateway"); | ||
|
|
||
| let (mut args, matches) = | ||
| parse_with_args(&["openshell-gateway", "--db-url", "sqlite::memory:"]); | ||
| let file = config_file_from_toml( | ||
| r#" | ||
| [openshell.gateway] | ||
| name = "file-gateway" | ||
| log_level = "debug" | ||
| "#, | ||
| ); | ||
| merge_file_into_args(&mut args, &file.openshell.gateway, &matches); | ||
|
|
||
| assert_eq!(args.log_level, "trace", "env var must win over file"); | ||
| assert_eq!(args.name, "env-gateway"); | ||
| } | ||
|
|
||
| #[test] | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it seems
prepared.config.compute_driversis often intentionally left empty, leaving it up to openshell to choose the right driver. I'm not sure when that auto-detection hapens, and if it mutates this field, or if maybe the gateway_resource naming has to happen later?