update histogram types

bytedance · Nov 1, 2023 · 735728b · 735728b
1 parent f923c70
commit 735728b
Show file tree

Hide file tree

Showing 18 changed files with 305 additions and 79 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/g3bench/src/target/dns/stats/histogram.rs b/g3bench/src/target/dns/stats/histogram.rs
@@ -18,18 +18,18 @@ use std::time::Duration;
 
 use cadence::StatsdClient;
 
-use g3_histogram::{HistogramRecorder, SyncHistogram};
+use g3_histogram::{DurationHistogram, HistogramRecorder};
 use g3_types::ext::DurationExt;
 
 use crate::target::BenchHistogram;
 
 pub(crate) struct DnsHistogram {
-    total_time: SyncHistogram<u64>,
+    total_time: DurationHistogram<u64>,
 }
 
 impl DnsHistogram {
     pub(crate) fn new() -> (Self, DnsHistogramRecorder) {
-        let (h, r) = SyncHistogram::new(3).unwrap();
+        let (h, r) = DurationHistogram::new();
         (
             DnsHistogram { total_time: h },
             DnsHistogramRecorder { total_time: r },

diff --git a/g3bench/src/target/http/stats/histogram.rs b/g3bench/src/target/http/stats/histogram.rs
@@ -18,24 +18,24 @@ use std::time::Duration;
 
 use cadence::StatsdClient;
 
-use g3_histogram::{HistogramRecorder, SyncHistogram};
+use g3_histogram::{DurationHistogram, HistogramRecorder};
 use g3_types::ext::DurationExt;
 
 use crate::target::BenchHistogram;
 
 pub(crate) struct HttpHistogram {
-    send_hdr_time: SyncHistogram<u64>,
-    recv_hdr_time: SyncHistogram<u64>,
-    total_time: SyncHistogram<u64>,
-    conn_reuse_count: SyncHistogram<u64>,
+    send_hdr_time: DurationHistogram<u64>,
+    recv_hdr_time: DurationHistogram<u64>,
+    total_time: DurationHistogram<u64>,
+    conn_reuse_count: DurationHistogram<u64>,
 }
 
 impl HttpHistogram {
     pub(crate) fn new() -> (Self, HttpHistogramRecorder) {
-        let (send_hdr_time_h, send_hdr_time_r) = SyncHistogram::new(3).unwrap();
-        let (recv_hdr_time_h, recv_hdr_time_r) = SyncHistogram::new(3).unwrap();
-        let (total_time_h, total_time_r) = SyncHistogram::new(3).unwrap();
-        let (conn_reuse_count_h, conn_reuse_count_r) = SyncHistogram::new(3).unwrap();
+        let (send_hdr_time_h, send_hdr_time_r) = DurationHistogram::new();
+        let (recv_hdr_time_h, recv_hdr_time_r) = DurationHistogram::new();
+        let (total_time_h, total_time_r) = DurationHistogram::new();
+        let (conn_reuse_count_h, conn_reuse_count_r) = DurationHistogram::new();
         let h = HttpHistogram {
             send_hdr_time: send_hdr_time_h,
             recv_hdr_time: recv_hdr_time_h,

diff --git a/g3bench/src/target/keyless/cloudflare/stats/histogram.rs b/g3bench/src/target/keyless/cloudflare/stats/histogram.rs
@@ -18,20 +18,20 @@ use std::time::Duration;
 
 use cadence::StatsdClient;
 
-use g3_histogram::{HistogramRecorder, SyncHistogram};
+use g3_histogram::{DurationHistogram, HistogramRecorder};
 use g3_types::ext::DurationExt;
 
 use crate::target::BenchHistogram;
 
 pub(crate) struct KeylessHistogram {
-    total_time: SyncHistogram<u64>,
-    conn_reuse_count: SyncHistogram<u64>,
+    total_time: DurationHistogram<u64>,
+    conn_reuse_count: DurationHistogram<u64>,
 }
 
 impl KeylessHistogram {
     pub(crate) fn new() -> (Self, KeylessHistogramRecorder) {
-        let (total_time_h, total_time_r) = SyncHistogram::new(3).unwrap();
-        let (conn_reuse_count_h, conn_reuse_count_r) = SyncHistogram::new(3).unwrap();
+        let (total_time_h, total_time_r) = DurationHistogram::new();
+        let (conn_reuse_count_h, conn_reuse_count_r) = DurationHistogram::new();
         let h = KeylessHistogram {
             total_time: total_time_h,
             conn_reuse_count: conn_reuse_count_h,

diff --git a/g3bench/src/target/keyless/openssl/stats/histogram.rs b/g3bench/src/target/keyless/openssl/stats/histogram.rs
@@ -18,18 +18,18 @@ use std::time::Duration;
 
 use cadence::StatsdClient;
 
-use g3_histogram::{HistogramRecorder, SyncHistogram};
+use g3_histogram::{DurationHistogram, HistogramRecorder};
 use g3_types::ext::DurationExt;
 
 use crate::target::BenchHistogram;
 
 pub(crate) struct KeylessHistogram {
-    total_time: SyncHistogram<u64>,
+    total_time: DurationHistogram<u64>,
 }
 
 impl KeylessHistogram {
     pub(crate) fn new() -> (Self, KeylessHistogramRecorder) {
-        let (h, r) = SyncHistogram::new(3).unwrap();
+        let (h, r) = DurationHistogram::new();
         (
             KeylessHistogram { total_time: h },
             KeylessHistogramRecorder { total_time: r },

diff --git a/g3bench/src/target/ssl/stats/histogram.rs b/g3bench/src/target/ssl/stats/histogram.rs
@@ -18,18 +18,18 @@ use std::time::Duration;
 
 use cadence::StatsdClient;
 
-use g3_histogram::{HistogramRecorder, SyncHistogram};
+use g3_histogram::{DurationHistogram, HistogramRecorder};
 use g3_types::ext::DurationExt;
 
 use crate::target::BenchHistogram;
 
 pub(crate) struct SslHistogram {
-    total_time: SyncHistogram<u64>,
+    total_time: DurationHistogram<u64>,
 }
 
 impl SslHistogram {
     pub(crate) fn new() -> (Self, SslHistogramRecorder) {
-        let (h, r) = SyncHistogram::new(3).unwrap();
+        let (h, r) = DurationHistogram::new();
         (
             SslHistogram { total_time: h },
             SslHistogramRecorder { total_time: r },

diff --git a/g3keymess/Cargo.toml b/g3keymess/Cargo.toml
@@ -34,7 +34,7 @@ arc-swap.workspace = true
 serde_json.workspace = true
 g3-daemon = { workspace = true, features = ["register"] }
 g3-signal.workspace = true
-g3-yaml = { workspace = true, features = [] }
+g3-yaml = { workspace = true, features = ["histogram"] }
 g3-types = { workspace = true, features = [] }
 g3-socket.workspace = true
 g3-io-ext.workspace = true

diff --git a/g3keymess/src/config/server/mod.rs b/g3keymess/src/config/server/mod.rs
@@ -14,6 +14,7 @@
  * limitations under the License.
  */
 
+use std::collections::BTreeSet;
 use std::path::Path;
 use std::sync::Arc;
 use std::time::Duration;
@@ -23,6 +24,7 @@ use ascii::AsciiString;
 use slog::Logger;
 use yaml_rust::{yaml, Yaml};
 
+use g3_histogram::Quantile;
 use g3_types::metrics::{MetricsName, StaticMetricsTags};
 use g3_types::net::TcpListenConfig;
 use g3_yaml::{HybridParser, YamlDocPosition};
@@ -39,6 +41,7 @@ pub(crate) struct KeyServerConfig {
     pub(crate) listen: TcpListenConfig,
     pub(crate) multiplex_queue_depth: usize,
     pub(crate) request_read_timeout: Duration,
+    pub(crate) request_duration_quantile: BTreeSet<Quantile>,
     pub(crate) async_op_timeout: Duration,
     pub(crate) concurrency_limit: usize,
     pub(crate) extra_metrics_tags: Option<Arc<StaticMetricsTags>>,
@@ -53,6 +56,7 @@ impl KeyServerConfig {
             listen: TcpListenConfig::default(),
             multiplex_queue_depth: 0,
             request_read_timeout: Duration::from_millis(100),
+            request_duration_quantile: BTreeSet::new(),
             async_op_timeout: Duration::from_millis(10),
             concurrency_limit: 0,
             extra_metrics_tags: None,
@@ -111,6 +115,10 @@ impl KeyServerConfig {
                 self.request_read_timeout = g3_yaml::humanize::as_duration(v)?;
                 Ok(())
             }
+            "request_duration_quantile" => {
+                self.request_duration_quantile = g3_yaml::value::as_quantile_list(v)?;
+                Ok(())
+            }
             "async_op_timeout" => {
                 self.async_op_timeout = g3_yaml::humanize::as_duration(v)?;
                 Ok(())

diff --git a/g3keymess/src/serve/server.rs b/g3keymess/src/serve/server.rs
@@ -94,7 +94,8 @@ impl KeyServer {
     pub(crate) fn prepare_initial(config: KeyServerConfig) -> KeyServer {
         let server_stats = KeyServerStats::new(config.name());
         let listen_stats = ListenStats::new(config.name());
-        let (duration_recorder, duration_stats) = KeyServerDurationRecorder::new(config.name());
+        let (duration_recorder, duration_stats) =
+            KeyServerDurationRecorder::new(config.name(), &config.request_duration_quantile);
         let concurrency_limit = if config.concurrency_limit > 0 {
             Some(Arc::new(Semaphore::new(config.concurrency_limit)))
         } else {

diff --git a/g3keymess/src/serve/stats.rs b/g3keymess/src/serve/stats.rs
@@ -14,12 +14,13 @@
  * limitations under the License.
  */
 
+use std::collections::BTreeSet;
 use std::sync::atomic::{AtomicI32, AtomicIsize, AtomicU64, Ordering};
 use std::sync::Arc;
 
 use arc_swap::ArcSwapOption;
 
-use g3_histogram::{HistogramRecorder, HistogramStats, SyncHistogram};
+use g3_histogram::{DurationHistogram, HistogramRecorder, HistogramStats, Quantile};
 use g3_types::metrics::{MetricsName, StaticMetricsTags};
 use g3_types::stats::StatId;
 
@@ -275,14 +276,17 @@ pub(crate) struct KeyServerDurationRecorder {
 }
 
 impl KeyServerDurationRecorder {
-    pub(crate) fn new(name: &MetricsName) -> (KeyServerDurationRecorder, KeyServerDurationStats) {
-        let (ping_pong_r, ping_pong_s) = create_duration_pair();
-        let (rsa_decrypt_r, rsa_decrypt_s) = create_duration_pair();
-        let (rsa_sign_r, rsa_sign_s) = create_duration_pair();
-        let (rsa_pss_sign_r, rsa_pss_sign_s) = create_duration_pair();
-        let (ecdsa_sign_r, ecdsa_sign_s) = create_duration_pair();
-        let (ed25519_sign_r, ed25519_sign_s) = create_duration_pair();
-        let (_, noop_r) = SyncHistogram::new(3).unwrap();
+    pub(crate) fn new(
+        name: &MetricsName,
+        quantile: &BTreeSet<Quantile>,
+    ) -> (KeyServerDurationRecorder, KeyServerDurationStats) {
+        let (ping_pong_r, ping_pong_s) = create_duration_pair(quantile);
+        let (rsa_decrypt_r, rsa_decrypt_s) = create_duration_pair(quantile);
+        let (rsa_sign_r, rsa_sign_s) = create_duration_pair(quantile);
+        let (rsa_pss_sign_r, rsa_pss_sign_s) = create_duration_pair(quantile);
+        let (ecdsa_sign_r, ecdsa_sign_s) = create_duration_pair(quantile);
+        let (ed25519_sign_r, ed25519_sign_s) = create_duration_pair(quantile);
+        let (_, noop_r) = DurationHistogram::new();
 
         let r = KeyServerDurationRecorder {
             ping_pong: ping_pong_r,
@@ -309,9 +313,19 @@ impl KeyServerDurationRecorder {
     }
 }
 
-fn create_duration_pair() -> (Arc<HistogramRecorder<u64>>, Arc<HistogramStats>) {
-    let (h, r) = SyncHistogram::new(3).unwrap();
-    let stats = Arc::new(HistogramStats::default());
+fn create_duration_pair(
+    quantile: &BTreeSet<Quantile>,
+) -> (Arc<HistogramRecorder<u64>>, Arc<HistogramStats>) {
+    let (h, r) = DurationHistogram::new();
+    let stats = if quantile.is_empty() {
+        Arc::new(HistogramStats::default())
+    } else {
+        let mut stats = HistogramStats::new();
+        for q in quantile {
+            stats = stats.with_quantile(q.clone());
+        }
+        Arc::new(stats)
+    };
     let s = stats.clone();
     tokio::spawn(async move {
         let mut h = h;

diff --git a/lib/g3-histogram/Cargo.toml b/lib/g3-histogram/Cargo.toml
@@ -7,6 +7,7 @@ edition.workspace = true
 # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
 
 [dependencies]
+thiserror.workspace = true
 hdrhistogram.workspace = true
 tokio = { workspace = true, features = ["sync", "rt", "time"] }
 rust_decimal.workspace = true

diff --git a/lib/g3-histogram/src/lib.rs b/lib/g3-histogram/src/lib.rs
@@ -15,7 +15,10 @@
  */
 
 mod sync;
-pub use sync::{HistogramRecorder, SyncHistogram};
+pub use sync::{DurationHistogram, HistogramRecorder};
 
 mod stats;
 pub use stats::HistogramStats;
+
+mod quantile;
+pub use quantile::Quantile;