From 0ebe623dee88bce4db109ad9b93d13f7d096ef9f 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 | 8 ++--- 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, 73 insertions(+), 35 deletions(-) create mode 100644 autopush_rs/src/util/user_agent.rs diff --git a/autopush_rs/Cargo.lock b/autopush_rs/Cargo.lock index 3830a858..acc95015 100644 --- a/autopush_rs/Cargo.lock +++ b/autopush_rs/Cargo.lock @@ -20,7 +20,7 @@ name = "autopush" version = "0.1.0" dependencies = [ "bytes 0.4.5 (registry+https://github.com/rust-lang/crates.io-index)", - "cadence 0.12.1 (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.4.3 (registry+https://github.com/rust-lang/crates.io-index)", "error-chain 0.10.0 (registry+https://github.com/rust-lang/crates.io-index)", @@ -109,8 +109,8 @@ dependencies = [ [[package]] name = "cadence" -version = "0.12.1" -source = "registry+https://github.com/rust-lang/crates.io-index" +version = "0.12.2" +source = "git+https://github.com/pjenvey/cadence?branch=datadog-tags-initial#c929704ea787534f2002bed7ade168c9da775260" dependencies = [ "crossbeam 0.2.10 (registry+https://github.com/rust-lang/crates.io-index)", ] @@ -1004,7 +1004,7 @@ dependencies = [ "checksum bitflags 0.9.1 (registry+https://github.com/rust-lang/crates.io-index)" = "4efd02e230a02e18f92fc2735f44597385ed02ad8f831e7c1c1156ee5e1ab3a5" "checksum byteorder 1.1.0 (registry+https://github.com/rust-lang/crates.io-index)" = "ff81738b726f5d099632ceaffe7fb65b90212e8dce59d518729e7e8634032d3d" "checksum bytes 0.4.5 (registry+https://github.com/rust-lang/crates.io-index)" = "d828f97b58cc5de3e40c421d0cf2132d6b2da4ee0e11b8632fa838f0f9333ad6" -"checksum cadence 0.12.1 (registry+https://github.com/rust-lang/crates.io-index)" = "b48d3db3b2321bc9275c142fa2ddf04d2bcaa651a3b2acfaf8f4a1919402c88e" +"checksum cadence 0.12.2 (git+https://github.com/pjenvey/cadence?branch=datadog-tags-initial)" = "" "checksum cc 1.0.3 (registry+https://github.com/rust-lang/crates.io-index)" = "a9b13a57efd6b30ecd6598ebdb302cca617930b5470647570468a65d12ef9719" "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 9d73a607..0ad1a00a 100644 --- a/autopush_rs/Cargo.toml +++ b/autopush_rs/Cargo.toml @@ -8,7 +8,7 @@ crate-type = ["cdylib"] [dependencies] bytes = "0.4" -cadence = "0.12.1" +cadence = { git = "https://github.com/pjenvey/cadence", branch = "datadog-tags-initial"} chrono = "0.4" env_logger = { version = "0.4", default-features = false } error-chain = "0.10" diff --git a/autopush_rs/src/client.rs b/autopush_rs/src/client.rs index d83768dc..d5d95125 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..6a611b7f --- /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: &'static [&'static 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: &'static [&'static 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) +}