diff --git a/core/main/src/firebolt/handlers/metrics_rpc.rs b/core/main/src/firebolt/handlers/metrics_rpc.rs index cf629dd7a..86bd91d36 100644 --- a/core/main/src/firebolt/handlers/metrics_rpc.rs +++ b/core/main/src/firebolt/handlers/metrics_rpc.rs @@ -654,11 +654,17 @@ impl MetricsServer for MetricsImpl { } async fn app_info(&self, ctx: CallContext, app_info_params: AppInfoParams) -> RpcResult<()> { trace!("metrics.app_info: app_info_params={:?}", app_info_params); - self.state + match self + .state .app_manager_state - .set_app_metrics_version(&ctx.app_id, app_info_params.build); - - Ok(()) + .set_app_metrics_version(&ctx.app_id, app_info_params.build) + { + Ok(_) => Ok(()), + Err(e) => { + error!("Error setting app metrics version: {:?}", e); + Err(rpc_err("Unable to set app info")) + } + } } } diff --git a/core/main/src/processor/metrics_processor.rs b/core/main/src/processor/metrics_processor.rs index 921e85478..2a2fd7062 100644 --- a/core/main/src/processor/metrics_processor.rs +++ b/core/main/src/processor/metrics_processor.rs @@ -83,16 +83,12 @@ pub async fn update_app_context( if let Some(app) = ps.app_manager_state.get(&ctx.app_id) { context.app_session_id = app.loaded_session_id.to_owned(); context.app_user_session_id = app.active_session_id; - context.app_version = ps + context.product_version = ps .version .clone() .unwrap_or(String::from(SEMVER_LIGHTWEIGHT)); - // TODO: Meeting 12/11/24 to discuss where version from Metrics.appInfo call should be used. - // If part of BehavioralMetricContext, un/comment below/above and modify send_behavioral - // to NOT populate app_ver using the context, get per above instead. - - //context.app_version = app.app_metrics_version.unwrap_or(String::default()); + context.app_version = app.app_metrics_version.clone(); } if let Some(session) = ps.session_state.get_account_session() { context.partner_id = session.id; @@ -141,7 +137,7 @@ pub async fn send_metric_for_app_state_change( if let Some(app) = ps.app_manager_state.get(app_id) { context.app_session_id = app.loaded_session_id.to_owned(); context.app_user_session_id = app.active_session_id; - context.app_version = ps + context.product_version = ps .version .clone() .unwrap_or(String::from(SEMVER_LIGHTWEIGHT)); diff --git a/core/main/src/service/apps/delegated_launcher_handler.rs b/core/main/src/service/apps/delegated_launcher_handler.rs index d784b5910..cd1d916a3 100644 --- a/core/main/src/service/apps/delegated_launcher_handler.rs +++ b/core/main/src/service/apps/delegated_launcher_handler.rs @@ -344,11 +344,13 @@ impl AppManagerState { intents.remove(app_id) } - pub fn set_app_metrics_version(&self, app_id: &str, version: String) { + pub fn set_app_metrics_version(&self, app_id: &str, version: String) -> Result<(), AppError> { let mut apps = self.apps.write().unwrap(); if let Some(app) = apps.get_mut(app_id) { app.app_metrics_version = Some(version); + return Ok(()); } + Err(AppError::NotFound) } } @@ -559,10 +561,11 @@ impl DelegatedLauncherHandler { let context = BehavioralMetricContext { app_id: app_id.to_string(), - app_version: SEMVER_LIGHTWEIGHT.to_string(), + product_version: SEMVER_LIGHTWEIGHT.to_string(), partner_id: String::from("partner.id.not.set"), app_session_id: String::from("app_session_id.not.set"), durable_app_id: app_id.to_string(), + app_version: None, app_user_session_id: None, governance_state: None, }; diff --git a/core/main/src/state/metrics_state.rs b/core/main/src/state/metrics_state.rs index e03720133..533569fa6 100644 --- a/core/main/src/state/metrics_state.rs +++ b/core/main/src/state/metrics_state.rs @@ -296,7 +296,7 @@ impl MetricsState { }) }) }) - .unwrap_or_default(); + .unwrap_or("not.set".into()); let device_name = rpc_value_result_to_string_result( BrokerUtils::process_internal_main_request(state, "device.name", None).await, diff --git a/core/sdk/src/api/firebolt/fb_metrics.rs b/core/sdk/src/api/firebolt/fb_metrics.rs index 09bf21cf5..e241b2040 100644 --- a/core/sdk/src/api/firebolt/fb_metrics.rs +++ b/core/sdk/src/api/firebolt/fb_metrics.rs @@ -37,7 +37,9 @@ use super::fb_telemetry::{OperationalMetricRequest, TelemetryPayload}; #[derive(Debug, PartialEq, Serialize, Deserialize, Clone)] pub struct BehavioralMetricContext { pub app_id: String, - pub app_version: String, + #[serde(skip_serializing_if = "Option::is_none")] + pub app_version: Option, + pub product_version: String, pub partner_id: String, pub app_session_id: String, pub app_user_session_id: Option, @@ -49,10 +51,11 @@ impl From for BehavioralMetricContext { fn from(call_context: CallContext) -> Self { BehavioralMetricContext { app_id: call_context.app_id.clone(), - app_version: String::from("app.version.not.implemented"), + product_version: String::from("product.version.not.implemented"), partner_id: String::from("partner.id.not.set"), app_session_id: String::from("app_session_id.not.set"), durable_app_id: call_context.app_id, + app_version: None, app_user_session_id: None, governance_state: None, } @@ -1103,11 +1106,12 @@ mod tests { fn test_extn_request_behavioral_metric() { let behavioral_metric_context = BehavioralMetricContext { app_id: "test_app_id".to_string(), - app_version: "test_app_version".to_string(), + product_version: "test_product_version".to_string(), partner_id: "test_partner_id".to_string(), app_session_id: "test_app_session_id".to_string(), app_user_session_id: Some("test_user_session_id".to_string()), durable_app_id: "test_durable_app_id".to_string(), + app_version: Some("test_app_version".to_string()), governance_state: Some(AppDataGovernanceState { data_tags_to_apply: HashSet::new(), }), @@ -1170,11 +1174,12 @@ mod tests { let behavior_metric_payload = BehavioralMetricPayload::Ready(Ready { context: BehavioralMetricContext { app_id: "test_app_id".to_string(), - app_version: "test_app_version".to_string(), + product_version: "test_product_version".to_string(), partner_id: "test_partner_id".to_string(), app_session_id: "test_app_session_id".to_string(), app_user_session_id: Some("test_user_session_id".to_string()), durable_app_id: "test_durable_app_id".to_string(), + app_version: Some("test_app_version".to_string()), governance_state: Some(AppDataGovernanceState { data_tags_to_apply: HashSet::new(), }), @@ -1251,11 +1256,12 @@ mod tests { fn test_behavioral_metric_payload() { let behavioral_metric_context = BehavioralMetricContext { app_id: "test_app_id".to_string(), - app_version: "test_app_version".to_string(), + product_version: "test_product_version".to_string(), partner_id: "test_partner_id".to_string(), app_session_id: "test_app_session_id".to_string(), app_user_session_id: Some("test_user_session_id".to_string()), durable_app_id: "test_durable_app_id".to_string(), + app_version: Some("test_app_version".to_string()), governance_state: Some(AppDataGovernanceState { data_tags_to_apply: HashSet::new(), }), @@ -1275,11 +1281,12 @@ mod tests { let new_behavioral_metric_context = BehavioralMetricContext { app_id: "new_test_app_id".to_string(), - app_version: "new_test_app_version".to_string(), + product_version: "new_test_product_version".to_string(), partner_id: "new_test_partner_id".to_string(), app_session_id: "new_test_app_session_id".to_string(), app_user_session_id: Some("new_test_user_session_id".to_string()), durable_app_id: "new_test_durable_app_id".to_string(), + app_version: Some("test_app_version".to_string()), governance_state: Some(AppDataGovernanceState { data_tags_to_apply: HashSet::new(), }),