Don't produce `*_lag_seconds` when all metric's values are hardcoded to `0`. by x10an14-nav · Pull Request #93 · softwaremill/klag-exporter

x10an14-nav · 2026-07-08T08:18:21Z

devex: add nix tooling for reproducible builds
feat(rust): bump to latest stable version
upkeep(clippy): clean up 100+ clippy lints
fix: don't produce *_lag_seconds metrics when they are unconditionally hardcoded to 0

Standardize it across all (`Dockerfile.dev` used a different one from the rest)

I used ``` cargo clippy -- -W clippy::pedantic -W clippy::nursery -W clippy::unwrap_used -A clippy::cast_precision_loss -A clippy::cast_possible_truncation -A clippy::too_many_lines -A clippy::missing_fields_in_debug ``` to reduce the amount of warnings from 201 to 24, and clippy suggestions from 151 to 1. I did this to re-familiarize myself with this code-base, I want to give it another try, our kafka clusters are on the orders of magnitude: - 5+ partitions for biggest topics - 1.5k topics - ~1-3k consumergroups (I am not sure if I'm counting double the ACLs or not, since I know we keep some for rotation purposes per consumer) Thus, when I last tried in february, even after a lot of work on the Rust base (code @ https://github.com/nais/klag-exporter/tree/deploy_our_own_helmchart), we could NOT manage to get `klag-exporter` to scrape lag metrics for an entire cluster reliably within ~20-40 seconds without memory leak. Looking at the code now, I suspect you may have dealt with the librdkafka memory leak I struggled with back then.

x10an14-nav · 2026-07-08T08:18:53Z

Based on #89, expecting to rebase this one when that one lands.

…lly hardcoded to `0` When the config toml's `exporter.timestamp_sampling.enabled` is false, all `*_lag_seconds` metrics are hardcodet to produce `0`. This causes a lot of confusion, and cannot be expected/intended behaviour ref existing documentation. So whenever `exporter.timestamp_sampling.enabled` is false, all `*_lag_seconds` metrics should be omitted from `/metrics` endpoint.

Copilot

Warning

Copilot's review of this pull request may be incomplete because some of the changed files are excluded by your Copilot content exclusion settings. See Excluding content from Copilot for details.

Pull request overview

This PR updates the exporter to omit *_lag_seconds metric families when time-lag sampling is disabled (rather than emitting misleading hardcoded 0), alongside a Rust/tooling bump and various refactors/cleanups across the metrics, Kafka, and collector code.

Changes:

Add a time_lag_enabled switch to lag calculation so lag_seconds / max_lag_seconds become None when timestamp sampling is disabled (preventing emission of *_lag_seconds).
Refactor Prometheus rendering and various internal helpers (more const fn, formatting/locking cleanups, minor clippy-driven simplifications).
Add Nix flake tooling and bump Rust toolchain references in CI/Docker.

Reviewed changes

Copilot reviewed 23 out of 25 changed files in this pull request and generated 10 comments.

Show a summary per file

File	Description
src/metrics/types.rs	Makes small const/refactor changes to metric value/type helpers; updates OTEL datapoint timestamp type.
src/metrics/registry.rs	Adjusts registry update signatures to borrow metrics; refactors Prometheus output formatting; updates OTEL timestamp acquisition.
src/main.rs	Minor spawn/shutdown control-flow simplifications and signature adjustment for `ClusterManager::new`.
src/leadership/noop.rs	Marks `NoopLeader::new` as `const fn`.
src/leadership/mod.rs	Marks `LeadershipState::is_leader` as `const fn` and simplifies matching.
src/kafka/consumer.rs	Refactors pooling and polling logic; minor lock poison handling cleanup.
src/kafka/client.rs	Refactors lock poison handling; adjusts RSS reclaimed computation; minor doc/comment tweaks.
src/kafka/admin.rs	Replaces wildcard bindings import with explicit list; minor FFI pointer handling refactors; widens some helper visibilities.
src/http/server.rs	Simplifies address formatting/parsing.
src/export/prometheus.rs	Marks exporter constructor `const fn` and updates tests for borrowed update API.
src/config.rs	Refactors defaults into `const fn` and tweaks error formatting; (also introduces invalid `Duration::from_mins/from_hours` calls).
src/collector/timestamp_sampler.rs	Minor option-handling simplifications and doc formatting tweaks.
src/collector/rate_sampler.rs	Refactors mutex poison handling and some iterator/style details.
src/collector/offset_collector.rs	Mutex poison handling cleanup; improves compacted-topics cache logging; minor formatting tweaks.
src/collector/lag_calculator.rs	Core change: add `time_lag_enabled` and ensure lag-seconds are omitted when disabled; adds a targeted test.
src/cluster/manager.rs	Propagates `time_lag_enabled` based on sampler presence; borrows config; (also introduces invalid `Duration::from_mins` calls).
flake.nix	Adds Nix flake dev shell/build; currently pins Rust toolchain to 1.93.0 (inconsistent with CI/Docker).
flake.lock	Locks Nix inputs for the new flake.
Dockerfile.dev	Bumps Rust image version used for dev builds.
Cargo.toml	Excludes flake files and `.envrc` from packaging.
.gitignore	Ignores direnv outputs and `.env.*` files.
.github/workflows/release.yml	Bumps Rust toolchain action version.
.github/workflows/release-plz-pr.yml	Bumps Rust toolchain action version.
.github/workflows/post-release.yml	Bumps Rust toolchain action version.

Files excluded by content exclusion policy (1)

.envrc

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

+/// Default timeout for a single collection cycle (should be less than `poll_interval`)
+const DEFAULT_COLLECTION_TIMEOUT: Duration = Duration::from_mins(1);


            poll_interval: exporter_config.poll_interval,
-            max_backoff: Duration::from_secs(300),
+            max_backoff: Duration::from_mins(5),
            granularity: exporter_config.granularity,


+const fn default_cache_ttl() -> Duration {
+    Duration::from_mins(1)
 }


+const fn default_rate_history_max_age() -> Duration {
+    Duration::from_mins(10)
 }


+const fn default_compacted_topics_cache_ttl() -> Duration {
+    Duration::from_hours(1)
 }


+const fn default_metadata_cache_ttl() -> Duration {
+    Duration::from_mins(5)
 }


+const fn default_consumer_groups_cache_ttl() -> Duration {
+    Duration::from_mins(1)
 }


+const fn default_export_interval() -> Duration {
+    Duration::from_mins(1)
 }


        info!(
            cluster = %self.config.name,
            rss_before_kb = rss_before,
            rss_after_kb = rss_after,
-            rss_reclaimed_kb = rss_before as i64 - rss_after as i64,
+            rss_reclaimed_kb = rss_before.cast_signed() - rss_after.cast_signed(),
            "Recycled Kafka clients"


+        inherit (pkgs) lib;
+        cargoToml = lib.fromTOML (lib.readFile ./Cargo.toml);
+
+        rustToolchain = pkgs.rust-bin.stable."1.93.0".default.override {


Copilot

Warning

Copilot's review of this pull request may be incomplete because some of the changed files are excluded by your Copilot content exclusion settings. See Excluding content from Copilot for details.

Pull request overview

Copilot reviewed 23 out of 25 changed files in this pull request and generated 7 comments.

Files excluded by content exclusion policy (1)

.envrc

+const fn default_cache_ttl() -> Duration {
+    Duration::from_mins(1)
 }


+const fn default_rate_history_max_age() -> Duration {
+    Duration::from_mins(10)
 }


+const fn default_compacted_topics_cache_ttl() -> Duration {
+    Duration::from_hours(1)
 }

-fn default_metadata_cache_ttl() -> Duration {
-    Duration::from_secs(300)
+const fn default_metadata_cache_ttl() -> Duration {
+    Duration::from_mins(5)


+/// Default timeout for a single collection cycle (should be less than `poll_interval`)
+const DEFAULT_COLLECTION_TIMEOUT: Duration = Duration::from_mins(1);



            poll_interval: exporter_config.poll_interval,
-            max_backoff: Duration::from_secs(300),
+            max_backoff: Duration::from_mins(5),
            granularity: exporter_config.granularity,


        info!(
            cluster = %self.config.name,
            rss_before_kb = rss_before,
            rss_after_kb = rss_after,
-            rss_reclaimed_kb = rss_before as i64 - rss_after as i64,
+            rss_reclaimed_kb = rss_before.cast_signed() - rss_after.cast_signed(),
            "Recycled Kafka clients"


+        inherit (pkgs) lib;
+        cargoToml = lib.fromTOML (lib.readFile ./Cargo.toml);
+
+        rustToolchain = pkgs.rust-bin.stable."1.93.0".default.override {


x10an14-nav added 3 commits July 8, 2026 09:05

devex: add nix tooling for reproducible builds

3a6c31a

feat(rust): bump to latest stable version

fa60156

Standardize it across all (`Dockerfile.dev` used a different one from the rest)

Copilot AI review requested due to automatic review settings July 8, 2026 08:18

Copilot started reviewing on behalf of x10an14-nav July 8, 2026 08:18 View session

x10an14-nav force-pushed the fix_lag_seconds branch from 04e1cf4 to ee1fad2 Compare July 8, 2026 08:19

x10an14-nav force-pushed the fix_lag_seconds branch from ee1fad2 to f00b57c Compare July 8, 2026 08:20

Copilot AI reviewed Jul 8, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings July 8, 2026 08:22

Copilot started reviewing on behalf of x10an14-nav July 8, 2026 08:23 View session

Copilot AI reviewed Jul 8, 2026

View reviewed changes

softberries merged commit c0c2bfd into softwaremill:main Jul 9, 2026
5 checks passed

This was referenced Jul 9, 2026

chore: align remaining Rust pins at 1.96 #96

Merged

Clippy cleanup #89

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Don't produce `*_lag_seconds` when all metric's values are hardcoded to `0`.#93

Don't produce `*_lag_seconds` when all metric's values are hardcoded to `0`.#93
softberries merged 4 commits into
softwaremill:mainfrom
nais:fix_lag_seconds

x10an14-nav commented Jul 8, 2026 •

edited

Loading

Uh oh!

x10an14-nav commented Jul 8, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		/// Default timeout for a single collection cycle (should be less than `poll_interval`)
		const DEFAULT_COLLECTION_TIMEOUT: Duration = Duration::from_mins(1);

Uh oh!

Conversation

x10an14-nav commented Jul 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

x10an14-nav commented Jul 8, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

x10an14-nav commented Jul 8, 2026 •

edited

Loading