From 49fa63cde827190a08cab7452899dbcccc6bb5da Mon Sep 17 00:00:00 2001 From: Philip Jenvey Date: Wed, 24 Jan 2018 11:55:32 -0800 Subject: [PATCH] feat: add metric tags relies on my WIP cadence fork, see: https://github.com/tshlabs/cadence/issues/41 also move ua parsing into utils, adding in the setup of metrics' values Closes #1054 --- autopush_rs/Cargo.lock | 6 ++-- autopush_rs/Cargo.toml | 2 +- autopush_rs/src/client.rs | 50 +++++++++++++----------------- autopush_rs/src/util/mod.rs | 6 ++-- autopush_rs/src/util/user_agent.rs | 42 +++++++++++++++++++++++++ 5 files changed, 72 insertions(+), 34 deletions(-) create mode 100644 autopush_rs/src/util/user_agent.rs diff --git a/autopush_rs/Cargo.lock b/autopush_rs/Cargo.lock index 3692f71f..449b2218 100644 --- a/autopush_rs/Cargo.lock +++ b/autopush_rs/Cargo.lock @@ -21,7 +21,7 @@ name = "autopush" version = "0.1.0" dependencies = [ "bytes 0.4.6 (registry+https://github.com/rust-lang/crates.io-index)", - "cadence 0.12.2 (registry+https://github.com/rust-lang/crates.io-index)", + "cadence 0.12.2 (git+https://github.com/pjenvey/cadence?branch=datadog-tags-initial)", "chrono 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)", "env_logger 0.5.3 (registry+https://github.com/rust-lang/crates.io-index)", "error-chain 0.11.0 (registry+https://github.com/rust-lang/crates.io-index)", @@ -109,7 +109,7 @@ dependencies = [ [[package]] name = "cadence" version = "0.12.2" -source = "registry+https://github.com/rust-lang/crates.io-index" +source = "git+https://github.com/pjenvey/cadence?branch=datadog-tags-initial#acc6a4966f50c8fb670d65e7a5cf8df55cba88aa" dependencies = [ "crossbeam 0.2.12 (registry+https://github.com/rust-lang/crates.io-index)", ] @@ -1091,7 +1091,7 @@ dependencies = [ "checksum bitflags 1.0.1 (registry+https://github.com/rust-lang/crates.io-index)" = "b3c30d3802dfb7281680d6285f2ccdaa8c2d8fee41f93805dba5c4cf50dc23cf" "checksum byteorder 1.2.1 (registry+https://github.com/rust-lang/crates.io-index)" = "652805b7e73fada9d85e9a6682a4abd490cb52d96aeecc12e33a0de34dfd0d23" "checksum bytes 0.4.6 (registry+https://github.com/rust-lang/crates.io-index)" = "1b7db437d718977f6dc9b2e3fd6fc343c02ac6b899b73fdd2179163447bd9ce9" -"checksum cadence 0.12.2 (registry+https://github.com/rust-lang/crates.io-index)" = "7b8183488b4696bd166a383122be938b16a4382eb4b04d6a7dc47496cb0d3adf" +"checksum cadence 0.12.2 (git+https://github.com/pjenvey/cadence?branch=datadog-tags-initial)" = "" "checksum cc 1.0.4 (registry+https://github.com/rust-lang/crates.io-index)" = "deaf9ec656256bb25b404c51ef50097207b9cbb29c933d31f92cae5a8a0ffee0" "checksum cfg-if 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)" = "d4c819a1287eb618df47cc647173c5c4c66ba19d888a6e50d605672aed3140de" "checksum chrono 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)" = "7c20ebe0b2b08b0aeddba49c609fe7957ba2e33449882cb186a180bc60682fa9" diff --git a/autopush_rs/Cargo.toml b/autopush_rs/Cargo.toml index de648d37..9779351a 100644 --- a/autopush_rs/Cargo.toml +++ b/autopush_rs/Cargo.toml @@ -8,7 +8,7 @@ crate-type = ["cdylib"] [dependencies] bytes = "0.4.6" -cadence = "0.12.2" +cadence = { git = "https://github.com/pjenvey/cadence", branch = "datadog-tags-initial"} chrono = "0.4.0" env_logger = { version = "0.5.3", default-features = false } error-chain = "0.11.0" diff --git a/autopush_rs/src/client.rs b/autopush_rs/src/client.rs index d83768dc..a1615755 100644 --- a/autopush_rs/src/client.rs +++ b/autopush_rs/src/client.rs @@ -16,12 +16,13 @@ use futures::{Stream, Sink, Future, Poll, Async}; use tokio_core::reactor::Timeout; use time; use uuid::Uuid; -use woothee::parser::{Parser, WootheeResult}; +use woothee::parser::Parser; use call; use errors::*; use protocol::{ClientAck, ClientMessage, ServerMessage, ServerNotification, Notification}; use server::Server; +use util::parse_user_agent; pub struct RegisteredClient { pub uaid: Uuid, @@ -223,13 +224,12 @@ where if message.topic.is_some() { self.data.srv.metrics.incr("ua.notification.topic")?; } - // XXX: tags - self.data.srv.metrics.count( - "ua.message_data", - message.data.as_ref().map_or(0, |d| { - d.len() as i64 - }), - )?; + let mlen = message.data.as_ref().map_or(0, |d| d.len() as i64); + // XXX: not emitted for direct notifications (nor was it in websocket.py) + self.data.srv.metrics + .count_with_tags("ua.message_data", mlen) + .with_tag("source", "Stored") + .send()?; ClientState::FinishSend( Some(ServerMessage::Notification(message)), Some(Box::new(ClientState::SendMessages(if messages.len() > 0 { @@ -591,7 +591,7 @@ where uaid_reset: reset_uaid, existing_uaid: check_storage, connection_type: String::from("webpush"), - host: String::from("unknown"), + host: self.host.clone(), direct_acked: 0, direct_storage: 0, stored_retrieved: 0, @@ -719,8 +719,16 @@ where let now = time::precise_time_ns() / 1000; let elapsed = now - webpush.connected_at; - // XXX: tags - self.srv.metrics.time("ua.connection.lifespan", elapsed).ok(); + + let parser = Parser::new(); + let (ua_result, metrics_os, metrics_browser) = parse_user_agent(&parser, &self.user_agent); + self.srv.metrics + .time_with_tags("ua.connection.lifespan", elapsed) + .with_tag("ua_os_family", metrics_os) + .with_tag("ua_browser_family", metrics_browser) + .with_tag("host", &webpush.stats.host) + .send() + .ok(); // If there's direct unack'd messages, they need to be saved out without blocking // here @@ -743,27 +751,13 @@ where ) } - // Parse the user-agent string - let parser = Parser::new(); - let ua_result = parser - .parse(&self.user_agent) - .unwrap_or_else(|| WootheeResult { - name: "", - category: "", - os: "", - os_version: "".to_string(), - browser_type: "", - version: "".to_string(), - vendor: "", - }); - // Log out the final stats message info!("Session"; - "uaid_hash" => stats.uaid.as_str(), + "uaid_hash" => &stats.uaid, "uaid_reset" => stats.uaid_reset, "existing_uaid" => stats.existing_uaid, - "connection_type" => stats.connection_type.as_str(), - "host" => self.host.clone(), + "connection_type" => &stats.connection_type, + "host" => &stats.host, "ua_name" => ua_result.name, "ua_os_family" => ua_result.os, "ua_os_ver" => ua_result.os_version, diff --git a/autopush_rs/src/util/mod.rs b/autopush_rs/src/util/mod.rs index 5bd340b5..c6e501bb 100644 --- a/autopush_rs/src/util/mod.rs +++ b/autopush_rs/src/util/mod.rs @@ -13,12 +13,14 @@ use tokio_core::reactor::{Handle, Timeout}; use errors::*; -mod send_all; -mod rc; mod autojson; +mod rc; +mod send_all; +mod user_agent; pub use self::send_all::MySendAll; pub use self::rc::RcObject; +pub use self::user_agent::parse_user_agent; /// Convenience future to time out the resolution of `f` provided within the /// duration provided. diff --git a/autopush_rs/src/util/user_agent.rs b/autopush_rs/src/util/user_agent.rs new file mode 100644 index 00000000..7c2ca2ce --- /dev/null +++ b/autopush_rs/src/util/user_agent.rs @@ -0,0 +1,42 @@ +use woothee::parser::{Parser, WootheeResult}; + +// List of valid user-agent attributes to keep, anything not in this +// list is considered 'Other'. We log the user-agent on connect always +// to retain the full string, but for DD more tags are expensive so we +// limit to these. +const VALID_UA_BROWSER: &[&str] = &["Chrome", "Firefox", "Safari", "Opera"]; + +// See dataset.rs in https://github.com/woothee/woothee-rust for the +// full list (WootheeResult's 'os' field may fall back to its 'name' +// field). Windows has many values and we only care that its Windows +const VALID_UA_OS: &[&str] = &["Firefox OS", "Linux", "Mac OSX"]; + +pub fn parse_user_agent<'a>( + parser: &'a Parser, + agent: &str, +) -> (WootheeResult<'a>, &'a str, &'a str) { + let wresult = parser.parse(&agent).unwrap_or_else(|| WootheeResult { + name: "", + category: "", + os: "", + os_version: "".to_string(), + browser_type: "", + version: "".to_string(), + vendor: "", + }); + + // Determine a base os/browser for metrics' tags + let metrics_os = if wresult.os.starts_with("Windows") { + "Windows" + } else if VALID_UA_OS.contains(&wresult.os) { + wresult.os + } else { + "Other" + }; + let metrics_browser = if VALID_UA_BROWSER.contains(&wresult.name) { + wresult.name + } else { + "Other" + }; + (wresult, metrics_os, metrics_browser) +}