Skip to content
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

feat: Add metrics for user creation/deletion. #506

Merged
merged 14 commits into from
Nov 9, 2023
Merged
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ impl WebPushClient {
// Log out the final stats message
info!("Session";
"uaid_hash" => self.uaid.as_simple().to_string(),
"uaid_reset" => self.flags.reset_uaid,
"uaid_reset" => self.flags.old_record_version,
"existing_uaid" => stats.existing_uaid,
"connection_type" => "webpush",
"ua_name" => &ua_info.browser_name,
Expand Down Expand Up @@ -297,6 +297,7 @@ pub async fn process_existing_user(
.metrics
.incr_with_tags("ua.expiration")
.with_tag("code", "104")
.with_tag("reason", "no_current_month")
.send();
app_state.db.remove_user(&user.uaid).await?;
return Ok(None);
Expand All @@ -311,6 +312,7 @@ pub async fn process_existing_user(
.metrics
.incr_with_tags("ua.expiration")
.with_tag("code", "105")
.with_tag("reason", "invalid_current_month")
.send();
app_state.db.remove_user(&user.uaid).await?;
return Ok(None);
Expand All @@ -319,7 +321,7 @@ pub async fn process_existing_user(

let flags = ClientFlags {
check_storage: true,
reset_uaid: user
old_record_version: user
.record_version
.map_or(true, |rec_ver| rec_ver < USER_RECORD_VERSION),
..Default::default()
Expand All @@ -336,7 +338,7 @@ pub struct ClientFlags {
/// Whether this client needs to check storage for messages
check_storage: bool,
/// Flags the need to drop the user record
reset_uaid: bool,
old_record_version: bool,
}

impl Default for ClientFlags {
Expand All @@ -345,7 +347,7 @@ impl Default for ClientFlags {
include_topic: true,
increment_storage: false,
check_storage: false,
reset_uaid: false,
old_record_version: false,
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -314,8 +314,13 @@ impl WebPushClient {
// All Ack'd and finished checking/incrementing storage
debug_assert!(!self.ack_state.unacked_notifs());
let flags = &self.flags;
if flags.reset_uaid {
debug!("▶️ WebPushClient:post_process_all_acked reset_uaid");
if flags.old_record_version {
debug!("▶️ WebPushClient:post_process_all_acked; resetting uaid");
self.app_state
.metrics
.incr_with_tags("ua.expiration")
.with_tag("reason", "old_record_version")
.send();
self.app_state.db.remove_user(&self.uaid).await?;
Err(SMErrorKind::UaidReset.into())
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -295,6 +295,11 @@ impl WebPushClient {
if self.sent_from_storage > self.app_state.settings.msg_limit {
// Exceeded the max limit of stored messages: drop the user to
// trigger a re-register
self.app_state
.metrics
.incr_with_tags("ua.expiration")
.with_tag("reason", "too_many_messages")
.send();
self.app_state.db.remove_user(&self.uaid).await?;
return Err(SMErrorKind::UaidReset.into());
}
Expand Down
18 changes: 15 additions & 3 deletions autoconnect/autoconnect-ws/autoconnect-ws-sm/src/unidentified.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,19 +67,31 @@ impl UnidentifiedClient {
);

// Ignore invalid uaids (treat as None) so they'll be issued a new one
let uaid = uaid.as_deref().and_then(|uaid| Uuid::try_parse(uaid).ok());
let original_uaid = uaid.as_deref().and_then(|uaid| Uuid::try_parse(uaid).ok());

let GetOrCreateUser {
user,
existing_user,
flags,
} = self.get_or_create_user(uaid).await?;
} = self.get_or_create_user(original_uaid).await?;
let uaid = user.uaid;
debug!(
"💬UnidentifiedClient::on_client_msg Hello! uaid: {} existing_user: {}",
uaid, existing_user,
);
let _ = self.app_state.metrics.incr("ua.command.hello");
self.app_state
.metrics
.incr_with_tags("ua.command.hello")
.with_tag("uaid", {
if existing_user {
"existing"
} else if original_uaid.unwrap_or(uaid) != uaid {
"reassigned"
} else {
"new"
}
})
.send();

let (broadcast_subs, broadcasts) = self
.broadcast_init(&Broadcast::from_hashmap(broadcasts.unwrap_or_default()))
Expand Down
22 changes: 12 additions & 10 deletions autopush/src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -215,8 +215,10 @@ pub struct ClientFlags {
increment_storage: bool,
/// Whether this client needs to check storage for messages
check: bool,
/// Flags the need to drop the user record
reset_uaid: bool,
/// Flags the need to drop the user record if the User record_version is less
/// than the USER_RECORD_VERSION constant. The reset is done after all existing
/// message traffic is sent over.
old_record_version: bool,
rotate_message_table: bool,
}

Expand All @@ -226,7 +228,7 @@ impl ClientFlags {
include_topic: true,
increment_storage: false,
check: false,
reset_uaid: false,
old_record_version: false,
rotate_message_table: false,
}
}
Expand Down Expand Up @@ -421,7 +423,7 @@ where
uaid,
message_month,
check_storage,
reset_uaid,
old_record_version,
rotate_message_table,
connected_at,
deferred_user_registration,
Expand All @@ -432,7 +434,7 @@ where
uaid: Some(uaid),
message_month,
check_storage,
reset_uaid,
old_record_version,
rotate_message_table,
connected_at,
deferred_user_registration,
Expand All @@ -442,7 +444,7 @@ where
uaid,
message_month,
check_storage,
reset_uaid,
old_record_version,
rotate_message_table,
connected_at,
deferred_user_registration,
Expand Down Expand Up @@ -484,7 +486,7 @@ where
// Setup the objects and such needed for a WebPushClient
let mut flags = ClientFlags::new();
flags.check = check_storage;
flags.reset_uaid = reset_uaid;
flags.old_record_version = old_record_version;
flags.rotate_message_table = rotate_message_table;
let (initialized_subs, broadcasts) = srv.broadcast_init(&desired_broadcasts);
broadcast_subs.replace(initialized_subs);
Expand All @@ -498,7 +500,7 @@ where
connected_at,
stats: SessionStatistics {
uaid: uaid.as_simple().to_string(),
uaid_reset: reset_uaid,
uaid_reset: old_record_version,
existing_uaid: check_storage,
connection_type: String::from("webpush"),
..Default::default()
Expand Down Expand Up @@ -949,8 +951,8 @@ where
.migrate_user(&webpush.uaid, &webpush.message_month),
);
transition!(AwaitMigrateUser { response, data });
} else if all_acked && webpush.flags.reset_uaid {
debug!("Dropping user: flagged reset_uaid");
} else if all_acked && webpush.flags.old_record_version {
debug!("Dropping user: flagged old_record_version");
let response = Box::new(data.srv.ddb.drop_uaid(&webpush.uaid));
transition!(AwaitDropUser { response, data });
}
Expand Down
2 changes: 1 addition & 1 deletion autopush/src/db/commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -484,7 +484,7 @@ fn handle_user_result(
hello_response.check_storage = true;
hello_response.rotate_message_table = user_month != *cur_month;
hello_response.message_month = user_month;
hello_response.reset_uaid = user
hello_response.old_record_version = user
.record_version
.map_or(true, |rec_ver| rec_ver < USER_RECORD_VERSION);

Expand Down
2 changes: 1 addition & 1 deletion autopush/src/db/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ pub struct HelloResponse {
pub uaid: Option<Uuid>,
pub message_month: String,
pub check_storage: bool,
pub reset_uaid: bool,
pub old_record_version: bool,
pub rotate_message_table: bool,
pub connected_at: u64,
// Exists when we didn't register this user during HELLO
Expand Down