Make metrics collection optional/faster #1147

QuerthDP · 2024-12-10T13:28:48Z

This patch contains:

Implementation of lock-free histogram with hot and cold bucket pools
Introduction of crate feature "metrics" to make using them optional
Functionality to gather latency statistics via histogram snapshots
Implementation of rates of queries per second based on cpp-driver implementation.
Initial implementation of connection metrics (changes required after review)

Fixes: #330

Pre-review checklist

I have split my patch into logically separate commits.
All commit messages clearly explain what they change and why.
I added relevant tests for new features and bug fixes.
All commits compile, pass static checks and pass test.
PR description sums up the changes and reasons why they should be introduced.
I have provided docstrings for the public items that I want to introduce.
I have adjusted the documentation in ./docs/source/.
I added appropriate Fixes: annotations to PR description.

github-actions · 2024-12-10T13:35:03Z

cargo semver-checks detected some API incompatibilities in this PR.
Checked commit: 4392cb8

See the following report for details:

cargo semver-checks output

./scripts/semver-checks.sh --baseline-rev f9f0940dec71b0c6762d813af2da6b4a2578058e
+ cargo semver-checks -p scylla -p scylla-cql --baseline-rev f9f0940dec71b0c6762d813af2da6b4a2578058e
     Cloning f9f0940dec71b0c6762d813af2da6b4a2578058e
    Building scylla v0.15.0 (current)
       Built [  23.383s] (current)
     Parsing scylla v0.15.0 (current)
      Parsed [   0.053s] (current)
    Building scylla v0.15.0 (baseline)
       Built [  22.814s] (baseline)
     Parsing scylla v0.15.0 (baseline)
      Parsed [   0.049s] (baseline)
    Checking scylla v0.15.0 -> v0.15.0 (no change)
     Checked [   0.107s] 107 checks: 106 pass, 1 fail, 0 warn, 0 skip

--- failure struct_with_no_pub_fields_changed_type: public API struct with no public fields is no longer a struct ---

Description:
A struct without pub fields became an enum or union, breaking pattern matching.
        ref: https://internals.rust-lang.org/t/rest-patterns-foo-should-match-non-struct-types/21607
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.38.0/src/lints/struct_with_no_pub_fields_changed_type.ron

Failed in:
  struct scylla::observability::metrics::MetricsError became enum in file /home/runner/work/scylla-rust-driver/scylla-rust-driver/scylla/src/observability/metrics.rs:10

     Summary semver requires new major version: 1 major and 0 minor checks failed
    Finished [  47.407s] scylla
    Building scylla-cql v0.4.0 (current)
       Built [  11.449s] (current)
     Parsing scylla-cql v0.4.0 (current)
      Parsed [   0.034s] (current)
    Building scylla-cql v0.4.0 (baseline)
       Built [  11.431s] (baseline)
     Parsing scylla-cql v0.4.0 (baseline)
      Parsed [   0.033s] (baseline)
    Checking scylla-cql v0.4.0 -> v0.4.0 (no change)
     Checked [   0.112s] 107 checks: 107 pass, 0 skip
     Summary no semver update required
    Finished [  24.010s] scylla-cql
make: *** [Makefile:61: semver-rev] Error 1

QuerthDP · 2025-01-13T12:43:27Z

Changelog:

rebased on main

muzarski

I still need to review commits that introduce Meter and connection metrics. Posting this review early, because there are some matters to discuss.

muzarski · 2025-01-15T12:46:06Z

scylla/src/observability/lock_free_histogram.rs

+#[derive(Error, Debug, PartialEq)]
+pub enum LFError {
+    #[error("invalid use of histogram")]
+    HistogramErr(#[from] histogram::Error),
+    #[error("could not lock the snapshot mutex")]
+    Mutex,
+}


Please add a docstring. Even the simplest one - mentioning that it indicates some failure during LockFreeHistogram's operation

I don't like the name. Since this is a public type, the error name should be more descriptive (current version would be fine for driver's internal error type). I suggest a verbose name such as LockFreeHistogramError (WDYT @wprzytula ?)

The additional context is not displayed for the variants. You can display it via {0}, such as #[error("Invalid use of histogram: {0}")]. Also, please begin the error messages with capital letter (this is a convention we currently use in the driver).

HistogramErr -> HistogramError (variant name)

