Skip to content

Commit

Permalink
bug: fix Tokenserver metrics (#1218)
Browse files Browse the repository at this point in the history
Closes #1214
  • Loading branch information
ethowitz authored Mar 16, 2022
1 parent 0e9b0f6 commit d2dc006
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 22 deletions.
42 changes: 24 additions & 18 deletions src/server/metrics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,9 @@ pub struct MetricTimer {

#[derive(Debug, Clone)]
pub struct Metrics {
client: Option<StatsdClient>,
tags: Option<Tags>,
timer: Option<MetricTimer>,
pub client: Option<StatsdClient>,
pub tags: Option<Tags>,
pub timer: Option<MetricTimer>,
}

impl Drop for Metrics {
Expand Down Expand Up @@ -63,21 +63,27 @@ impl FromRequest for Metrics {
type Future = Ready<Result<Self, Self::Error>>;

fn from_request(req: &HttpRequest, _: &mut Payload) -> Self::Future {
let exts = req.extensions();
let def_tags = Tags::from(req.head());
let tags = exts.get::<Tags>().unwrap_or(&def_tags);

future::ok(Metrics {
client: match req.app_data::<Data<ServerState>>() {
Some(v) => Some(*v.metrics.clone()),
None => {
warn!("⚠️ metric error: No App State");
None
}
},
tags: Some(tags.clone()),
timer: None,
})
future::ok(metrics_from_request(
req,
req.app_data::<Data<ServerState>>()
.map(|state| state.metrics.clone()),
))
}
}

pub fn metrics_from_request(req: &HttpRequest, client: Option<Box<StatsdClient>>) -> Metrics {
let exts = req.extensions();
let def_tags = Tags::from(req.head());
let tags = exts.get::<Tags>().unwrap_or(&def_tags);

if client.is_none() {
warn!("⚠️ metric error: No App State");
}

Metrics {
client: client.as_deref().cloned(),
tags: Some(tags.clone()),
timer: None,
}
}

Expand Down
28 changes: 24 additions & 4 deletions src/tokenserver/extractors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use actix_web::{
Error, FromRequest, HttpRequest,
};
use actix_web_httpauth::extractors::bearer::BearerAuth;
use futures::future::LocalBoxFuture;
use futures::future::{self, LocalBoxFuture, Ready};
use hmac::{Hmac, Mac, NewMac};
use lazy_static::lazy_static;
use regex::Regex;
Expand All @@ -22,8 +22,8 @@ use sha2::Sha256;
use super::db::{models::Db, params, pool::DbPool, results};
use super::error::{ErrorLocation, TokenserverError};
use super::support::TokenData;
use super::{LogItemsMutator, NodeType, ServerState};
use crate::server::metrics::Metrics;
use super::{LogItemsMutator, NodeType, ServerState, TokenserverMetrics};
use crate::server::metrics;
use crate::settings::Secrets;

lazy_static! {
Expand Down Expand Up @@ -329,7 +329,7 @@ impl FromRequest for TokenData {
let state = get_server_state(&req)?.as_ref().as_ref().unwrap();
let oauth_verifier = state.oauth_verifier.clone();

let mut metrics = Metrics::extract(&req).await?;
let TokenserverMetrics(mut metrics) = TokenserverMetrics::extract(&req).await?;
metrics.start_timer("tokenserver.oauth_token_verification", None);

web::block(move || oauth_verifier.verify_token(auth.token()))
Expand Down Expand Up @@ -419,6 +419,26 @@ impl FromRequest for KeyId {
}
}

impl FromRequest for TokenserverMetrics {
type Config = ();
type Error = Error;
type Future = Ready<Result<Self, Self::Error>>;

fn from_request(req: &HttpRequest, _payload: &mut Payload) -> Self::Future {
let state = match get_server_state(req) {
// XXX: Tokenserver state will no longer be an Option once the Tokenserver
// code is rolled out, so we will eventually be able to remove this unwrap().
Ok(state) => state.as_ref().as_ref().unwrap(),
Err(e) => return future::err(e),
};

future::ok(TokenserverMetrics(metrics::metrics_from_request(
req,
Some(state.metrics.clone()),
)))
}
}

fn get_server_state(req: &HttpRequest) -> Result<&Data<Option<ServerState>>, Error> {
req.app_data::<Data<Option<ServerState>>>().ok_or_else(|| {
error!("⚠️ Could not load the app state");
Expand Down
2 changes: 2 additions & 0 deletions src/tokenserver/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,8 @@ impl ServerState {
}
}

pub struct TokenserverMetrics(Metrics);

#[derive(Clone, Copy, Debug, Deserialize, PartialEq, Serialize)]
pub enum NodeType {
#[serde(rename = "mysql")]
Expand Down

0 comments on commit d2dc006

Please sign in to comment.