Skip to content

Commit

Permalink
bug: Restore hawk error metrics (#1033)
Browse files Browse the repository at this point in the history
* chore: clippy updates for rust 1.51

Closes: #1032

* bug: Restore hawk error metrics

(Includes clippy changes for rust 1.51)

Hawk errors should be returned as metrics. During the middleware purge, this broke. This PR includes metric reporting into the sentry handler.

Closes #812.
  • Loading branch information
jrconlin committed Apr 5, 2021
1 parent 487ac11 commit f795eb0
Show file tree
Hide file tree
Showing 8 changed files with 14 additions and 23 deletions.
2 changes: 1 addition & 1 deletion Dockerfile
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
FROM rust:1.50-buster as builder
FROM rust:1.51-buster as builder
WORKDIR /app
ADD . /app
ENV PATH=$PATH:/root/.cargo/bin
Expand Down
2 changes: 1 addition & 1 deletion src/db/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ impl DbError {

pub fn metric_label(&self) -> Option<String> {
match self.inner.get_context() {
DbErrorKind::Conflict => Some("request.error.db.conflict".to_owned()),
DbErrorKind::Conflict => Some("storage.conflict".to_owned()),
_ => None,
}
}
Expand Down
8 changes: 0 additions & 8 deletions src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,6 @@ use serde::{
};

use crate::db::error::{DbError, DbErrorKind};
use crate::server::metrics::Metrics;
use crate::server::ServerState;
use crate::web::error::{HawkError, ValidationError, ValidationErrorKind};
use crate::web::extractors::RequestErrorLocation;

Expand Down Expand Up @@ -137,12 +135,6 @@ impl ApiError {
self.kind().metric_label().is_none()
}

pub fn on_response(&self, state: &ServerState) {
if self.is_conflict() {
Metrics::from(state).incr("storage.confict")
}
}

fn weave_error_code(&self) -> WeaveError {
match self.kind() {
ApiErrorKind::Validation(ver) => match ver.kind() {
Expand Down
2 changes: 0 additions & 2 deletions src/server/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -250,9 +250,7 @@ async fn test_endpoint_with_body(
.call(req)
.await
.expect("Could not get sresponse in test_endpoint_with_body");
dbg!("got response", sresponse.response().status());
assert!(sresponse.response().status().is_success());
dbg!("all good");
test::read_body(sresponse).await
}

Expand Down
6 changes: 1 addition & 5 deletions src/web/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,7 @@ impl HawkError {
}

pub fn is_reportable(&self) -> bool {
matches!(
&self.kind(),
HawkErrorKind::TruncatedId | HawkErrorKind::Parse(_)
)
matches!(&self.kind(), HawkErrorKind::TruncatedId)
}

pub fn metric_label(&self) -> Option<String> {
Expand Down Expand Up @@ -207,7 +204,6 @@ impl Serialize for ValidationErrorKind {
S: Serializer,
{
let mut seq = serializer.serialize_seq(None)?;

match *self {
ValidationErrorKind::FromDetails(
ref description,
Expand Down
1 change: 0 additions & 1 deletion src/web/middleware/rejectua.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@ where
}
}
#[allow(clippy::clippy::upper_case_acronyms)]

pub struct RejectUAMiddleware<S> {
service: S,
}
Expand Down
15 changes: 11 additions & 4 deletions src/web/middleware/sentry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use sentry::protocol::Event;
use std::task::Poll;

use crate::error::ApiError;
use crate::server::ServerState;
use crate::server::{metrics::Metrics, ServerState};
use crate::web::tags::Tags;

pub struct SentryWrapper;
Expand Down Expand Up @@ -100,6 +100,11 @@ where
fn call(&mut self, sreq: ServiceRequest) -> Self::Future {
let mut tags = Tags::from(sreq.head());
sreq.extensions_mut().insert(tags.clone());
let metrics = if let Some(state) = sreq.app_data::<Data<ServerState>>() {
Some(Metrics::from(state.get_ref()))
} else {
None
};

Box::pin(self.service.call(sreq).and_then(move |mut sresp| {
// handed an actix_error::error::Error;
Expand Down Expand Up @@ -151,9 +156,11 @@ where
}
Some(e) => {
if let Some(apie) = e.as_error::<ApiError>() {
if let Some(state) = sresp.request().app_data::<Data<ServerState>>() {
apie.on_response(state.as_ref());
};
if let Some(metrics) = metrics {
if let Some(label) = apie.kind().metric_label() {
metrics.incr(&label);
}
}
if !apie.is_reportable() {
trace!("Sentry: Not reporting error: {:?}", apie);
return future::ok(sresp);
Expand Down
1 change: 0 additions & 1 deletion src/web/tags.rs
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,6 @@ impl From<Tags> for BTreeMap<String, String> {
for (k, v) in tags.tags {
result.insert(k.clone(), v.clone());
}

result
}
}
Expand Down

0 comments on commit f795eb0

Please sign in to comment.