HistogramErr variant currently has a type from a pre-1.0.0 crate. We need to think what to do with this. Corresponding issue: Remove types from pre-1.0 crates from the public API, or hide them behind features #771. I see that later in this PR, you introuce a metrics feature, so I assume that this error type will be hidden behind it as well. Is it OK if feature name does not directly correspond to the unstable crate's name? In this case, the unstable crate is histogram, and feature name is metrics. I'm not sure how this interacts with API stability. cc: @piodul @Lorak-mmk

scylla/src/observability/lock_free_histogram.rs

muzarski · 2025-01-15T12:49:50Z

scylla/src/observability/lock_free_histogram.rs

+    Mutex,
+}
+
+pub struct LockFreeHistogram {


This needs a docstring as well. Not only this is a public type (does it need to be public?), but its methods contain some very complex logic, which could be briefly explained here.

Also, it would be nice to mention the motivation behind this struct. I understand the logic (after reviewing the methods), but I still don't quite get WHY we need it. Why can't we just use the AtomicHistogram from histogram crate? Are there any races that can occur without this additional layer of synchronization?

Okay, so there are a couple of things to mention here, but I'll start from the beginning.

In the issue description here it is noted that no suitable solution for the problem was found on crates.io (I believe AtomicHistogram already existed by the time of writing of that issue), so I assumed it must have had a flaw. And, as it turned out, it did.

I highly recommend reading through the issue I opened up on the crate histogram repo, where I explain all of the motivation in detail, but in short: the .load() method has no synchronisation with increments, which causes a logical race (the state of the loaded histogram is dependent on the speed at which it is loaded).

The idea of some sort of a lock-free algorithm was also proposed in the "Make metrics optional" issue, along with sharding, which I considered potentially harder to implement, thus I went with a lock-free algorithm.

However (!), the LockFreeHistogram's implementation comes with potential drawbacks in terms of performance (in comparison to AtomicHistogram, not a global mutex) due to global atomic counters accessed upon each bucket increment.

I haven't managed to run any benchmarks in this regard, nor do I have concrete examples of cases when AtomicHistogram's implementation yields a very significant error in results (though I did come up with some ideas and calculations; they can be found on my linked issue on crate histogram's repo). Therefore, the decision of which histogram implementation to incorporate into this driver is up to you. I've just provided a safe alternative and done some research.

Also, should you choose to go with AtomicHistogram, the change will be rather effortless, as I maintained the API schema used in crate histogram for my implementation.

Thank you for the explanation! I think part of this deserves to be put in the docstring.

Also, should you choose to go with AtomicHistogram, the change will be rather effortless, as I maintained the API schema used in crate histogram for my implementation.

This is great. Also, please unpub (pub(crate)) the LockFreeHistogram and its methods. Only then will we be 100% sure that if we ever decide to drop/modify LockFreeHistogram, such change will not be API breaking.

Which histogram implementation we want to incorporate to the driver? Well, this needs to be discussed. I'd wait until @Lorak-mmk and @wprzytula review the code.

I unpubbed LockFreeHistogram and its methods, but now I can see it might be difficult to make such a change completely non-API-breaking. That's because LockFreeHistogramError is propagated as MetricsError's cause and thus needs to be pub. Upon change to AtomicHistogram this error struct would be removed entirely.

I'm not yet sure how we could go around this issue.

Ah, good catch. There are some workarounds for this, however. For example, we could always hide underlying LockFreeHistogramError under Arc<dyn Error>. I wouldn't worry about this now. Let's wait for others to join the discussion.

For what it's worth, while .load() is not atomic wrt concurrent increments into the histogram. I'd still consider using the AtomicHistogram which is used in metrics for both rpc-perf and Pelikan.

As I said in the issue opened in rustcommon, I'm not sure that the potential skew here would be meaningful. But I do welcome some concrete details if such skew does prove to be meaningful.

The TLDR is I'm not sure how much it matters whether it's on one side or the other of loading a histogram. If you envision periodically snapshotting the histogram, it seems you have to accept that the latency is already being recorded at the tail end of the event. Imagine a request that takes a long time, that latency gets incremented after the fact, when the service might be back to responding quickly.

My feeling is that ultimately this is all an approximation and I've found the AtomicHistogram as in the histogram crate to be satisfactory for projects I work on.

scylla/src/observability/lock_free_histogram.rs

muzarski · 2025-01-15T12:56:43Z

scylla/src/observability/lock_free_histogram.rs

+impl Default for LockFreeHistogram {
+    fn default() -> Self {
+        // Config: 64ms error, values in range [0ms, ~262_000ms].
+        // Size: (2^13 + 5 * 2^12) * 8B * 2 ~= 450kB.
+        let grouping_power = 12;
+        let max_value_power = 18;
+        LockFreeHistogram::new(grouping_power, max_value_power)
+    }
+}


Where are these defaults taken from?

Since the histogram crate no longer provides defaults, I had to come up with some choice here. It was my best guess at what might be needed, though that is obviously to be discussed and modified if needed.

You may find our calculator useful while evaluating what parameters are appropriate: https://observablehq.com/@iopsystems/h2-histogram

muzarski · 2025-01-15T12:57:59Z

scylla/src/observability/metrics.rs

+#[derive(Error, Debug, PartialEq)]
+pub enum MetricsError {
+    #[error("lock-free histogram error")]
+    LFHistogramErr(#[from] LFError),
+    #[error("histogram error")]
+    HistogramErr(#[from] histogram::Error),
+    #[error("histogram is empty")]
+    Empty,


ditto: same comments as in regards to LFError

I know that previous MetricsError was not documented, but we will appreciate this change as a bonus :). This will make a progress towards #519

muzarski · 2025-01-15T13:12:42Z

scylla/src/observability/metrics.rs

+
+    // Metric implementations
+
+    fn mean(h: Histogram) -> Result<u64, MetricsError> {


I wonder why they decided to remove Histogram::mean() method

The histogram has no way of providing a true mean. Do we use the lower or upper end of the bucket range while calculating? Somewhere in the middle? It seems more appropriate to let the caller decide how they want to deal with this detail. Same when determining a percentile, the best we can do is return the Bucket where the percentile falls into its range. It may depend on your use-case on what value to report. Previous assumptions of over-reporting latencies by using the upper-edge of the bucket might not be appropriate for all use-cases.

examples/Cargo.toml

Implement a lock-free histogram with hot and cold bucket pools. Implementation inspired by Prometheus library in Go and the histogram crate. This commit also updates crate histogram dependency from 0.6.9 to 0.11.1 for atomic functionalities and cleaner error handling (since the update introduced breaking changes to crate histogram's API, it was merged in one commit with the new, lock-free histogram implementation).

Implement metrics making use of Lock-Free Histogram to measure query latencies. Added metrics are provided by the Snapshot structure containing metrics such as: min, max, mean, median, standard deviation and various percentiles. Co-authored-by: NikodemGapski <[email protected]>

Implement rates similar to those available in cpp-driver. [CPP-Driver implementation](https://github.com/scylladb/cpp-driver/blob/9d6b05c9d4ebd0a6d7006af4df3e33fcdf956eeb/src/metrics.hpp#L39C1-L252C5)

Implement gathering of connection metrics like total number of active connections, connection timeouts and request timeouts. This implementation seems to need thorough refactoring due to improper passing of metrics to the PoolRefiller.

Add metrics crate feature which enables usage and gathering of metrics. Therefore everyone willing to use metrics in their code is required to add metrics feature in their Cargo.toml file or compile otherwise with --features metrics flag. Additionally, add a CI step with cargo checks for this feature.

Inform that metrics may now only be used under crate feature 'metrics'. Mention new metrics in documentation and show an example how to collect them. Adjust examples to include new metrics.

wprzytula · 2025-01-17T10:48:44Z

scylla/src/observability/lock_free_histogram.rs

+/// This file was inspired by a lock-free histogram implementation
+/// from the Prometheus library in Go.
+/// https://github.com/prometheus/client_golang/blob/main/prometheus/histogram.go
+/// Note: in the current implementation, the histogram *may* incur a data race
+/// after (1 << 63) increments (which is quite a lot).


🔧 IIUC, this is intended to be a module docstring. In such case, this should use //! instead of ///. Also, let's leave an empty line after the module docstring.

wprzytula · 2025-01-17T10:52:25Z

scylla/src/observability/lock_free_histogram.rs

+    hot_idx_and_count: AtomicU64,
+    /// Finished observations for each bucket pool.
+    pool_counts: [AtomicU64; 2],
+    /// Two histgorams in a pool: hot and cold.


⛏️ typo: histgorams

wprzytula · 2025-01-17T12:49:19Z

scylla/src/observability/lock_free_histogram.rs

+impl std::fmt::Debug for LockFreeHistogram {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        let h0 = self.histogram_pool[0].load();
+        let h1 = self.histogram_pool[1].load();
+        write!(f,
+            "LockFreeHistogram {{ hot_idx_and_count: {:?}, pool_counts: {:?}, histogram_pool: {:?}, snapshot_mutex: {:?} }}",
+            self.hot_idx_and_count,
+            self.pool_counts,
+            [h0, h1],
+            self.snapshot_mutex,
+        )
+    }
+}


🔧 I think [debug_struct](https://doc.rust-lang.org/std/fmt/struct.Formatter.html#method.debug_struct) could aid here.

wprzytula · 2025-01-17T12:50:16Z

scylla/src/observability/lock_free_histogram.rs

+        // Observer thread lambda.
+        let h_1 = h.clone();
+        let fin_s_1 = finished_snapshots.clone();
+        let observer = move || {
+            while fin_s_1.load(Ordering::Relaxed) < snapshot_count {
+                h_1.increment(0).unwrap();
+            }
+        };
+
+        // Snapshot thread lambda.


❓ What's lambda? Is it something related to histogram maths?

Oh, not at all 😆 . I just used it as a synonym for a closure. I can change it to closure if you believe it will improve understandability. 👍

😄 Yes, please. Apparently the Rust world tries so hard to cut itself off C++ that I didn't think of it.

wprzytula · 2025-01-17T13:17:40Z

scylla/src/observability/metrics.rs

+    fn stddev(h: Histogram) -> Result<u64, MetricsError> {
+        let total_count = h
+            .clone()
+            .into_iter()
+            .map(|bucket| bucket.count() as u128)
+            .sum::<u128>();
+
+        let mean = Self::mean(h.clone())? as u128;
+        let mut variance_sum = 0;
+        for bucket in h.into_iter() {
+            let count = bucket.count() as u128;
+            let mid = ((bucket.start() + bucket.end()) / 2) as u128;
+
+            variance_sum += count * (mid as i128 - mean as i128).pow(2) as u128;
+        }
+        let variance = variance_sum / total_count;
+
+        Ok((variance as f64).sqrt() as u64)
+    }
+
+    fn minmax(h: Histogram) -> Result<(u64, u64), MetricsError> {
+        let mut min = u64::MAX;
+        let mut max = 0;
+        for bucket in h.into_iter() {
+            if bucket.count() == 0 {
+                continue;
+            }
+            let lower_bound = bucket.start();
+            let upper_bound = bucket.end();
+
+            if lower_bound < min {
+                min = lower_bound;
+            }
+            if upper_bound > max {
+                max = upper_bound;
+            }
+        }
+
+        if min > max {
+            Err(MetricsError::Empty)
+        } else {
+            Ok((min, max))
+        }
+    }


🤔 Although it makes no sense to me, the creators of histogram crate implemented IntoIterator for &Histogram! The idiomatic way would be to implement an iter() method, just like it's done on owned collections to construct an iterator of references to the collection.

🔧 This means that you should never clone a Histogram, but simply pass it by a shared reference and call .into_iter() on that reference. Weird, but no cloning involved.

I believe that IntoIterator is needed for the for loop syntax to work so here you could do:

for bucket in &h { // some code here }

But there's no reason not to have .iter() as well, so I've added that in histogram-0.11.2 which is now live on crates.io

I believe that IntoIterator is needed for the for loop syntax to work so here you could do:

for bucket in &h { // some code here }

You're right, actually the standard library defines IntoIterator on references to collections as well. TIL, thank you! 🙇

But there's no reason not to have .iter() as well, so I've added that in histogram-0.11.2 which is now live on crates.io

Great! 🚀

wprzytula · 2025-01-17T13:20:50Z

scylla/src/observability/metrics.rs

+/// Snapshot is a structure that contains histogram statistics such as
+/// min, max, mean, standard deviation, median, and most common percentiles
+/// collected in a certain moment.
+#[derive(Debug)]
+pub struct Snapshot {
+    pub min: u64,
+    pub max: u64,
+    pub mean: u64,
+    pub stddev: u64,
+    pub median: u64,
+    pub percentile_75: u64,
+    pub percentile_95: u64,
+    pub percentile_98: u64,
+    pub percentile_99: u64,
+    pub percentile_99_9: u64,
+}


🔧 For future compatibility, let's either:

make this struct #[non_exhaustive] to allow adding more fields in the future without breaking the API;

or make all those fields private and expose a getter for each field.

Which one do you find a more suitable solution? @Lorak-mmk @muzarski

wprzytula · 2025-01-17T16:31:40Z

scylla/src/observability/metrics.rs

+#[derive(Debug)]
+struct ExponentiallyWeightedMovingAverage {
+    alpha: f64,
+    uncounted: AtomicU64,
+    is_initialized: AtomicBool,
+    rate: AtomicU64,
+}
+
+impl ExponentiallyWeightedMovingAverage {
+    fn new(alpha: f64) -> Self {
+        Self {
+            alpha,
+            uncounted: AtomicU64::new(0),
+            is_initialized: AtomicBool::new(false),
+            rate: AtomicU64::new(0),
+        }
+    }
+
+    fn rate(&self) -> f64 {
+        f64::from_bits(self.rate.load(Ordering::Acquire))
+    }
+
+    fn update(&self) {
+        self.uncounted.fetch_add(1, ORDER_TYPE);
+    }
+
+    fn tick(&self) {
+        let count = self.uncounted.swap(0, ORDER_TYPE);
+        let instant_rate = count as f64 / INTERVAL as f64;
+
+        if self.is_initialized.load(Ordering::Acquire) {
+            let rate = f64::from_bits(self.rate.load(Ordering::Acquire));
+            self.rate.store(
+                f64::to_bits(rate + self.alpha * (instant_rate - rate)),
+                Ordering::Release,
+            );
+        } else {
+            self.rate
+                .store(f64::to_bits(instant_rate), Ordering::Release);
+            self.is_initialized.store(true, Ordering::Release);
+        }
+    }
+}


🔧 I can see that there are barely any comments in the cpp-driver's implementation of this, but still I'd love to see more explanation of what's going on here.

Docstrings/comments on ExponentiallyWeightedMovingAverage's fields would be much appreciated.

wprzytula · 2025-01-17T16:48:16Z

scylla/src/observability/metrics.rs

+    /// Increments counter for request timeouts
+    pub(crate) fn inc_request_timeouts(&self) {
+        self.request_timeouts.fetch_add(1, ORDER_TYPE);
+    }


❓ Is this counter intented to measure client-side request timeouts, server-side request timeouts, or both?

AFAIK this metric was used in cpp-driver to count client timeouts on requests to the database.
So is this method called in our implementation (alongside returning QueryError::RequestTimeout, the docstring of which mentions that it regards the client side).

I think it should be specified in the docstring of our inc method as well, so I will adjust it.

wprzytula · 2025-01-17T16:49:28Z

scylla/src/observability/metrics.rs

+    /// Increments counter for total number of connections
+    pub(crate) fn inc_total_connections(&self) {
+        self.total_connections.fetch_add(1, ORDER_TYPE);
+    }
+
+    /// Decrements counter for total number of connections
+    pub(crate) fn dec_total_connections(&self) {
+        self.total_connections.fetch_sub(1, ORDER_TYPE);
+    }


❓ These docstrings pretty much repeat what's said in the functions' names, but they don't tell me when I should call those.

Those functions are not meant to be called by the user.

Nevertheless I surely will adjust the docstrings to better describe the logic behind those functions for devs to understand and use properly if needed in other places in the future.

for devs to understand and use properly if needed in other places in the future.

That's what I had in mind - internal usage.

wprzytula · 2025-01-17T16:56:42Z

scylla/src/observability/metrics.rs

 #[derive(Default, Debug)]
 pub struct Metrics {


🏕️ Not related to what you've done, but I can see Metrics::new being a pub method, whereas it's only intended for in-crate use. If you could, please make it pub(crate) as a bonus.

QuerthDP force-pushed the 330-make-metrics-collection-optional/faster branch from 74b7fa3 to d989a59 Compare December 11, 2024 10:50

QuerthDP force-pushed the 330-make-metrics-collection-optional/faster branch from d7b32d0 to f9a7153 Compare January 7, 2025 22:28

wprzytula added this to the 0.16.0 milestone Jan 9, 2025

QuerthDP force-pushed the 330-make-metrics-collection-optional/faster branch from bff846c to 88de7ff Compare January 12, 2025 18:46

QuerthDP marked this pull request as ready for review January 12, 2025 19:01

QuerthDP force-pushed the 330-make-metrics-collection-optional/faster branch from 88de7ff to 185ff57 Compare January 13, 2025 12:42

wprzytula requested review from wprzytula, Lorak-mmk and muzarski January 13, 2025 12:45

github-actions bot added the semver-checks-breaking cargo-semver-checks reports that this PR introduces breaking API changes label Jan 13, 2025

NikodemGapski force-pushed the 330-make-metrics-collection-optional/faster branch from 185ff57 to 7be0e38 Compare January 14, 2025 19:03

muzarski reviewed Jan 15, 2025

View reviewed changes

NikodemGapski force-pushed the 330-make-metrics-collection-optional/faster branch from 7be0e38 to 2f6ae32 Compare January 15, 2025 18:31

QuerthDP force-pushed the 330-make-metrics-collection-optional/faster branch from 2f6ae32 to b36ef1f Compare January 16, 2025 11:46

NikodemGapski force-pushed the 330-make-metrics-collection-optional/faster branch from 0bcde8e to cf43ed2 Compare January 16, 2025 17:20

NikodemGapski and others added 6 commits January 16, 2025 20:54

metrics: add rates of queries per second

2c17a74

Implement rates similar to those available in cpp-driver. [CPP-Driver implementation](https://github.com/scylladb/cpp-driver/blob/9d6b05c9d4ebd0a6d7006af4df3e33fcdf956eeb/src/metrics.hpp#L39C1-L252C5)

metrics: add connection metrics

f437e2f

Implement gathering of connection metrics like total number of active connections, connection timeouts and request timeouts. This implementation seems to need thorough refactoring due to improper passing of metrics to the PoolRefiller.

docs: adjust metrics documentation

4392cb8

Inform that metrics may now only be used under crate feature 'metrics'. Mention new metrics in documentation and show an example how to collect them. Adjust examples to include new metrics.

QuerthDP force-pushed the 330-make-metrics-collection-optional/faster branch from cf43ed2 to 4392cb8 Compare January 16, 2025 19:57

wprzytula requested changes Jan 17, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make metrics collection optional/faster #1147

Make metrics collection optional/faster #1147

QuerthDP commented Dec 10, 2024 •

edited

Loading

github-actions bot commented Dec 10, 2024 •

edited

Loading

QuerthDP commented Jan 13, 2025

muzarski left a comment

muzarski Jan 15, 2025

muzarski Jan 15, 2025

NikodemGapski Jan 15, 2025

muzarski Jan 15, 2025 •

edited

Loading

NikodemGapski Jan 15, 2025

muzarski Jan 15, 2025

brayniac Jan 17, 2025

muzarski Jan 15, 2025

NikodemGapski Jan 15, 2025

brayniac Jan 17, 2025

muzarski Jan 15, 2025

muzarski Jan 15, 2025

muzarski Jan 15, 2025

brayniac Jan 17, 2025

wprzytula Jan 17, 2025

wprzytula Jan 17, 2025

wprzytula Jan 17, 2025

wprzytula Jan 17, 2025

NikodemGapski Jan 18, 2025

wprzytula Jan 18, 2025

wprzytula Jan 17, 2025

brayniac Jan 17, 2025

wprzytula Jan 17, 2025

wprzytula Jan 17, 2025

wprzytula Jan 17, 2025

wprzytula Jan 17, 2025

QuerthDP Jan 18, 2025

wprzytula Jan 17, 2025

QuerthDP Jan 18, 2025

wprzytula Jan 18, 2025

wprzytula Jan 17, 2025 •

edited

Loading


		// Metric implementations

		fn mean(h: Histogram) -> Result<u64, MetricsError> {

Make metrics collection optional/faster #1147

Are you sure you want to change the base?

Make metrics collection optional/faster #1147

Conversation

QuerthDP commented Dec 10, 2024 • edited Loading

Pre-review checklist

github-actions bot commented Dec 10, 2024 • edited Loading

QuerthDP commented Jan 13, 2025

muzarski left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

muzarski Jan 15, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wprzytula Jan 17, 2025 • edited Loading

Choose a reason for hiding this comment

QuerthDP commented Dec 10, 2024 •

edited

Loading

github-actions bot commented Dec 10, 2024 •

edited

Loading

muzarski Jan 15, 2025 •

edited

Loading

wprzytula Jan 17, 2025 •

edited

Loading