Skip to content
This repository has been archived by the owner on Feb 8, 2024. It is now read-only.

Commit

Permalink
Revert "Add proper e2e histrogram based on metadata created_at (#64)" (
Browse files Browse the repository at this point in the history
  • Loading branch information
bretthoerner authored Feb 5, 2024
1 parent 615c61d commit 304852c
Show file tree
Hide file tree
Showing 8 changed files with 22 additions and 47 deletions.
1 change: 0 additions & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion hook-api/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand Down
11 changes: 4 additions & 7 deletions hook-api/src/handlers/webhook.rs
Original file line number Diff line number Diff line change
@@ -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)]
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -153,7 +153,6 @@ mod tests {
team_id: 1,
plugin_id: 2,
plugin_config_id: 3,
created_at: Utc::now(),
},
max_attempts: 1,
})
Expand Down Expand Up @@ -196,7 +195,6 @@ mod tests {
team_id: 1,
plugin_id: 2,
plugin_config_id: 3,
created_at: Utc::now(),
},
max_attempts: 1,
})
Expand Down Expand Up @@ -285,7 +283,6 @@ mod tests {
team_id: 1,
plugin_id: 2,
plugin_config_id: 3,
created_at: Utc::now(),
},
max_attempts: 1,
})
Expand Down
9 changes: 3 additions & 6 deletions hook-common/src/kafka_messages/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
7 changes: 0 additions & 7 deletions hook-common/src/webhook.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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<Utc>,
}

/// An error originating during a Webhook Job invocation.
Expand Down
30 changes: 15 additions & 15 deletions hook-janitor/src/fixtures/webhook_cleanup.sql
Original file line number Diff line number Diff line change
Expand Up @@ -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',
'{}',
Expand All @@ -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',
'{}',
Expand All @@ -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',
'{}',
Expand All @@ -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',
'{}',
Expand All @@ -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',
'{}',
Expand All @@ -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',
'{}',
Expand All @@ -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',
'{}',
Expand All @@ -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',
'{}',
Expand All @@ -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',
'{}',
Expand All @@ -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',
'{}',
Expand All @@ -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',
'{}',
Expand All @@ -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',
'{}',
Expand All @@ -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',
'{}',
Expand All @@ -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"}',
Expand All @@ -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,
'{}',
Expand Down
2 changes: 0 additions & 2 deletions hook-janitor/src/webhooks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand All @@ -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");
Expand Down
8 changes: 0 additions & 8 deletions hook-worker/src/worker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down Expand Up @@ -265,10 +264,6 @@ async fn process_webhook_job<W: WebhookJob>(

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
Expand Down Expand Up @@ -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};
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 304852c

Please sign in to comment.