From 304852cf2b92626e382a43af5076ca4e9a225729 Mon Sep 17 00:00:00 2001 From: Brett Hoerner Date: Mon, 5 Feb 2024 11:48:49 -0700 Subject: [PATCH] Revert "Add proper e2e histrogram based on metadata created_at (#64)" (#66) --- Cargo.lock | 1 - hook-api/Cargo.toml | 1 - hook-api/src/handlers/webhook.rs | 11 +++---- hook-common/src/kafka_messages/mod.rs | 9 ++---- hook-common/src/webhook.rs | 7 ----- hook-janitor/src/fixtures/webhook_cleanup.sql | 30 +++++++++---------- hook-janitor/src/webhooks.rs | 2 -- hook-worker/src/worker.rs | 8 ----- 8 files changed, 22 insertions(+), 47 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 836810e..17b608c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -768,7 +768,6 @@ name = "hook-api" version = "0.1.0" dependencies = [ "axum", - "chrono", "envconfig", "eyre", "hook-common", diff --git a/hook-api/Cargo.toml b/hook-api/Cargo.toml index 5e3530e..96c897c 100644 --- a/hook-api/Cargo.toml +++ b/hook-api/Cargo.toml @@ -9,7 +9,6 @@ edition = "2021" axum = { workspace = true } envconfig = { workspace = true } eyre = { workspace = true } -chrono = { workspace = true } hook-common = { path = "../hook-common" } http-body-util = { workspace = true } metrics = { workspace = true } diff --git a/hook-api/src/handlers/webhook.rs b/hook-api/src/handlers/webhook.rs index 3712aa2..16ebc6d 100644 --- a/hook-api/src/handlers/webhook.rs +++ b/hook-api/src/handlers/webhook.rs @@ -1,13 +1,14 @@ use std::time::Instant; use axum::{extract::State, http::StatusCode, Json}; -use hook_common::pgqueue::{NewJob, PgQueue}; use hook_common::webhook::{WebhookJobMetadata, WebhookJobParameters}; -use serde::Serialize; use serde_derive::Deserialize; -use tracing::{debug, error}; use url::Url; +use hook_common::pgqueue::{NewJob, PgQueue}; +use serde::Serialize; +use tracing::{debug, error}; + const MAX_BODY_SIZE: usize = 1_000_000; #[derive(Serialize, Deserialize)] @@ -115,7 +116,6 @@ mod tests { http::{self, Request, StatusCode}, Router, }; - use chrono::Utc; use hook_common::pgqueue::PgQueue; use hook_common::webhook::{HttpMethod, WebhookJobParameters}; use http_body_util::BodyExt; @@ -153,7 +153,6 @@ mod tests { team_id: 1, plugin_id: 2, plugin_config_id: 3, - created_at: Utc::now(), }, max_attempts: 1, }) @@ -196,7 +195,6 @@ mod tests { team_id: 1, plugin_id: 2, plugin_config_id: 3, - created_at: Utc::now(), }, max_attempts: 1, }) @@ -285,7 +283,6 @@ mod tests { team_id: 1, plugin_id: 2, plugin_config_id: 3, - created_at: Utc::now(), }, max_attempts: 1, }) diff --git a/hook-common/src/kafka_messages/mod.rs b/hook-common/src/kafka_messages/mod.rs index a1de9d5..f548563 100644 --- a/hook-common/src/kafka_messages/mod.rs +++ b/hook-common/src/kafka_messages/mod.rs @@ -16,12 +16,9 @@ where D: Deserializer<'de>, { let formatted: String = Deserialize::deserialize(deserializer)?; - let datetime = match DateTime::parse_from_rfc3339(&formatted) { - Ok(d) => d.with_timezone(&Utc), - Err(_) => match NaiveDateTime::parse_from_str(&formatted, "%Y-%m-%d %H:%M:%S") { - Ok(d) => d.and_utc(), - Err(_) => return Err(serde::de::Error::custom("Invalid datetime format")), - }, + let datetime = match NaiveDateTime::parse_from_str(&formatted, "%Y-%m-%d %H:%M:%S") { + Ok(d) => d.and_utc(), + Err(_) => return Err(serde::de::Error::custom("Invalid datetime format")), }; Ok(datetime) diff --git a/hook-common/src/webhook.rs b/hook-common/src/webhook.rs index 4122c20..11e0285 100644 --- a/hook-common/src/webhook.rs +++ b/hook-common/src/webhook.rs @@ -3,11 +3,9 @@ use std::convert::From; use std::fmt; use std::str::FromStr; -use chrono::{DateTime, Utc}; use serde::{de::Visitor, Deserialize, Serialize}; use crate::kafka_messages::app_metrics; -use crate::kafka_messages::{deserialize_datetime, serialize_datetime}; use crate::pgqueue::PgQueueError; /// Supported HTTP methods for webhooks. @@ -137,11 +135,6 @@ pub struct WebhookJobMetadata { pub team_id: u32, pub plugin_id: i32, pub plugin_config_id: i32, - #[serde( - serialize_with = "serialize_datetime", - deserialize_with = "deserialize_datetime" - )] - pub created_at: DateTime, } /// An error originating during a Webhook Job invocation. diff --git a/hook-janitor/src/fixtures/webhook_cleanup.sql b/hook-janitor/src/fixtures/webhook_cleanup.sql index 5f2f6c1..5dfa827 100644 --- a/hook-janitor/src/fixtures/webhook_cleanup.sql +++ b/hook-janitor/src/fixtures/webhook_cleanup.sql @@ -13,7 +13,7 @@ VALUES -- team:1, plugin_config:2, completed in hour 20 ( NULL, - '{"team_id": 1, "plugin_id": 99, "plugin_config_id": 2, "created_at": "2023-02-05T16:35:06.650Z"}', + '{"team_id": 1, "plugin_id": 99, "plugin_config_id": 2}', '2023-12-19 20:01:18.799371+00', '2023-12-19 20:01:18.799371+00', '{}', @@ -24,7 +24,7 @@ VALUES -- team:1, plugin_config:2, completed in hour 20 (purposeful duplicate) ( NULL, - '{"team_id": 1, "plugin_id": 99, "plugin_config_id": 2, "created_at": "2023-02-05T16:35:06.650Z"}', + '{"team_id": 1, "plugin_id": 99, "plugin_config_id": 2}', '2023-12-19 20:01:18.799371+00', '2023-12-19 20:01:18.799371+00', '{}', @@ -35,7 +35,7 @@ VALUES -- team:1, plugin_config:2, completed in hour 21 (different hour) ( NULL, - '{"team_id": 1, "plugin_id": 99, "plugin_config_id": 2, "created_at": "2023-02-05T16:35:06.650Z"}', + '{"team_id": 1, "plugin_id": 99, "plugin_config_id": 2}', '2023-12-19 21:01:18.799371+00', '2023-12-19 21:01:18.799371+00', '{}', @@ -46,7 +46,7 @@ VALUES -- team:1, plugin_config:3, completed in hour 20 (different plugin_config) ( NULL, - '{"team_id": 1, "plugin_id": 99, "plugin_config_id": 3, "created_at": "2023-02-05T16:35:06.650Z"}', + '{"team_id": 1, "plugin_id": 99, "plugin_config_id": 3}', '2023-12-19 20:01:18.80335+00', '2023-12-19 20:01:18.80335+00', '{}', @@ -57,7 +57,7 @@ VALUES -- team:1, plugin_config:2, completed but in a different queue ( NULL, - '{"team_id": 1, "plugin_id": 99, "plugin_config_id": 2, "created_at": "2023-02-05T16:35:06.650Z"}', + '{"team_id": 1, "plugin_id": 99, "plugin_config_id": 2}', '2023-12-19 20:01:18.799371+00', '2023-12-19 20:01:18.799371+00', '{}', @@ -68,7 +68,7 @@ VALUES -- team:2, plugin_config:4, completed in hour 20 (different team) ( NULL, - '{"team_id": 2, "plugin_id": 99, "plugin_config_id": 4, "created_at": "2023-02-05T16:35:06.650Z"}', + '{"team_id": 2, "plugin_id": 99, "plugin_config_id": 4}', '2023-12-19 20:01:18.799371+00', '2023-12-19 20:01:18.799371+00', '{}', @@ -79,7 +79,7 @@ VALUES -- team:1, plugin_config:2, failed in hour 20 ( ARRAY ['{"type":"TimeoutError","details":{"error":{"name":"Timeout"}}}'::jsonb], - '{"team_id": 1, "plugin_id": 99, "plugin_config_id": 2, "created_at": "2023-02-05T16:35:06.650Z"}', + '{"team_id": 1, "plugin_id": 99, "plugin_config_id": 2}', '2023-12-19 20:01:18.799371+00', '2023-12-19 20:01:18.799371+00', '{}', @@ -90,7 +90,7 @@ VALUES -- team:1, plugin_config:2, failed in hour 20 (purposeful duplicate) ( ARRAY ['{"type":"TimeoutError","details":{"error":{"name":"Timeout"}}}'::jsonb], - '{"team_id": 1, "plugin_id": 99, "plugin_config_id": 2, "created_at": "2023-02-05T16:35:06.650Z"}', + '{"team_id": 1, "plugin_id": 99, "plugin_config_id": 2}', '2023-12-19 20:01:18.799371+00', '2023-12-19 20:01:18.799371+00', '{}', @@ -101,7 +101,7 @@ VALUES -- team:1, plugin_config:2, failed in hour 20 (different error) ( ARRAY ['{"type":"ConnectionError","details":{"error":{"name":"Connection Error"}}}'::jsonb], - '{"team_id": 1, "plugin_id": 99, "plugin_config_id": 2, "created_at": "2023-02-05T16:35:06.650Z"}', + '{"team_id": 1, "plugin_id": 99, "plugin_config_id": 2}', '2023-12-19 20:01:18.799371+00', '2023-12-19 20:01:18.799371+00', '{}', @@ -112,7 +112,7 @@ VALUES -- team:1, plugin_config:2, failed in hour 21 (different hour) ( ARRAY ['{"type":"TimeoutError","details":{"error":{"name":"Timeout"}}}'::jsonb], - '{"team_id": 1, "plugin_id": 99, "plugin_config_id": 2, "created_at": "2023-02-05T16:35:06.650Z"}', + '{"team_id": 1, "plugin_id": 99, "plugin_config_id": 2}', '2023-12-19 21:01:18.799371+00', '2023-12-19 21:01:18.799371+00', '{}', @@ -123,7 +123,7 @@ VALUES -- team:1, plugin_config:3, failed in hour 20 (different plugin_config) ( ARRAY ['{"type":"TimeoutError","details":{"error":{"name":"Timeout"}}}'::jsonb], - '{"team_id": 1, "plugin_id": 99, "plugin_config_id": 3, "created_at": "2023-02-05T16:35:06.650Z"}', + '{"team_id": 1, "plugin_id": 99, "plugin_config_id": 3}', '2023-12-19 20:01:18.799371+00', '2023-12-19 20:01:18.799371+00', '{}', @@ -134,7 +134,7 @@ VALUES -- team:1, plugin_config:2, failed but in a different queue ( ARRAY ['{"type":"TimeoutError","details":{"error":{"name":"Timeout"}}}'::jsonb], - '{"team_id": 1, "plugin_id": 99, "plugin_config_id": 2, "created_at": "2023-02-05T16:35:06.650Z"}', + '{"team_id": 1, "plugin_id": 99, "plugin_config_id": 2}', '2023-12-19 20:01:18.799371+00', '2023-12-19 20:01:18.799371+00', '{}', @@ -145,7 +145,7 @@ VALUES -- team:2, plugin_config:4, failed in hour 20 (purposeful duplicate) ( ARRAY ['{"type":"TimeoutError","details":{"error":{"name":"Timeout"}}}'::jsonb], - '{"team_id": 2, "plugin_id": 99, "plugin_config_id": 4, "created_at": "2023-02-05T16:35:06.650Z"}', + '{"team_id": 2, "plugin_id": 99, "plugin_config_id": 4}', '2023-12-19 20:01:18.799371+00', '2023-12-19 20:01:18.799371+00', '{}', @@ -156,7 +156,7 @@ VALUES -- team:1, plugin_config:2, available ( NULL, - '{"team_id": 1, "plugin_id": 99, "plugin_config_id": 2, "created_at": "2023-02-05T16:35:06.650Z"}', + '{"team_id": 1, "plugin_id": 99, "plugin_config_id": 2}', '2023-12-19 20:01:18.799371+00', '2023-12-19 20:01:18.799371+00', '{"body": "hello world", "headers": {}, "method": "POST", "url": "https://myhost/endpoint"}', @@ -167,7 +167,7 @@ VALUES -- team:1, plugin_config:2, running ( NULL, - '{"team_id": 1, "plugin_id": 99, "plugin_config_id": 2, "created_at": "2023-02-05T16:35:06.650Z"}', + '{"team_id": 1, "plugin_id": 99, "plugin_config_id": 2}', '2023-12-19 20:01:18.799371+00', now() - '1 hour' :: interval, '{}', diff --git a/hook-janitor/src/webhooks.rs b/hook-janitor/src/webhooks.rs index c1390a7..5cdf431 100644 --- a/hook-janitor/src/webhooks.rs +++ b/hook-janitor/src/webhooks.rs @@ -892,7 +892,6 @@ mod tests { team_id: 1, plugin_id: 2, plugin_config_id: 3, - created_at: Utc::now(), }; let new_job = NewJob::new(1, job_metadata, job_parameters, &"target"); queue.enqueue(new_job).await.expect("failed to enqueue job"); @@ -919,7 +918,6 @@ mod tests { team_id: 1, plugin_id: 2, plugin_config_id: 3, - created_at: Utc::now(), }; let new_job = NewJob::new(1, job_metadata, job_parameters, &"target"); queue.enqueue(new_job).await.expect("failed to enqueue job"); diff --git a/hook-worker/src/worker.rs b/hook-worker/src/worker.rs index 14edea8..c526c3f 100644 --- a/hook-worker/src/worker.rs +++ b/hook-worker/src/worker.rs @@ -2,7 +2,6 @@ use std::collections; use std::sync::Arc; use std::time; -use chrono::Utc; use hook_common::health::HealthHandle; use hook_common::{ pgqueue::{Job, PgJob, PgJobError, PgQueue, PgQueueError, PgQueueJob, PgTransactionJob}, @@ -265,10 +264,6 @@ async fn process_webhook_job( match send_result { Ok(_) => { - let end_to_end_duration = Utc::now() - webhook_job.metadata().created_at; - metrics::histogram!("webhook_jobs_end_to_end_duration_seconds", &labels) - .record((end_to_end_duration.num_milliseconds() as f64) / 1_000_f64); - webhook_job .complete() .await @@ -455,8 +450,6 @@ mod tests { // This is due to a long-standing cargo bug that reports imports and helper functions as unused. // See: https://github.com/rust-lang/rust/issues/46379. #[allow(unused_imports)] - use chrono::Utc; - #[allow(unused_imports)] use hook_common::health::HealthRegistry; #[allow(unused_imports)] use hook_common::pgqueue::{JobStatus, NewJob}; @@ -530,7 +523,6 @@ mod tests { team_id: 1, plugin_id: 2, plugin_config_id: 3, - created_at: Utc::now(), }; let registry = HealthRegistry::new("liveness"); let liveness = registry