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

Revert "Add proper e2e histrogram based on metadata created_at (#64)" #66

Merged
merged 1 commit into from
Feb 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading