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

Detect emptied nos contact lists + metrics #17

Merged
merged 1 commit into from
Sep 12, 2024
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 41 additions & 8 deletions src/domain/follows_differ.rs
Original file line number Diff line number Diff line change
@@ -70,6 +70,7 @@ where
"Skipping follow list for {} as it's older than the last update",
follower.to_bech32().unwrap_or_default()
);
metrics::already_seen_contact_lists().increment(1);
return Ok(());
}
}
@@ -287,13 +288,7 @@ fn probably_inactive_or_spam(event: &Event) -> bool {
let number_of_p_tags = event
.tags()
.iter()
.filter(|tag| {
if let Some(TagStandard::PublicKey { .. }) = tag.as_standardized() {
true
} else {
false
}
})
.filter(|tag| matches!(tag.as_standardized(), Some(TagStandard::PublicKey { .. })))
.count();

return number_of_p_tags < 2;
@@ -357,13 +352,19 @@ fn log_line(

// Investigate states in which there are no followees but there are unfollowed followees
if followed_counter == 0 && unfollowed_counter > 0 && unchanged == 0 {
metrics::sudden_follow_drops().increment(1);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do these metrics calls automatically include metadata like the ID of the event that triggered it and/or the author pubkey? I think those would be really useful for post-mortems.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That wouldn't be a good idea because of cardinality issues. In fact, we should get back to the event service and remove those metrics that include free form labels. But we do log each of these instances. So when we receive alerts, we can always track what happened through the logs.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh I see. I finally looked up the definition of cardinality and it isn't what I thought. TIL 👍


if has_nos_agent(event) {
metrics::nos_sudden_follow_drops().increment(1);
}

return Some(format!(
"ALL UNFOLLOWED: Npub {}: date {}, {} unfollowed, {} unchanged, {}",
follower.to_bech32().unwrap_or_default(),
timestamp_diff,
unfollowed_counter,
unchanged,
event.as_json()
event.as_json(),
));
Comment on lines 361 to 368
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is what we log when this happens, we can filter by the nos client, but we log for any client tag when this situation arises.

}

@@ -377,6 +378,14 @@ fn log_line(
))
}

fn has_nos_agent(event: &Event) -> bool {
event.tags.iter().any(|tag| {
let tag_components = tag.as_vec();

tag_components[0] == "client" && tag_components[1] == "nos"
})
}

fn convert_timestamp(timestamp: u64) -> Result<DateTime<Utc>> {
DateTime::<Utc>::from_timestamp(timestamp as i64, 0).ok_or("Invalid timestamp".into())
}
@@ -388,6 +397,7 @@ mod tests {
use crate::repo::RepoError;
use chrono::{Duration, Utc};
use nostr_sdk::PublicKey;
use std::borrow::Cow;
use std::collections::HashMap;
use std::sync::Arc;
use std::sync::LazyLock;
@@ -922,6 +932,29 @@ mod tests {
assert_follow_changes(vec![wrong_event], vec![]).await;
}

#[test]
fn test_nos_client_tag() {
let tag = Tag::custom(TagKind::Custom(Cow::Borrowed("client")), ["foobar"]);

let keys = Keys::generate();
let event_not_matching = EventBuilder::new(Kind::ContactList, "", vec![tag])
.to_event(&keys)
.unwrap();

let tag = Tag::custom(TagKind::Custom(Cow::Borrowed("client")), ["nos"]);
let event_matching = EventBuilder::new(Kind::ContactList, "", vec![tag])
.to_event(&keys)
.unwrap();

let event_with_no_tag = EventBuilder::new(Kind::ContactList, "", vec![])
.to_event(&keys)
.unwrap();

assert!(has_nos_agent(&event_matching));
assert!(!has_nos_agent(&event_not_matching));
assert!(!has_nos_agent(&event_with_no_tag));
}

async fn assert_follow_changes(contact_events: Vec<Event>, mut expected: Vec<FollowChange>) {
let follow_changes = get_follow_changes_from_contact_events(contact_events)
.await
24 changes: 24 additions & 0 deletions src/metrics.rs
Original file line number Diff line number Diff line change
@@ -9,6 +9,18 @@ pub fn contact_lists_processed() -> Counter {
metrics::counter!("contact_lists_processed")
}

pub fn already_seen_contact_lists() -> Counter {
metrics::counter!("already_seen_contact_lists")
}

pub fn sudden_follow_drops() -> Counter {
metrics::counter!("sudden_follow_drops")
}

pub fn nos_sudden_follow_drops() -> Counter {
metrics::counter!("nos_sudden_follow_drops")
}

pub fn follows() -> Counter {
metrics::counter!("follows")
}
@@ -66,6 +78,18 @@ pub fn setup_metrics() -> Result<PrometheusHandle, anyhow::Error> {
"contact_lists_processed",
"Number of contact lists processed"
);
describe_counter!(
"already_seen_contact_lists",
"Number of contact lists we have already processed"
);
describe_counter!(
"sudden_follow_drops",
"Number of contact lists that got a sudden drop on followees"
);
describe_counter!(
"nos_sudden_follow_drops",
"Number of contact lists that got a sudden drop on followees using the Nos agent"
);
describe_counter!("follows", "Number of follows");
describe_counter!("unfollows", "Number of unfollows");
describe_counter!("worker_lagged", "Number of times a worker lagged behind");