From 7e31a3175cba1ad7eb9fa5129c3965dc7abe4a39 Mon Sep 17 00:00:00 2001 From: Oleksii Sholik Date: Tue, 23 Jun 2026 16:01:01 +0200 Subject: [PATCH 01/13] Add SystemMetrics with BEAM allocator (vm.alloc.*) metrics Scaffold ElectricTelemetry.SystemMetrics, a module of poller-invoked measurement functions complementing ApplicationTelemetry's VM stats. This first piece delivers: - Cached platform/cgroup detection (system_info/0) stored in :persistent_term. Detects OS via :os.type() and cgroup version (v1/v2/ none) by stat-ing /sys/fs/cgroup. Later tasks (cgroup/host readers) build on this. - recon_alloc aggregate metrics (every 5s tick): vm.alloc.allocated, vm.alloc.used, vm.alloc.unused (allocated - used), and vm.alloc.carrier_usage (used/allocated ratio). - Per-allocator fragmentation (vm.alloc.fragmentation.unused, tagged by allocator), gated via an atomic :counters ref created once and cached in :persistent_term (one put per gate key), bumped via :counters.add/3 on the hot path. Roughly once a minute since :recon_alloc.fragmentation/1 is O(carriers). Registers the measurements in ApplicationTelemetry and adds the corresponding Telemetry.Metrics definitions. Adds recon ~> 2.5 dep. --- .../telemetry/application_telemetry.ex | 15 +- .../lib/electric/telemetry/system_metrics.ex | 206 ++++++++++++++++++ packages/electric-telemetry/mix.exs | 1 + packages/electric-telemetry/mix.lock | 1 + .../telemetry/system_metrics_test.exs | 154 +++++++++++++ 5 files changed, 376 insertions(+), 1 deletion(-) create mode 100644 packages/electric-telemetry/lib/electric/telemetry/system_metrics.ex create mode 100644 packages/electric-telemetry/test/electric/telemetry/system_metrics_test.exs diff --git a/packages/electric-telemetry/lib/electric/telemetry/application_telemetry.ex b/packages/electric-telemetry/lib/electric/telemetry/application_telemetry.ex index d2479ec687..598f024ad6 100644 --- a/packages/electric-telemetry/lib/electric/telemetry/application_telemetry.ex +++ b/packages/electric-telemetry/lib/electric/telemetry/application_telemetry.ex @@ -86,7 +86,14 @@ defmodule ElectricTelemetry.ApplicationTelemetry do :get_system_memory_usage ], &{__MODULE__, &1, [telemetry_opts]} - ) + ) ++ + [ + # BEAM allocator metrics (vm.alloc.*) live in ElectricTelemetry.SystemMetrics. + # The cheap aggregate view runs every tick; the expensive per-allocator + # fragmentation breakdown is internally gated to ~once a minute. + {ElectricTelemetry.SystemMetrics, :recon_alloc_measurement, [telemetry_opts]}, + {ElectricTelemetry.SystemMetrics, :allocator_fragmentation_measurement, [telemetry_opts]} + ] end def metrics(telemetry_opts) do @@ -117,6 +124,12 @@ defmodule ElectricTelemetry.ApplicationTelemetry do last_value("vm.memory.processes_used", unit: :byte), last_value("vm.memory.system", unit: :byte), last_value("vm.memory.total", unit: :byte), + # BEAM allocator metrics (emitted by ElectricTelemetry.SystemMetrics). + last_value("vm.alloc.allocated", unit: :byte), + last_value("vm.alloc.used", unit: :byte), + last_value("vm.alloc.unused", unit: :byte), + last_value("vm.alloc.carrier_usage"), + last_value("vm.alloc.fragmentation.unused", tags: [:allocator], unit: :byte), sum("vm.monitor.long_message_queue.length", tags: [:process_type]), distribution("vm.monitor.long_schedule.timeout", tags: [:process_type], diff --git a/packages/electric-telemetry/lib/electric/telemetry/system_metrics.ex b/packages/electric-telemetry/lib/electric/telemetry/system_metrics.ex new file mode 100644 index 0000000000..49c5313755 --- /dev/null +++ b/packages/electric-telemetry/lib/electric/telemetry/system_metrics.ex @@ -0,0 +1,206 @@ +defmodule ElectricTelemetry.SystemMetrics do + @moduledoc """ + Periodic measurement functions for low-level system/runtime metrics that + complement the application-level VM stats in `ElectricTelemetry.ApplicationTelemetry`. + + These functions are not a process; they are invoked by `ElectricTelemetry.Poller` + (via `telemetry_poller`) as `{__MODULE__, fun, [telemetry_opts]}` MFA tuples. The + poller wraps every invocation in `ElectricTelemetry.Poller.safe_invoke/3`, so a + crash here is logged and swallowed rather than removing the measurement. + + This module currently exposes: + + * BEAM allocator metrics (`vm.alloc.*`), derived from `:recon_alloc`. These are + cross-platform (recon works everywhere) and sit alongside the existing + `vm.memory.*` metrics. The cheap aggregate view is sampled every poll tick; + the expensive per-allocator fragmentation breakdown is gated to run roughly + once a minute. + + * A cached platform/cgroup detection helper (`system_info/0`). Only the OS and + cgroup version are computed here; later tasks build cgroup/host readers on top + of this scaffolding. + """ + + @system_info_key {__MODULE__, :system_info} + @fragmentation_gate_key {__MODULE__, :fragmentation_gate} + + # The poller runs all measurements at a single ~5s period. The per-allocator + # fragmentation breakdown (`:recon_alloc.fragmentation/1`) is O(carriers) and too + # expensive to run every tick, so we gate it to run roughly once a minute. With a + # 5s poll interval, every 12th tick is ~60s. + # + # We gate with an atomic `:counters` ref rather than a second `:telemetry_poller` + # child: the poller's period is shared across all measurements (see + # `ElectricTelemetry.Poller.child_spec/2`), so there is no per-measurement period + # to configure, and a counter-gate keeps this self-contained. + # + # The `:counters` ref is created once and cached in `:persistent_term` (a single + # put per gate key, ever). We deliberately avoid bumping the tick count via + # `:persistent_term.put` on every poll: each `:persistent_term.put` triggers a + # global scan of all process heaps for GC, which is exactly the per-tick overhead a + # cheap telemetry module must avoid. `:counters.add/3` is a lock-free atomic with no + # such cost and no read-modify-write race. + @fragmentation_interval_ticks 12 + + @doc """ + Number of poll ticks between per-allocator fragmentation samples. + """ + @spec allocator_fragmentation_interval_ticks() :: pos_integer() + def allocator_fragmentation_interval_ticks, do: @fragmentation_interval_ticks + + @doc """ + Boot-time platform/cgroup detection, computed once and cached in `:persistent_term`. + + Returns a map of the form `%{os: {family, name}, cgroup_version: :v1 | :v2 | :none}`. + + Cgroup version is detected by stat-ing the filesystem mounted at `/sys/fs/cgroup`: + a `cgroup2fs` filesystem indicates v2; `tmpfs`/`cgroup` indicates v1; anything else + (including non-Linux platforms or a missing mount) is reported as `:none`. + + Later tasks (cgroup/host readers) reuse this detection. + """ + @spec system_info() :: %{os: {atom(), atom()}, cgroup_version: :v1 | :v2 | :none} + def system_info do + case :persistent_term.get(@system_info_key, :undefined) do + :undefined -> + info = compute_system_info() + :persistent_term.put(@system_info_key, info) + info + + info -> + info + end + end + + defp compute_system_info do + os = :os.type() + %{os: os, cgroup_version: detect_cgroup_version(os)} + end + + defp detect_cgroup_version({:unix, :linux}) do + # `stat -fc %T` prints the filesystem type of the mount backing the path. + case System.cmd("stat", ["-fc", "%T", "/sys/fs/cgroup"], stderr_to_stdout: true) do + {output, 0} -> + case String.trim(output) do + "cgroup2fs" -> :v2 + "tmpfs" -> :v1 + "cgroup" -> :v1 + _ -> :none + end + + _ -> + :none + end + rescue + _ -> :none + end + + defp detect_cgroup_version(_non_linux), do: :none + + @doc """ + Cheap aggregate BEAM allocator metrics, sampled every poll tick. + + Emits the `[:vm, :alloc]` telemetry event carrying: + + * `:allocated` — bytes the allocators have requested from the OS (carriers) + * `:used` — bytes actually in use by blocks + * `:unused` — `allocated - used`, i.e. fragmentation/headroom held in carriers + * `:carrier_usage` — `used / allocated`, the carrier usage ratio in `0..1` + + These map to the `vm.alloc.allocated`, `vm.alloc.unused`, and + `vm.alloc.carrier_usage` metrics (plus `vm.alloc.used`). + """ + @spec recon_alloc_measurement(map()) :: :ok + def recon_alloc_measurement(_telemetry_opts) do + allocated = :recon_alloc.memory(:allocated) + used = :recon_alloc.memory(:used) + unused = max(allocated - used, 0) + carrier_usage = :recon_alloc.memory(:usage) + + :telemetry.execute([:vm, :alloc], %{ + allocated: allocated, + used: used, + unused: unused, + carrier_usage: carrier_usage + }) + + :ok + end + + @doc """ + Per-allocator fragmentation breakdown (slow tier, ~once a minute). + + Uses `:recon_alloc.fragmentation(:current)` (an O(carriers) call). It keys results + by `{type, instance}` (e.g. `{:eheap_alloc, 3}`); we compute unused bytes held in + carriers (`sbcs_carriers_size + mbcs_carriers_size - sbcs_block_size - mbcs_block_size`) + and sum them per allocator *type*. One `[:vm, :alloc, :fragmentation]` event is + emitted per type, tagged by `allocator` (the type name as a string). + + Because the poller runs every measurement at one ~5s period, this is gated by a + counter in `:persistent_term` and early-returns on non-due ticks (see + `@fragmentation_interval_ticks`). Pass `force: true` to bypass the gate (tests). + + Options: + * `:force` — when `true`, always run regardless of the gate + * `:gate_key` — override the `:persistent_term` key holding the tick counter + (used by tests to avoid contending with the live poller's counter) + """ + @spec allocator_fragmentation_measurement(map(), keyword()) :: :ok + def allocator_fragmentation_measurement(_telemetry_opts, opts \\ []) do + if due?(opts) do + :recon_alloc.fragmentation(:current) + |> Enum.reduce(%{}, fn {{type, _instance}, info}, acc -> + Map.update(acc, type, unused_bytes(info), &(&1 + unused_bytes(info))) + end) + |> Enum.each(fn {type, unused} -> + :telemetry.execute( + [:vm, :alloc, :fragmentation], + %{unused: unused}, + %{allocator: to_string(type)} + ) + end) + end + + :ok + end + + defp due?(opts) do + if Keyword.get(opts, :force, false) do + true + else + gate_key = Keyword.get(opts, :gate_key, @fragmentation_gate_key) + ref = gate_counter(gate_key) + :counters.add(ref, 1, 1) + # The first tick reads 1, so the gate fires on the Nth tick (not on boot). + rem(:counters.get(ref, 1), @fragmentation_interval_ticks) == 0 + end + end + + # Returns a cached single-slot `:counters` ref for the gate key, creating and + # caching it on first use. The `:persistent_term.put` here happens at most once per + # gate key (on first call), never on the per-tick hot path. + defp gate_counter(gate_key) do + case :persistent_term.get(gate_key, :undefined) do + :undefined -> + ref = :counters.new(1, [:write_concurrency]) + :persistent_term.put(gate_key, ref) + ref + + ref -> + ref + end + end + + # `:recon_alloc.fragmentation/1` returns a proplist per allocator with carrier and + # block sizes for both single-block (sbcs) and multi-block (mbcs) carriers. Unused + # bytes are the carrier bytes the OS gave us minus the bytes actually filled by blocks. + defp unused_bytes(info) do + carriers = + Keyword.get(info, :sbcs_carriers_size, 0) + Keyword.get(info, :mbcs_carriers_size, 0) + + blocks = + Keyword.get(info, :sbcs_block_size, 0) + Keyword.get(info, :mbcs_block_size, 0) + + max(carriers - blocks, 0) + end +end diff --git a/packages/electric-telemetry/mix.exs b/packages/electric-telemetry/mix.exs index 5ad3b79f28..f8fe494745 100644 --- a/packages/electric-telemetry/mix.exs +++ b/packages/electric-telemetry/mix.exs @@ -20,6 +20,7 @@ defmodule ElectricTelemetry.MixProject do List.flatten( [ {:otel_metric_exporter, "~> 0.4.1"}, + {:recon, "~> 2.5"}, {:req, "~> 0.5"}, {:telemetry, "~> 1.3"}, {:telemetry_metrics, "~> 1.1"}, diff --git a/packages/electric-telemetry/mix.lock b/packages/electric-telemetry/mix.lock index 5f7cd4e908..4aa7b357b0 100644 --- a/packages/electric-telemetry/mix.lock +++ b/packages/electric-telemetry/mix.lock @@ -28,6 +28,7 @@ "plug_crypto": {:hex, :plug_crypto, "2.1.1", "19bda8184399cb24afa10be734f84a16ea0a2bc65054e23a62bb10f06bc89491", [:mix], [], "hexpm", "6470bce6ffe41c8bd497612ffde1a7e4af67f36a15eea5f921af71cf3e11247c"}, "protobuf": {:hex, :protobuf, "0.17.0", "39e24e43c9648e148feba16ed51100b5b2028ea900b55460377b0476f6e10613", [:mix], [{:jason, "~> 1.2", [hex: :jason, repo: "hexpm", optional: true]}], "hexpm", "ca6c91f6f63e2c147b47f03eefd10b80538aa6fc55ff4b12b795efb786b0152f"}, "ranch": {:hex, :ranch, "1.8.1", "208169e65292ac5d333d6cdbad49388c1ae198136e4697ae2f474697140f201c", [:make, :rebar3], [], "hexpm", "aed58910f4e21deea992a67bf51632b6d60114895eb03bb392bb733064594dd0"}, + "recon": {:hex, :recon, "2.5.6", "9052588e83bfedfd9b72e1034532aee2a5369d9d9343b61aeb7fbce761010741", [:mix, :rebar3], [], "hexpm", "96c6799792d735cc0f0fd0f86267e9d351e63339cbe03df9d162010cefc26bb0"}, "req": {:hex, :req, "0.6.1", "7b904c8b42d0e08136a5c6aba024fd12fc79a1ed8856e7a3522b0917f7e75113", [:mix], [{:brotli, "~> 0.3.1", [hex: :brotli, repo: "hexpm", optional: true]}, {:ezstd, "~> 1.0", [hex: :ezstd, repo: "hexpm", optional: true]}, {:finch, "~> 0.21.0 or ~> 0.22.0", [hex: :finch, repo: "hexpm", optional: false]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: false]}, {:mime, "~> 2.0.6 or ~> 2.1", [hex: :mime, repo: "hexpm", optional: false]}, {:nimble_csv, "~> 1.0", [hex: :nimble_csv, repo: "hexpm", optional: true]}, {:plug, "~> 1.0", [hex: :plug, repo: "hexpm", optional: true]}], "hexpm", "aaf11c9c80f2df2364630b3594e1857fe610d8ea7cb994e1ce3dcb55f204ff1c"}, "retry": {:hex, :retry, "0.19.0", "aeb326d87f62295d950f41e1255fe6f43280a1b390d36e280b7c9b00601ccbc2", [:mix], [], "hexpm", "85ef376aa60007e7bff565c366310966ec1bd38078765a0e7f20ec8a220d02ca"}, "telemetry": {:hex, :telemetry, "1.4.2", "a0cb522801dffb1c49fe6e30561badffc7b6d0e180db1300df759faa22062855", [:rebar3], [], "hexpm", "928f6495066506077862c0d1646609eed891a4326bee3126ba54b60af61febb1"}, diff --git a/packages/electric-telemetry/test/electric/telemetry/system_metrics_test.exs b/packages/electric-telemetry/test/electric/telemetry/system_metrics_test.exs new file mode 100644 index 0000000000..207f3552ac --- /dev/null +++ b/packages/electric-telemetry/test/electric/telemetry/system_metrics_test.exs @@ -0,0 +1,154 @@ +defmodule ElectricTelemetry.SystemMetricsTest do + use ExUnit.Case, async: true + + alias ElectricTelemetry.SystemMetrics + + describe "system_info/0" do + test "returns a well-formed map" do + info = SystemMetrics.system_info() + + assert %{os: os, cgroup_version: cgroup_version} = info + assert is_tuple(os) + assert cgroup_version in [:v1, :v2, :none] + end + + test "is stable across calls (cached)" do + assert SystemMetrics.system_info() == SystemMetrics.system_info() + end + + test "reports the correct cgroup version for the current platform" do + info = SystemMetrics.system_info() + + case :os.type() do + {:unix, :linux} -> + # On Linux CI/dev this is typically v2, but we don't hard-code it; + # we only assert it's a real cgroup detection result. + assert info.cgroup_version in [:v1, :v2, :none] + + _ -> + assert info.cgroup_version == :none + end + end + end + + describe "recon_alloc_measurement/1" do + test "emits vm.alloc.* measurements with sane numeric values" do + ref = make_ref() + handler_id = {__MODULE__, ref} + + :telemetry.attach( + handler_id, + [:vm, :alloc], + fn _event, measurements, _meta, pid -> send(pid, {ref, measurements}) end, + self() + ) + + on_exit(fn -> :telemetry.detach(handler_id) end) + + assert :ok = SystemMetrics.recon_alloc_measurement(%{}) + + assert_received {^ref, measurements} + + assert is_integer(measurements.allocated) + assert is_integer(measurements.used) + assert is_integer(measurements.unused) + assert is_number(measurements.carrier_usage) + + assert measurements.allocated >= measurements.used + assert measurements.unused == measurements.allocated - measurements.used + assert measurements.unused >= 0 + # carrier_usage is recon_alloc's used/allocated ratio — nominally 0..1, but an + # instantaneous live-VM sample can momentarily exceed 1.0 (observed ~1.003) because + # the underlying mbcs/sbcs stats are sampled non-atomically. Use a tolerant sanity + # ceiling rather than a hard 1.0 to avoid a flaky test. + assert measurements.carrier_usage >= 0.0 + assert measurements.carrier_usage <= 1.5 + end + end + + describe "allocator_fragmentation_measurement/1" do + test "emits per-allocator unused bytes when due" do + ref = make_ref() + handler_id = {__MODULE__, ref} + + events = :ets.new(:events, [:public, :duplicate_bag]) + + :telemetry.attach( + handler_id, + [:vm, :alloc, :fragmentation], + fn _event, measurements, meta, _ -> + :ets.insert(events, {meta.allocator, measurements.unused}) + end, + nil + ) + + on_exit(fn -> :telemetry.detach(handler_id) end) + + # Force a due tick regardless of the persistent counter. + assert :ok = SystemMetrics.allocator_fragmentation_measurement(%{}, force: true) + + results = :ets.tab2list(events) + assert results != [] + + for {allocator, unused} <- results do + assert is_binary(allocator) + assert is_integer(unused) + assert unused >= 0 + end + end + + test "gating only fires every Nth tick" do + ref = make_ref() + handler_id = {__MODULE__, ref} + + counter = :counters.new(1, []) + + :telemetry.attach( + handler_id, + [:vm, :alloc, :fragmentation], + fn _event, _measurements, _meta, _ -> :counters.add(counter, 1, 1) end, + nil + ) + + on_exit(fn -> :telemetry.detach(handler_id) end) + + # Use an isolated gate key so we don't interfere with the real poller's counter. + gate_key = {:test_gate, ref} + every = SystemMetrics.allocator_fragmentation_interval_ticks() + + # Run exactly `every` ticks; exactly one of them should be due. + for _ <- 1..every do + SystemMetrics.allocator_fragmentation_measurement(%{}, gate_key: gate_key) + end + + # Exactly one tick in `every` is due; on that tick we emit one event per + # distinct allocator *type*. + fired = :counters.get(counter, 1) + + type_count = + :recon_alloc.fragmentation(:current) + |> Enum.map(fn {{type, _instance}, _info} -> type end) + |> Enum.uniq() + |> length() + + assert fired == type_count + end + end + + describe "metric definitions" do + test "vm.alloc.* metrics are present in ApplicationTelemetry.metrics/1" do + metrics = ElectricTelemetry.ApplicationTelemetry.metrics(%{}) + names = Enum.map(metrics, & &1.name) + + assert [:vm, :alloc, :allocated] in names + assert [:vm, :alloc, :unused] in names + assert [:vm, :alloc, :carrier_usage] in names + + frag = + Enum.find(metrics, &(&1.name == [:vm, :alloc, :fragmentation, :unused])) + + assert frag, "expected per-allocator fragmentation metric to be defined" + assert :allocator in frag.tags + end + end +end From 1f404fbe9ff0794dffd45ccc64758a465830832f Mon Sep 17 00:00:00 2001 From: Oleksii Sholik Date: Tue, 23 Jun 2026 16:13:10 +0200 Subject: [PATCH 02/13] Add cgroup v1/v2 reader emitting cgroup.* metrics Adds ElectricTelemetry.SystemMetrics.Cgroup, a defensive reader for cgroup v1 and v2 accounting files, plus a thin cgroup_measurement/2 delegator in SystemMetrics registered as a builtin periodic measurement. Emits cgroup.* memory (current/anon/file/working_set/max), cpu (usage_usec/nr_throttled/throttled_usec) and io (rbytes/wbytes) metrics, plus PSI full avg10 for memory and cpu on v2. v2 reads /sys/fs/cgroup/* directly; v1 reads per-controller subdirs. The cgroup version comes from the cached system_info/0 detection; :none (incl. non-Linux) no-ops cleanly. memory.max is skipped when unlimited ("max" on v2, the huge sentinel on v1); working_set is skipped when inactive_file is missing; cpu times are converted ns->us on v1 for a consistent microsecond unit. All reads are exhaustively defensive: missing/malformed files skip the metric rather than crash. IO is v2-only (v1 blkio is awkward and absent on our hosts). The cgroup root is configurable via opts for fixture-based tests. --- .../telemetry/application_telemetry.ex | 22 +- .../lib/electric/telemetry/system_metrics.ex | 16 + .../telemetry/system_metrics/cgroup.ex | 301 +++++++++++++++ .../telemetry/system_metrics/cgroup_test.exs | 364 ++++++++++++++++++ 4 files changed, 702 insertions(+), 1 deletion(-) create mode 100644 packages/electric-telemetry/lib/electric/telemetry/system_metrics/cgroup.ex create mode 100644 packages/electric-telemetry/test/electric/telemetry/system_metrics/cgroup_test.exs diff --git a/packages/electric-telemetry/lib/electric/telemetry/application_telemetry.ex b/packages/electric-telemetry/lib/electric/telemetry/application_telemetry.ex index 598f024ad6..16145e6ce9 100644 --- a/packages/electric-telemetry/lib/electric/telemetry/application_telemetry.ex +++ b/packages/electric-telemetry/lib/electric/telemetry/application_telemetry.ex @@ -92,7 +92,10 @@ defmodule ElectricTelemetry.ApplicationTelemetry do # The cheap aggregate view runs every tick; the expensive per-allocator # fragmentation breakdown is internally gated to ~once a minute. {ElectricTelemetry.SystemMetrics, :recon_alloc_measurement, [telemetry_opts]}, - {ElectricTelemetry.SystemMetrics, :allocator_fragmentation_measurement, [telemetry_opts]} + {ElectricTelemetry.SystemMetrics, :allocator_fragmentation_measurement, [telemetry_opts]}, + # cgroup (v1/v2) accounting metrics (cgroup.*) live in + # ElectricTelemetry.SystemMetrics.Cgroup; no-op when no cgroup is detected. + {ElectricTelemetry.SystemMetrics, :cgroup_measurement, [telemetry_opts]} ] end @@ -130,6 +133,23 @@ defmodule ElectricTelemetry.ApplicationTelemetry do last_value("vm.alloc.unused", unit: :byte), last_value("vm.alloc.carrier_usage"), last_value("vm.alloc.fragmentation.unused", tags: [:allocator], unit: :byte), + # cgroup (v1/v2) accounting metrics (emitted by + # ElectricTelemetry.SystemMetrics.Cgroup). + last_value("cgroup.memory.current", unit: :byte), + last_value("cgroup.memory.anon", unit: :byte), + last_value("cgroup.memory.file", unit: :byte), + last_value("cgroup.memory.working_set", unit: :byte), + last_value("cgroup.memory.max", unit: :byte), + # PSI "full avg10" stall percentage (v2 only). + last_value("cgroup.memory.pressure.full.avg10"), + # cpu usage / throttled time already in microseconds (v1 ns values are + # converted in the reader); :unit is a label only, no conversion. + last_value("cgroup.cpu.usage_usec", unit: :microsecond), + last_value("cgroup.cpu.nr_throttled"), + last_value("cgroup.cpu.throttled_usec", unit: :microsecond), + last_value("cgroup.cpu.pressure.full.avg10"), + last_value("cgroup.io.rbytes", unit: :byte), + last_value("cgroup.io.wbytes", unit: :byte), sum("vm.monitor.long_message_queue.length", tags: [:process_type]), distribution("vm.monitor.long_schedule.timeout", tags: [:process_type], diff --git a/packages/electric-telemetry/lib/electric/telemetry/system_metrics.ex b/packages/electric-telemetry/lib/electric/telemetry/system_metrics.ex index 49c5313755..48183f2b75 100644 --- a/packages/electric-telemetry/lib/electric/telemetry/system_metrics.ex +++ b/packages/electric-telemetry/lib/electric/telemetry/system_metrics.ex @@ -164,6 +164,22 @@ defmodule ElectricTelemetry.SystemMetrics do :ok end + @doc """ + cgroup (v1/v2) accounting metrics, sampled every poll tick. + + Reads the container's cgroup files and emits the `cgroup.*` family (memory, + cpu, io, and — on v2 — PSI pressure). Parsing lives in + `ElectricTelemetry.SystemMetrics.Cgroup`; this is a thin delegator so it + registers like the other measurements. No-ops when no cgroup is detected. + + Options are forwarded to `Cgroup.measurement/2` (`:cgroup_root` and + `:cgroup_version` overrides for tests). + """ + @spec cgroup_measurement(map(), keyword()) :: :ok + def cgroup_measurement(telemetry_opts, opts \\ []) do + ElectricTelemetry.SystemMetrics.Cgroup.measurement(telemetry_opts, opts) + end + defp due?(opts) do if Keyword.get(opts, :force, false) do true diff --git a/packages/electric-telemetry/lib/electric/telemetry/system_metrics/cgroup.ex b/packages/electric-telemetry/lib/electric/telemetry/system_metrics/cgroup.ex new file mode 100644 index 0000000000..64d6de7dab --- /dev/null +++ b/packages/electric-telemetry/lib/electric/telemetry/system_metrics/cgroup.ex @@ -0,0 +1,301 @@ +defmodule ElectricTelemetry.SystemMetrics.Cgroup do + @moduledoc """ + Reads cgroup (v1 and v2) accounting files and emits `cgroup.*` telemetry. + + Electric Cloud runs on cgroup v2 (container at the root of its own cgroup + namespace), so v2 controllers are read directly from `/sys/fs/cgroup/*`. v1 is + the legacy/fallback layout (Fargate and other hosts) where controllers live + under per-controller subdirs (`/sys/fs/cgroup/memory/`, `/sys/fs/cgroup/cpu/`, + …). + + All reads are defensive: a missing file, a permission error, or unexpected + content results in the corresponding metric being skipped, never a crash. The + cgroup version is taken from `ElectricTelemetry.SystemMetrics.system_info/0` + (cached at boot) so we never stat the filesystem on the hot path. When the + version is `:none` (including all non-Linux platforms) the measurement is a + clean no-op. + + Units: + + * memory metrics are bytes + * cpu usage / throttled time are emitted in **microseconds**. v1 reports + these in nanoseconds (`cpuacct.usage`, `cpu.stat` `throttled_time`), so we + convert to microseconds for a unit consistent with v2's `usage_usec` / + `throttled_usec`. + * io metrics are bytes + * PSI `full avg10` is a stall percentage (float). PSI exists on v2 only. + """ + + alias ElectricTelemetry.SystemMetrics + + @default_root "/sys/fs/cgroup" + + @doc """ + The default cgroup filesystem root. Tests override this via the `:cgroup_root` + option on `measurement/2`. + """ + @spec default_root() :: String.t() + def default_root, do: @default_root + + @doc """ + Read the cgroup accounting files and emit `cgroup.*` telemetry events. + + Reads every tick (no slow-tier gating). Options: + + * `:cgroup_root` — override the cgroup filesystem root (default + `#{@default_root}`); used by tests to point at fixture trees. + * `:cgroup_version` — override the detected cgroup version (`:v1`/`:v2`/ + `:none`); used by tests. Defaults to the cached + `SystemMetrics.system_info/0` value. + """ + @spec measurement(map(), keyword()) :: :ok + def measurement(_telemetry_opts, opts \\ []) do + version = + Keyword.get_lazy(opts, :cgroup_version, fn -> SystemMetrics.system_info().cgroup_version end) + + root = Keyword.get(opts, :cgroup_root, @default_root) + + case version do + :v2 -> read_v2(root) + :v1 -> read_v1(root) + :none -> :ok + end + + :ok + end + + ## cgroup v2 ------------------------------------------------------------- + + defp read_v2(root) do + emit_v2_memory(root) + emit_v2_cpu(root) + emit_v2_io(root) + end + + defp emit_v2_memory(root) do + current = read_int_file(Path.join(root, "memory.current")) + stat = read_stat_file(Path.join(root, "memory.stat")) + + emit(:memory, :current, current) + emit(:memory, :anon, Map.get(stat, "anon")) + emit(:memory, :file, Map.get(stat, "file")) + emit(:memory, :working_set, working_set(current, Map.get(stat, "inactive_file"))) + emit(:memory, :max, real_limit(read_raw_file(Path.join(root, "memory.max")))) + + emit_pressure(Path.join(root, "memory.pressure"), :memory) + end + + defp emit_v2_cpu(root) do + stat = read_stat_file(Path.join(root, "cpu.stat")) + + emit(:cpu, :usage_usec, Map.get(stat, "usage_usec")) + emit(:cpu, :nr_throttled, Map.get(stat, "nr_throttled")) + emit(:cpu, :throttled_usec, Map.get(stat, "throttled_usec")) + + emit_pressure(Path.join(root, "cpu.pressure"), :cpu) + end + + defp emit_v2_io(root) do + io = parse_io_stat(read_raw_file(Path.join(root, "io.stat"))) + + emit(:io, :rbytes, io.rbytes) + emit(:io, :wbytes, io.wbytes) + end + + ## cgroup v1 ------------------------------------------------------------- + + defp read_v1(root) do + emit_v1_memory(root) + emit_v1_cpu(root) + # NOTE: v1 blkio accounting (blkio.throttle.io_service_bytes) is awkward and + # not present on the hosts we care about (Electric Cloud is v2). IO metrics + # are emitted on v2 only; v1 deliberately skips them. + end + + defp emit_v1_memory(root) do + current = read_int_file(Path.join(root, "memory/memory.usage_in_bytes")) + stat = read_stat_file(Path.join(root, "memory/memory.stat")) + + emit(:memory, :current, current) + emit(:memory, :anon, Map.get(stat, "rss")) + emit(:memory, :file, Map.get(stat, "cache")) + emit(:memory, :working_set, working_set(current, Map.get(stat, "inactive_file"))) + + emit( + :memory, + :max, + real_limit(read_raw_file(Path.join(root, "memory/memory.limit_in_bytes"))) + ) + + # v1 has no PSI; skip cgroup.memory.pressure.full.avg10. + end + + defp emit_v1_cpu(root) do + usage_ns = read_int_file(Path.join(root, "cpuacct/cpuacct.usage")) + emit(:cpu, :usage_usec, ns_to_us(usage_ns)) + + stat = read_stat_file(Path.join(root, "cpu/cpu.stat")) + emit(:cpu, :nr_throttled, Map.get(stat, "nr_throttled")) + emit(:cpu, :throttled_usec, ns_to_us(Map.get(stat, "throttled_time"))) + + # v1 has no PSI; skip cgroup.cpu.pressure.full.avg10. + end + + ## emitting -------------------------------------------------------------- + + # Emit a `[:cgroup, plane]` event carrying a single keyed measurement, unless + # the value is nil (file missing / unparseable / intentionally skipped). + defp emit(_plane, _key, nil), do: :ok + + defp emit(plane, key, value) do + :telemetry.execute([:cgroup, plane], %{key => value}) + :ok + end + + # PSI files may be absent even on v2 (kernel built without PSI). Skip on any + # read/parse failure. + defp emit_pressure(path, plane) do + case psi_full_avg10(read_raw_file(path)) do + nil -> :ok + avg10 -> :telemetry.execute([:cgroup, plane, :pressure, :full], %{avg10: avg10}) + end + + :ok + end + + ## parsing helpers ------------------------------------------------------- + + # working_set = current - inactive_file. If either input is missing, skip the + # metric rather than emit a wrong number. + defp working_set(nil, _inactive_file), do: nil + defp working_set(_current, nil), do: nil + defp working_set(current, inactive_file), do: max(current - inactive_file, 0) + + # A memory limit is only meaningful when it's a real number. v2 unlimited is + # the literal string "max"; v1 unlimited is a huge sentinel (~9.2e18). In both + # cases skip the metric (the host-RAM ceiling comes from /proc readers). + @v1_unlimited_threshold 0x7000_0000_0000_0000 + defp real_limit(nil), do: nil + + defp real_limit(raw) do + case parse_int(raw) do + nil -> nil + value when value >= @v1_unlimited_threshold -> nil + value -> value + end + end + + defp ns_to_us(nil), do: nil + defp ns_to_us(ns), do: div(ns, 1000) + + # Parse a flat `key value` stat file (memory.stat, cpu.stat) into a map of + # binary key -> integer value. Lines that don't parse are dropped. + defp read_stat_file(path) do + case read_raw_file(path) do + nil -> + %{} + + content -> + content + |> String.split("\n", trim: true) + |> Enum.reduce(%{}, fn line, acc -> + case String.split(line, " ", trim: true) do + [key, value] -> + case parse_int(value) do + nil -> acc + int -> Map.put(acc, key, int) + end + + _ -> + acc + end + end) + end + end + + # io.stat is one line per device: "MAJ:MIN rbytes=… wbytes=… rios=… …". + # Sum rbytes/wbytes across all devices. + defp parse_io_stat(nil), do: %{rbytes: nil, wbytes: nil} + + defp parse_io_stat(content) do + {rbytes, wbytes, any?} = + content + |> String.split("\n", trim: true) + |> Enum.reduce({0, 0, false}, fn line, {r, w, any?} -> + fields = String.split(line, " ", trim: true) + + line_r = io_field(fields, "rbytes=") + line_w = io_field(fields, "wbytes=") + + {r + (line_r || 0), w + (line_w || 0), any? or line_r != nil or line_w != nil} + end) + + if any? do + %{rbytes: rbytes, wbytes: wbytes} + else + %{rbytes: nil, wbytes: nil} + end + end + + defp io_field(fields, prefix) do + Enum.find_value(fields, fn field -> + case field do + ^prefix <> rest -> parse_int(rest) + _ -> nil + end + end) + end + + # PSI file format (one "some"/"full" line each): + # some avg10=0.00 avg60=0.00 avg300=0.00 total=12345 + # full avg10=0.00 avg60=0.00 avg300=0.00 total=6789 + # Extract avg10 from the "full" line as a float. + defp psi_full_avg10(nil), do: nil + + defp psi_full_avg10(content) do + content + |> String.split("\n", trim: true) + |> Enum.find_value(fn line -> + case String.split(line, " ", trim: true) do + ["full" | fields] -> + Enum.find_value(fields, fn field -> + case field do + "avg10=" <> rest -> parse_float(rest) + _ -> nil + end + end) + + _ -> + nil + end + end) + end + + ## low-level file/number helpers ----------------------------------------- + + # Read a single-integer file (memory.current, usage_in_bytes, cpuacct.usage). + defp read_int_file(path), do: parse_int(read_raw_file(path)) + + defp read_raw_file(path) do + case File.read(path) do + {:ok, content} -> String.trim(content) + {:error, _reason} -> nil + end + end + + defp parse_int(nil), do: nil + + defp parse_int(str) do + case Integer.parse(String.trim(str)) do + {int, ""} -> int + _ -> nil + end + end + + defp parse_float(str) do + case Float.parse(String.trim(str)) do + {float, ""} -> float + _ -> nil + end + end +end diff --git a/packages/electric-telemetry/test/electric/telemetry/system_metrics/cgroup_test.exs b/packages/electric-telemetry/test/electric/telemetry/system_metrics/cgroup_test.exs new file mode 100644 index 0000000000..b61bb52fcd --- /dev/null +++ b/packages/electric-telemetry/test/electric/telemetry/system_metrics/cgroup_test.exs @@ -0,0 +1,364 @@ +defmodule ElectricTelemetry.SystemMetrics.CgroupTest do + use ExUnit.Case, async: true + + @moduletag :tmp_dir + + alias ElectricTelemetry.SystemMetrics.Cgroup + + # Attach handlers for every cgroup.* event and collect emitted measurements + # into a flat map of metric-name (dotted string) => value. Single-key + # measurement maps make this unambiguous. + defp collect_cgroup_metrics(fun) do + test_pid = self() + ref = make_ref() + + events = [ + [:cgroup, :memory], + [:cgroup, :cpu], + [:cgroup, :io], + [:cgroup, :memory, :pressure, :full], + [:cgroup, :cpu, :pressure, :full] + ] + + handler_id = {__MODULE__, ref} + + :telemetry.attach_many( + handler_id, + events, + fn event, measurements, _meta, _ -> + send(test_pid, {ref, event, measurements}) + end, + nil + ) + + try do + fun.() + after + :telemetry.detach(handler_id) + end + + drain(ref, %{}) + end + + defp drain(ref, acc) do + receive do + {^ref, event, measurements} -> + name = event |> Enum.map(&Atom.to_string/1) |> Enum.join(".") + + acc = + Enum.reduce(measurements, acc, fn {k, v}, acc -> + Map.put(acc, "#{name}.#{k}", v) + end) + + drain(ref, acc) + after + 0 -> acc + end + end + + defp write_file(root, rel, content) do + path = Path.join(root, rel) + File.mkdir_p!(Path.dirname(path)) + File.write!(path, content) + end + + describe "cgroup v2" do + setup %{tmp_dir: tmp_dir} do + root = Path.join(tmp_dir, "v2") + + write_file(root, "memory.current", "1048576\n") + + write_file(root, "memory.stat", """ + anon 524288 + file 262144 + inactive_file 131072 + slab 4096 + """) + + write_file(root, "memory.max", "max\n") + + write_file(root, "memory.pressure", """ + some avg10=1.50 avg60=0.30 avg300=0.10 total=12345 + full avg10=0.75 avg60=0.20 avg300=0.05 total=6789 + """) + + write_file(root, "cpu.stat", """ + usage_usec 9000000 + user_usec 6000000 + system_usec 3000000 + nr_periods 100 + nr_throttled 7 + throttled_usec 250000 + """) + + write_file(root, "cpu.pressure", """ + some avg10=2.00 avg60=0.50 avg300=0.10 total=99999 + full avg10=1.25 avg60=0.40 avg300=0.08 total=55555 + """) + + write_file(root, "io.stat", """ + 259:0 rbytes=1000 wbytes=2000 rios=10 wios=20 + 259:1 rbytes=500 wbytes=1500 rios=5 wios=15 + """) + + %{root: root} + end + + test "emits parsed memory metrics", %{root: root} do + m = + collect_cgroup_metrics(fn -> + Cgroup.measurement(%{}, cgroup_version: :v2, cgroup_root: root) + end) + + assert m["cgroup.memory.current"] == 1_048_576 + assert m["cgroup.memory.anon"] == 524_288 + assert m["cgroup.memory.file"] == 262_144 + # working_set = current - inactive_file = 1048576 - 131072 + assert m["cgroup.memory.working_set"] == 1_048_576 - 131_072 + end + + test "skips memory.max when value is \"max\"", %{root: root} do + m = + collect_cgroup_metrics(fn -> + Cgroup.measurement(%{}, cgroup_version: :v2, cgroup_root: root) + end) + + refute Map.has_key?(m, "cgroup.memory.max") + end + + test "emits a real numeric memory.max", %{root: root} do + write_file(root, "memory.max", "2147483648\n") + + m = + collect_cgroup_metrics(fn -> + Cgroup.measurement(%{}, cgroup_version: :v2, cgroup_root: root) + end) + + assert m["cgroup.memory.max"] == 2_147_483_648 + end + + test "parses PSI full avg10 for memory and cpu", %{root: root} do + m = + collect_cgroup_metrics(fn -> + Cgroup.measurement(%{}, cgroup_version: :v2, cgroup_root: root) + end) + + assert m["cgroup.memory.pressure.full.avg10"] == 0.75 + assert m["cgroup.cpu.pressure.full.avg10"] == 1.25 + end + + test "emits cpu usage / throttling in microseconds", %{root: root} do + m = + collect_cgroup_metrics(fn -> + Cgroup.measurement(%{}, cgroup_version: :v2, cgroup_root: root) + end) + + assert m["cgroup.cpu.usage_usec"] == 9_000_000 + assert m["cgroup.cpu.nr_throttled"] == 7 + assert m["cgroup.cpu.throttled_usec"] == 250_000 + end + + test "sums io rbytes/wbytes across devices", %{root: root} do + m = + collect_cgroup_metrics(fn -> + Cgroup.measurement(%{}, cgroup_version: :v2, cgroup_root: root) + end) + + assert m["cgroup.io.rbytes"] == 1500 + assert m["cgroup.io.wbytes"] == 3500 + end + + test "skips working_set when inactive_file is absent", %{root: root} do + write_file(root, "memory.stat", "anon 524288\nfile 262144\n") + + m = + collect_cgroup_metrics(fn -> + Cgroup.measurement(%{}, cgroup_version: :v2, cgroup_root: root) + end) + + refute Map.has_key?(m, "cgroup.memory.working_set") + # other metrics still emitted + assert m["cgroup.memory.anon"] == 524_288 + end + + test "skips PSI when pressure files are absent (kernel without PSI)", %{root: root} do + File.rm!(Path.join(root, "memory.pressure")) + File.rm!(Path.join(root, "cpu.pressure")) + + m = + collect_cgroup_metrics(fn -> + Cgroup.measurement(%{}, cgroup_version: :v2, cgroup_root: root) + end) + + refute Map.has_key?(m, "cgroup.memory.pressure.full.avg10") + refute Map.has_key?(m, "cgroup.cpu.pressure.full.avg10") + # rest still works + assert m["cgroup.memory.current"] == 1_048_576 + end + end + + describe "cgroup v1" do + setup %{tmp_dir: tmp_dir} do + root = Path.join(tmp_dir, "v1") + + write_file(root, "memory/memory.usage_in_bytes", "2097152\n") + + write_file(root, "memory/memory.stat", """ + rss 1048576 + cache 524288 + inactive_file 262144 + mapped_file 4096 + """) + + # unlimited sentinel + write_file(root, "memory/memory.limit_in_bytes", "9223372036854771712\n") + + # cpuacct.usage is in nanoseconds + write_file(root, "cpuacct/cpuacct.usage", "9000000000\n") + + write_file(root, "cpu/cpu.stat", """ + nr_periods 100 + nr_throttled 7 + throttled_time 250000000 + """) + + %{root: root} + end + + test "emits parsed memory metrics with v1 key mapping", %{root: root} do + m = + collect_cgroup_metrics(fn -> + Cgroup.measurement(%{}, cgroup_version: :v1, cgroup_root: root) + end) + + assert m["cgroup.memory.current"] == 2_097_152 + # v1 anon <- rss, file <- cache + assert m["cgroup.memory.anon"] == 1_048_576 + assert m["cgroup.memory.file"] == 524_288 + assert m["cgroup.memory.working_set"] == 2_097_152 - 262_144 + end + + test "skips memory.max when v1 unlimited sentinel", %{root: root} do + m = + collect_cgroup_metrics(fn -> + Cgroup.measurement(%{}, cgroup_version: :v1, cgroup_root: root) + end) + + refute Map.has_key?(m, "cgroup.memory.max") + end + + test "emits a real numeric v1 memory limit", %{root: root} do + write_file(root, "memory/memory.limit_in_bytes", "1073741824\n") + + m = + collect_cgroup_metrics(fn -> + Cgroup.measurement(%{}, cgroup_version: :v1, cgroup_root: root) + end) + + assert m["cgroup.memory.max"] == 1_073_741_824 + end + + test "converts cpu usage / throttled time from ns to us", %{root: root} do + m = + collect_cgroup_metrics(fn -> + Cgroup.measurement(%{}, cgroup_version: :v1, cgroup_root: root) + end) + + # 9_000_000_000 ns -> 9_000_000 us + assert m["cgroup.cpu.usage_usec"] == 9_000_000 + assert m["cgroup.cpu.nr_throttled"] == 7 + # 250_000_000 ns -> 250_000 us + assert m["cgroup.cpu.throttled_usec"] == 250_000 + end + + test "does not emit PSI on v1", %{root: root} do + m = + collect_cgroup_metrics(fn -> + Cgroup.measurement(%{}, cgroup_version: :v1, cgroup_root: root) + end) + + refute Map.has_key?(m, "cgroup.memory.pressure.full.avg10") + refute Map.has_key?(m, "cgroup.cpu.pressure.full.avg10") + end + + test "does not emit io metrics on v1", %{root: root} do + m = + collect_cgroup_metrics(fn -> + Cgroup.measurement(%{}, cgroup_version: :v1, cgroup_root: root) + end) + + refute Map.has_key?(m, "cgroup.io.rbytes") + refute Map.has_key?(m, "cgroup.io.wbytes") + end + end + + describe ":none / non-Linux" do + test "no-ops and emits nothing", %{tmp_dir: tmp_dir} do + m = + collect_cgroup_metrics(fn -> + assert :ok = Cgroup.measurement(%{}, cgroup_version: :none, cgroup_root: tmp_dir) + end) + + assert m == %{} + end + end + + describe "malformed / missing files" do + test "skips missing files without crashing, still emits available metrics", %{ + tmp_dir: tmp_dir + } do + root = Path.join(tmp_dir, "partial") + # Only a memory.current and a malformed memory.stat; everything else absent. + write_file(root, "memory.current", "4096\n") + write_file(root, "memory.stat", "this is not valid\nanon notanumber\nfile 8192\n") + + m = + collect_cgroup_metrics(fn -> + assert :ok = Cgroup.measurement(%{}, cgroup_version: :v2, cgroup_root: root) + end) + + assert m["cgroup.memory.current"] == 4096 + # malformed/absent keys are skipped + refute Map.has_key?(m, "cgroup.memory.anon") + assert m["cgroup.memory.file"] == 8192 + refute Map.has_key?(m, "cgroup.memory.working_set") + # entirely-absent planes simply emit nothing + refute Map.has_key?(m, "cgroup.cpu.usage_usec") + refute Map.has_key?(m, "cgroup.io.rbytes") + end + + test "empty io.stat emits nothing for io", %{tmp_dir: tmp_dir} do + root = Path.join(tmp_dir, "emptyio") + write_file(root, "io.stat", "") + + m = + collect_cgroup_metrics(fn -> + assert :ok = Cgroup.measurement(%{}, cgroup_version: :v2, cgroup_root: root) + end) + + refute Map.has_key?(m, "cgroup.io.rbytes") + refute Map.has_key?(m, "cgroup.io.wbytes") + end + end + + describe "metric definitions" do + test "cgroup.* metrics are present in ApplicationTelemetry.metrics/1" do + names = + ElectricTelemetry.ApplicationTelemetry.metrics(%{}) + |> Enum.map(& &1.name) + + assert [:cgroup, :memory, :current] in names + assert [:cgroup, :memory, :anon] in names + assert [:cgroup, :memory, :file] in names + assert [:cgroup, :memory, :working_set] in names + assert [:cgroup, :memory, :max] in names + assert [:cgroup, :memory, :pressure, :full, :avg10] in names + assert [:cgroup, :cpu, :usage_usec] in names + assert [:cgroup, :cpu, :nr_throttled] in names + assert [:cgroup, :cpu, :throttled_usec] in names + assert [:cgroup, :cpu, :pressure, :full, :avg10] in names + assert [:cgroup, :io, :rbytes] in names + assert [:cgroup, :io, :wbytes] in names + end + end +end From 9cadf4647053cec5be63fd9f9b604e670d76a545 Mon Sep 17 00:00:00 2001 From: Oleksii Sholik Date: Tue, 23 Jun 2026 16:21:05 +0200 Subject: [PATCH 03/13] Add /proc host + per-process BEAM memory/io metrics Add ElectricTelemetry.SystemMetrics.Proc, a defensive Linux /proc reader that emits host.mem.* (from /proc/meminfo) and host.proc.beam.* (from /proc//status and /proc//io). This bridges the cgroup plane and the BEAM allocator plane: per-process anon/file/shmem RSS shows cgroup anon tracking the BEAM footprint, and host meminfo gives the host-RAM ceiling/cache picture (one task per host on EC2, so host ~= container). Parsing of the flat /proc text files lives in a shared ElectricTelemetry.SystemMetrics.ProcfsParse helper. All reads are exhaustively defensive: missing files, EACCES on /proc//io, missing keys, and malformed values skip the affected metric rather than crash. meminfo and status Rss*/VmRSS values (kB) are converted to bytes; io read_bytes/write_bytes are already bytes and left unscaled. The reader no-ops cleanly on non-Linux. It is wired in as a thin proc_measurement/2 delegator on SystemMetrics, registered in ApplicationTelemetry.builtin_periodic_measurements/1 (read every ~5s tick) with last_value metric defs in metrics/1. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../telemetry/application_telemetry.ex | 16 +- .../lib/electric/telemetry/system_metrics.ex | 16 + .../telemetry/system_metrics/cgroup.ex | 21 +- .../electric/telemetry/system_metrics/proc.ex | 129 ++++++++ .../telemetry/system_metrics/procfs_parse.ex | 116 +++++++ .../telemetry/system_metrics/proc_test.exs | 290 ++++++++++++++++++ 6 files changed, 573 insertions(+), 15 deletions(-) create mode 100644 packages/electric-telemetry/lib/electric/telemetry/system_metrics/proc.ex create mode 100644 packages/electric-telemetry/lib/electric/telemetry/system_metrics/procfs_parse.ex create mode 100644 packages/electric-telemetry/test/electric/telemetry/system_metrics/proc_test.exs diff --git a/packages/electric-telemetry/lib/electric/telemetry/application_telemetry.ex b/packages/electric-telemetry/lib/electric/telemetry/application_telemetry.ex index 16145e6ce9..979421168d 100644 --- a/packages/electric-telemetry/lib/electric/telemetry/application_telemetry.ex +++ b/packages/electric-telemetry/lib/electric/telemetry/application_telemetry.ex @@ -95,7 +95,10 @@ defmodule ElectricTelemetry.ApplicationTelemetry do {ElectricTelemetry.SystemMetrics, :allocator_fragmentation_measurement, [telemetry_opts]}, # cgroup (v1/v2) accounting metrics (cgroup.*) live in # ElectricTelemetry.SystemMetrics.Cgroup; no-op when no cgroup is detected. - {ElectricTelemetry.SystemMetrics, :cgroup_measurement, [telemetry_opts]} + {ElectricTelemetry.SystemMetrics, :cgroup_measurement, [telemetry_opts]}, + # host/process /proc metrics (host.mem.*, host.proc.beam.*) live in + # ElectricTelemetry.SystemMetrics.Proc; no-op on non-Linux. + {ElectricTelemetry.SystemMetrics, :proc_measurement, [telemetry_opts]} ] end @@ -150,6 +153,17 @@ defmodule ElectricTelemetry.ApplicationTelemetry do last_value("cgroup.cpu.pressure.full.avg10"), last_value("cgroup.io.rbytes", unit: :byte), last_value("cgroup.io.wbytes", unit: :byte), + # host/process /proc metrics (emitted by + # ElectricTelemetry.SystemMetrics.Proc). + last_value("host.mem.total", unit: :byte), + last_value("host.mem.free", unit: :byte), + last_value("host.mem.cached", unit: :byte), + last_value("host.proc.beam.rss_anon", unit: :byte), + last_value("host.proc.beam.rss_file", unit: :byte), + last_value("host.proc.beam.rss_shmem", unit: :byte), + last_value("host.proc.beam.vm_rss", unit: :byte), + last_value("host.proc.beam.io.read_bytes", unit: :byte), + last_value("host.proc.beam.io.write_bytes", unit: :byte), sum("vm.monitor.long_message_queue.length", tags: [:process_type]), distribution("vm.monitor.long_schedule.timeout", tags: [:process_type], diff --git a/packages/electric-telemetry/lib/electric/telemetry/system_metrics.ex b/packages/electric-telemetry/lib/electric/telemetry/system_metrics.ex index 48183f2b75..303867f782 100644 --- a/packages/electric-telemetry/lib/electric/telemetry/system_metrics.ex +++ b/packages/electric-telemetry/lib/electric/telemetry/system_metrics.ex @@ -180,6 +180,22 @@ defmodule ElectricTelemetry.SystemMetrics do ElectricTelemetry.SystemMetrics.Cgroup.measurement(telemetry_opts, opts) end + @doc """ + Host/process `/proc` metrics, sampled every poll tick. + + Reads `/proc/meminfo` and the BEAM's own `/proc//status` and + `/proc//io` and emits the `host.mem.*` and `host.proc.beam.*` families. + Parsing lives in `ElectricTelemetry.SystemMetrics.Proc`; this is a thin + delegator so it registers like the other measurements. No-ops on non-Linux. + + Options are forwarded to `Proc.measurement/2` (`:proc_root`, `:pid` and `:os` + overrides for tests). + """ + @spec proc_measurement(map(), keyword()) :: :ok + def proc_measurement(telemetry_opts, opts \\ []) do + ElectricTelemetry.SystemMetrics.Proc.measurement(telemetry_opts, opts) + end + defp due?(opts) do if Keyword.get(opts, :force, false) do true diff --git a/packages/electric-telemetry/lib/electric/telemetry/system_metrics/cgroup.ex b/packages/electric-telemetry/lib/electric/telemetry/system_metrics/cgroup.ex index 64d6de7dab..c20d1bbe62 100644 --- a/packages/electric-telemetry/lib/electric/telemetry/system_metrics/cgroup.ex +++ b/packages/electric-telemetry/lib/electric/telemetry/system_metrics/cgroup.ex @@ -27,6 +27,7 @@ defmodule ElectricTelemetry.SystemMetrics.Cgroup do """ alias ElectricTelemetry.SystemMetrics + alias ElectricTelemetry.SystemMetrics.ProcfsParse @default_root "/sys/fs/cgroup" @@ -273,24 +274,16 @@ defmodule ElectricTelemetry.SystemMetrics.Cgroup do ## low-level file/number helpers ----------------------------------------- + # The format-agnostic file/integer primitives are shared with the /proc + # reader (ProcfsParse); the colon- vs space-delimited stat parsers above stay + # separate because they parse genuinely different file layouts. + # Read a single-integer file (memory.current, usage_in_bytes, cpuacct.usage). defp read_int_file(path), do: parse_int(read_raw_file(path)) - defp read_raw_file(path) do - case File.read(path) do - {:ok, content} -> String.trim(content) - {:error, _reason} -> nil - end - end - - defp parse_int(nil), do: nil + defp read_raw_file(path), do: ProcfsParse.read_raw_file(path) - defp parse_int(str) do - case Integer.parse(String.trim(str)) do - {int, ""} -> int - _ -> nil - end - end + defp parse_int(value), do: ProcfsParse.parse_int(value) defp parse_float(str) do case Float.parse(String.trim(str)) do diff --git a/packages/electric-telemetry/lib/electric/telemetry/system_metrics/proc.ex b/packages/electric-telemetry/lib/electric/telemetry/system_metrics/proc.ex new file mode 100644 index 0000000000..c83b53e7c5 --- /dev/null +++ b/packages/electric-telemetry/lib/electric/telemetry/system_metrics/proc.ex @@ -0,0 +1,129 @@ +defmodule ElectricTelemetry.SystemMetrics.Proc do + @moduledoc """ + Reads Linux `/proc` accounting files and emits `host.mem.*` and + `host.proc.beam.*` telemetry. + + This bridges the cgroup plane and the BEAM allocator plane: + + * `host.mem.*` (from `/proc/meminfo`) gives the host-RAM ceiling and cache + picture. On Electric Cloud there is one task per host (EC2), so host ≈ + container. + * `host.proc.beam.*` (from `/proc//status` and + `/proc//io`) gives the per-process RSS breakdown (anon vs file + vs shmem) and IO byte counters for *this* BEAM, so cgroup `anon` can be + shown to track the BEAM's anonymous footprint. + + All reads are defensive: a missing file, a permission error (`/proc//io` + can be EACCES-restricted even for self in some sandboxes), or unexpected + content results in the corresponding metric being skipped, never a crash. + + `/proc` only exists on Linux, so the measurement is a clean no-op on every + other platform. The OS is taken from + `ElectricTelemetry.SystemMetrics.system_info/0` (cached at boot) so we never + re-detect on the hot path. + + Units: + + * `/proc/meminfo` values and `/proc//status` `Rss*`/`VmRSS` are in kB + and converted to bytes (× 1024). + * `/proc//io` `read_bytes`/`write_bytes` are already bytes and emitted + as-is. + """ + + alias ElectricTelemetry.SystemMetrics + alias ElectricTelemetry.SystemMetrics.ProcfsParse + + @default_root "/proc" + + @doc """ + The default `/proc` filesystem root. Tests override this via the `:proc_root` + option on `measurement/2`. + """ + @spec default_root() :: String.t() + def default_root, do: @default_root + + @doc """ + Read the `/proc` accounting files and emit `host.mem.*` / `host.proc.beam.*` + telemetry events. + + Reads every tick (no slow-tier gating). Options: + + * `:proc_root` — override the `/proc` filesystem root (default + `#{@default_root}`); used by tests to point at fixture trees. + * `:pid` — override the OS pid of the BEAM whose per-process files are read + (default `:os.getpid()`); used by tests to point at `//`. + * `:os` — override the detected OS (`{family, name}` tuple); used by tests. + Defaults to the cached `SystemMetrics.system_info/0` value. + """ + @spec measurement(map(), keyword()) :: :ok + def measurement(_telemetry_opts, opts \\ []) do + os = Keyword.get_lazy(opts, :os, fn -> SystemMetrics.system_info().os end) + + case os do + {:unix, :linux} -> + root = Keyword.get(opts, :proc_root, @default_root) + pid = Keyword.get_lazy(opts, :pid, fn -> :os.getpid() end) |> to_string() + + emit_host_mem(root) + emit_beam_status(root, pid) + emit_beam_io(root, pid) + + _non_linux -> + :ok + end + + :ok + end + + ## host memory ----------------------------------------------------------- + + # /proc/meminfo is a flat ": kB" file. Read the keys we want and + # convert kB -> bytes. + defp emit_host_mem(root) do + info = ProcfsParse.read_meminfo(Path.join(root, "meminfo")) + + emit([:host, :mem], :total, kb_to_bytes(Map.get(info, "MemTotal"))) + emit([:host, :mem], :free, kb_to_bytes(Map.get(info, "MemFree"))) + emit([:host, :mem], :cached, kb_to_bytes(Map.get(info, "Cached"))) + end + + ## BEAM process status --------------------------------------------------- + + # /proc//status is a flat ":\t[ kB]" file. The Rss*/VmRSS + # values are in kB; convert to bytes. + defp emit_beam_status(root, pid) do + status = ProcfsParse.read_status(Path.join([root, pid, "status"])) + + emit([:host, :proc, :beam], :rss_anon, kb_to_bytes(Map.get(status, "RssAnon"))) + emit([:host, :proc, :beam], :rss_file, kb_to_bytes(Map.get(status, "RssFile"))) + emit([:host, :proc, :beam], :rss_shmem, kb_to_bytes(Map.get(status, "RssShmem"))) + emit([:host, :proc, :beam], :vm_rss, kb_to_bytes(Map.get(status, "VmRSS"))) + end + + ## BEAM process IO ------------------------------------------------------- + + # /proc//io is a flat ": " file with byte counters. May be + # EACCES-restricted; on any read failure we simply emit nothing. + defp emit_beam_io(root, pid) do + io = ProcfsParse.read_proc_io(Path.join([root, pid, "io"])) + + emit([:host, :proc, :beam, :io], :read_bytes, Map.get(io, "read_bytes")) + emit([:host, :proc, :beam, :io], :write_bytes, Map.get(io, "write_bytes")) + end + + ## emitting -------------------------------------------------------------- + + # Emit `event` carrying a single keyed measurement, unless the value is nil + # (file missing / unreadable / unparseable). + defp emit(_event, _key, nil), do: :ok + + defp emit(event, key, value) do + :telemetry.execute(event, %{key => value}) + :ok + end + + ## unit helpers ---------------------------------------------------------- + + defp kb_to_bytes(nil), do: nil + defp kb_to_bytes(kb), do: kb * 1024 +end diff --git a/packages/electric-telemetry/lib/electric/telemetry/system_metrics/procfs_parse.ex b/packages/electric-telemetry/lib/electric/telemetry/system_metrics/procfs_parse.ex new file mode 100644 index 0000000000..bdd7bedb81 --- /dev/null +++ b/packages/electric-telemetry/lib/electric/telemetry/system_metrics/procfs_parse.ex @@ -0,0 +1,116 @@ +defmodule ElectricTelemetry.SystemMetrics.ProcfsParse do + @moduledoc """ + Small, exhaustively defensive parsers for the flat `/proc` text files read by + `ElectricTelemetry.SystemMetrics.Proc`. + + Every function degrades to an empty map (or `nil` for scalars) on a missing + file, a permission error, or malformed content — it never raises. Parsing + `/proc` is Electric's #1 historical crash cause, so the contract here is + "skip the bad value, keep the good ones". + + No file content is ever turned into an atom (`String.to_atom/1`); keys stay + binaries and the caller looks them up against compile-time literals. + """ + + @doc """ + Parse `/proc/meminfo` into a map of `binary key => integer kB value`. + + Lines look like `MemTotal: 16384000 kB`. The trailing `kB` unit (and any + line that doesn't match `: [kB]`) is handled gracefully; values are + returned in their native kB (the caller converts to bytes). + """ + @spec read_meminfo(Path.t()) :: %{optional(binary()) => integer()} + def read_meminfo(path) do + case read_raw_file(path) do + nil -> %{} + content -> parse_keyed_kv(content) + end + end + + @doc """ + Parse `/proc//status` into a map of `binary key => integer kB value` for + the memory keys only. + + Lines look like `RssAnon:\t 12345 kB` or `VmRSS:\t 67890 kB`. Only lines + whose value parses as an integer are kept; non-numeric keys (e.g. `Name`, + `State`) are simply dropped. Values are returned in their native kB (the + caller converts to bytes). + """ + @spec read_status(Path.t()) :: %{optional(binary()) => integer()} + def read_status(path) do + case read_raw_file(path) do + nil -> %{} + content -> parse_keyed_kv(content) + end + end + + @doc """ + Parse `/proc//io` into a map of `binary key => integer byte value`. + + Lines look like `read_bytes: 12345`. This file can be permission-restricted + (EACCES) even for the reading process itself in some sandboxes; that read + failure degrades to an empty map. Values are already in bytes. + """ + @spec read_proc_io(Path.t()) :: %{optional(binary()) => integer()} + def read_proc_io(path) do + case read_raw_file(path) do + nil -> %{} + content -> parse_keyed_kv(content) + end + end + + # Parse a ":[ ]" file (status, io) into a map of + # binary key -> integer. The text after the colon is trimmed and split on + # spaces; the first token is taken as the value and any trailing unit (e.g. + # "kB") is ignored. Lines whose value doesn't parse as an integer are dropped. + defp parse_keyed_kv(content) do + content + |> String.split("\n", trim: true) + |> Enum.reduce(%{}, fn line, acc -> + case String.split(line, ":", parts: 2) do + [key, rest] -> + case rest |> String.trim() |> String.split(" ", trim: true) do + [value | _] -> + case parse_int(value) do + nil -> acc + int -> Map.put(acc, key, int) + end + + _ -> + acc + end + + _ -> + acc + end + end) + end + + ## low-level file/number helpers ----------------------------------------- + + @doc """ + Read a file, returning its trimmed content or `nil` on any error (missing + file, permission denied, …). + """ + @spec read_raw_file(Path.t()) :: binary() | nil + def read_raw_file(path) do + case File.read(path) do + {:ok, content} -> String.trim(content) + {:error, _reason} -> nil + end + end + + @doc """ + Parse a string as an integer, returning `nil` on `nil` input or any + non-integer content. + """ + @spec parse_int(binary() | nil) :: integer() | nil + def parse_int(nil), do: nil + + def parse_int(str) do + case Integer.parse(String.trim(str)) do + {int, ""} -> int + _ -> nil + end + end +end diff --git a/packages/electric-telemetry/test/electric/telemetry/system_metrics/proc_test.exs b/packages/electric-telemetry/test/electric/telemetry/system_metrics/proc_test.exs new file mode 100644 index 0000000000..928dbab1ef --- /dev/null +++ b/packages/electric-telemetry/test/electric/telemetry/system_metrics/proc_test.exs @@ -0,0 +1,290 @@ +defmodule ElectricTelemetry.SystemMetrics.ProcTest do + use ExUnit.Case, async: true + + @moduletag :tmp_dir + + alias ElectricTelemetry.SystemMetrics.Proc + + @pid "4242" + + # Attach handlers for every host.* event and collect emitted measurements + # into a flat map of metric-name (dotted string) => value. Single-key + # measurement maps make this unambiguous. + defp collect_host_metrics(fun) do + test_pid = self() + ref = make_ref() + + events = [ + [:host, :mem], + [:host, :proc, :beam], + [:host, :proc, :beam, :io] + ] + + handler_id = {__MODULE__, ref} + + :telemetry.attach_many( + handler_id, + events, + fn event, measurements, _meta, _ -> + send(test_pid, {ref, event, measurements}) + end, + nil + ) + + try do + fun.() + after + :telemetry.detach(handler_id) + end + + drain(ref, %{}) + end + + defp drain(ref, acc) do + receive do + {^ref, event, measurements} -> + name = event |> Enum.map(&Atom.to_string/1) |> Enum.join(".") + + acc = + Enum.reduce(measurements, acc, fn {k, v}, acc -> + Map.put(acc, "#{name}.#{k}", v) + end) + + drain(ref, acc) + after + 0 -> acc + end + end + + defp write_file(root, rel, content) do + path = Path.join(root, rel) + File.mkdir_p!(Path.dirname(path)) + File.write!(path, content) + end + + defp meminfo do + """ + MemTotal: 16384000 kB + MemFree: 2048000 kB + MemAvailable: 8192000 kB + Buffers: 102400 kB + Cached: 4096000 kB + SwapTotal: 0 kB + """ + end + + defp status do + """ + Name:\tbeam.smp + State:\tS (sleeping) + Pid:\t4242 + VmPeak:\t 2000000 kB + VmRSS:\t 500000 kB + RssAnon:\t 300000 kB + RssFile:\t 150000 kB + RssShmem:\t 50000 kB + Threads:\t42 + """ + end + + defp proc_io do + """ + rchar: 123456789 + wchar: 987654321 + syscr: 1000 + syscw: 2000 + read_bytes: 65536 + write_bytes: 131072 + cancelled_write_bytes: 0 + """ + end + + describe "linux /proc" do + setup %{tmp_dir: tmp_dir} do + root = Path.join(tmp_dir, "proc") + + write_file(root, "meminfo", meminfo()) + write_file(root, "#{@pid}/status", status()) + write_file(root, "#{@pid}/io", proc_io()) + + %{root: root} + end + + test "emits host memory metrics in bytes (kB * 1024)", %{root: root} do + m = + collect_host_metrics(fn -> + Proc.measurement(%{}, os: {:unix, :linux}, proc_root: root, pid: @pid) + end) + + assert m["host.mem.total"] == 16_384_000 * 1024 + assert m["host.mem.free"] == 2_048_000 * 1024 + assert m["host.mem.cached"] == 4_096_000 * 1024 + end + + test "emits BEAM Rss*/VmRSS in bytes (kB * 1024)", %{root: root} do + m = + collect_host_metrics(fn -> + Proc.measurement(%{}, os: {:unix, :linux}, proc_root: root, pid: @pid) + end) + + assert m["host.proc.beam.rss_anon"] == 300_000 * 1024 + assert m["host.proc.beam.rss_file"] == 150_000 * 1024 + assert m["host.proc.beam.rss_shmem"] == 50_000 * 1024 + assert m["host.proc.beam.vm_rss"] == 500_000 * 1024 + end + + test "emits BEAM io read/write in bytes (no scaling)", %{root: root} do + m = + collect_host_metrics(fn -> + Proc.measurement(%{}, os: {:unix, :linux}, proc_root: root, pid: @pid) + end) + + assert m["host.proc.beam.io.read_bytes"] == 65_536 + assert m["host.proc.beam.io.write_bytes"] == 131_072 + end + + test "io read_bytes/write_bytes are not multiplied by 1024", %{root: root} do + m = + collect_host_metrics(fn -> + Proc.measurement(%{}, os: {:unix, :linux}, proc_root: root, pid: @pid) + end) + + refute m["host.proc.beam.io.read_bytes"] == 65_536 * 1024 + end + end + + describe "non-linux guard" do + test "no-ops and emits nothing on macOS", %{tmp_dir: tmp_dir} do + root = Path.join(tmp_dir, "proc") + write_file(root, "meminfo", meminfo()) + write_file(root, "#{@pid}/status", status()) + write_file(root, "#{@pid}/io", proc_io()) + + m = + collect_host_metrics(fn -> + assert :ok = + Proc.measurement(%{}, os: {:unix, :darwin}, proc_root: root, pid: @pid) + end) + + assert m == %{} + end + end + + describe "missing / malformed files" do + test "missing meminfo skips host.mem.* but still emits process metrics", %{tmp_dir: tmp_dir} do + root = Path.join(tmp_dir, "proc") + write_file(root, "#{@pid}/status", status()) + write_file(root, "#{@pid}/io", proc_io()) + + m = + collect_host_metrics(fn -> + assert :ok = Proc.measurement(%{}, os: {:unix, :linux}, proc_root: root, pid: @pid) + end) + + refute Map.has_key?(m, "host.mem.total") + refute Map.has_key?(m, "host.mem.free") + refute Map.has_key?(m, "host.mem.cached") + assert m["host.proc.beam.rss_anon"] == 300_000 * 1024 + assert m["host.proc.beam.io.read_bytes"] == 65_536 + end + + test "missing status file skips beam.rss_* but other metrics still emit", %{tmp_dir: tmp_dir} do + root = Path.join(tmp_dir, "proc") + write_file(root, "meminfo", meminfo()) + write_file(root, "#{@pid}/io", proc_io()) + + m = + collect_host_metrics(fn -> + assert :ok = Proc.measurement(%{}, os: {:unix, :linux}, proc_root: root, pid: @pid) + end) + + refute Map.has_key?(m, "host.proc.beam.rss_anon") + refute Map.has_key?(m, "host.proc.beam.vm_rss") + assert m["host.mem.total"] == 16_384_000 * 1024 + assert m["host.proc.beam.io.read_bytes"] == 65_536 + end + + test "missing/unreadable io file skips io metrics but memory metrics still emit", %{ + tmp_dir: tmp_dir + } do + root = Path.join(tmp_dir, "proc") + write_file(root, "meminfo", meminfo()) + write_file(root, "#{@pid}/status", status()) + # no io file written + + m = + collect_host_metrics(fn -> + assert :ok = Proc.measurement(%{}, os: {:unix, :linux}, proc_root: root, pid: @pid) + end) + + refute Map.has_key?(m, "host.proc.beam.io.read_bytes") + refute Map.has_key?(m, "host.proc.beam.io.write_bytes") + assert m["host.mem.total"] == 16_384_000 * 1024 + assert m["host.proc.beam.rss_anon"] == 300_000 * 1024 + end + + test "malformed value skips that metric, others still emitted", %{tmp_dir: tmp_dir} do + root = Path.join(tmp_dir, "proc") + + write_file(root, "meminfo", """ + MemTotal: notanumber kB + MemFree: 2048000 kB + Cached: 4096000 kB + """) + + write_file(root, "#{@pid}/status", """ + VmRSS:\t garbage kB + RssAnon:\t 300000 kB + """) + + write_file(root, "#{@pid}/io", """ + read_bytes: nope + write_bytes: 131072 + """) + + m = + collect_host_metrics(fn -> + assert :ok = Proc.measurement(%{}, os: {:unix, :linux}, proc_root: root, pid: @pid) + end) + + refute Map.has_key?(m, "host.mem.total") + assert m["host.mem.free"] == 2_048_000 * 1024 + assert m["host.mem.cached"] == 4_096_000 * 1024 + + refute Map.has_key?(m, "host.proc.beam.vm_rss") + assert m["host.proc.beam.rss_anon"] == 300_000 * 1024 + + refute Map.has_key?(m, "host.proc.beam.io.read_bytes") + assert m["host.proc.beam.io.write_bytes"] == 131_072 + end + + test "all files missing emits nothing, no crash", %{tmp_dir: tmp_dir} do + root = Path.join(tmp_dir, "empty") + + m = + collect_host_metrics(fn -> + assert :ok = Proc.measurement(%{}, os: {:unix, :linux}, proc_root: root, pid: @pid) + end) + + assert m == %{} + end + end + + describe "metric definitions" do + test "host.* metrics are present in ApplicationTelemetry.metrics/1" do + names = + ElectricTelemetry.ApplicationTelemetry.metrics(%{}) + |> Enum.map(& &1.name) + + assert [:host, :mem, :total] in names + assert [:host, :mem, :free] in names + assert [:host, :mem, :cached] in names + assert [:host, :proc, :beam, :rss_anon] in names + assert [:host, :proc, :beam, :rss_file] in names + assert [:host, :proc, :beam, :rss_shmem] in names + assert [:host, :proc, :beam, :vm_rss] in names + assert [:host, :proc, :beam, :io, :read_bytes] in names + assert [:host, :proc, :beam, :io, :write_bytes] in names + end + end +end From 0437f7fe88cd3610fc0df6e68da772363024189d Mon Sep 17 00:00:00 2001 From: Oleksii Sholik Date: Tue, 23 Jun 2026 16:40:12 +0200 Subject: [PATCH 04/13] Add per-directory top-N bucketing to disk usage walk Extend ElectricTelemetry.DiskUsage to optionally compute per-directory subtotals during the existing periodic disk walk (a single traversal, no second pass and no new timer). Disk.recursive_usage_grouped/3 returns {total, %{dir_name => bytes}} where directories are bucketed by name at a configurable depth. The returned total is byte-identical to recursive_usage/2, including the legacy reset-to-0 behaviour on an unreadable directory; the bucket map is a best-effort tally that only grows on regular files. DiskUsage gains :group_depth (default nil = no bucketing, preserving current behaviour) and :top_n (default 10) options, caches the top-N largest buckets in ETS, and exposes them via current_dirs/1. StackTelemetry wires group_depth: 4 (the electric shape storage layout ////) and defines the new electric.storage.dir.bytes metric tagged [:stack_id, :shape]. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../lib/electric/telemetry/disk_usage.ex | 56 ++++++++- .../lib/electric/telemetry/disk_usage/disk.ex | 85 +++++++++++++ .../lib/electric/telemetry/stack_telemetry.ex | 14 ++- .../telemetry/disk_usage/disk_test.exs | 116 ++++++++++++++++++ .../electric/telemetry/disk_usage_test.exs | 79 ++++++++++++ 5 files changed, 346 insertions(+), 4 deletions(-) create mode 100644 packages/electric-telemetry/test/electric/telemetry/disk_usage/disk_test.exs diff --git a/packages/electric-telemetry/lib/electric/telemetry/disk_usage.ex b/packages/electric-telemetry/lib/electric/telemetry/disk_usage.ex index 08167c42f9..f04f242290 100644 --- a/packages/electric-telemetry/lib/electric/telemetry/disk_usage.ex +++ b/packages/electric-telemetry/lib/electric/telemetry/disk_usage.ex @@ -4,6 +4,7 @@ defmodule ElectricTelemetry.DiskUsage do alias ElectricTelemetry.DiskUsage.Disk @default_update_period 60_000 + @default_top_n 10 def start_link(args) do {:ok, stack_id} = Keyword.fetch(args, :stack_id) @@ -26,6 +27,22 @@ defmodule ElectricTelemetry.DiskUsage do ArgumentError -> :pending end + @doc """ + Returns the top-N largest per-directory subtotals as a list of + `{dir_name, bytes}` tuples sorted descending by size, or `:pending` if no + measurement with grouping enabled has completed yet. + """ + def current_dirs(stack_id) do + table = name(stack_id) + + case :ets.lookup(table, :top_dirs) do + [{:top_dirs, dirs}] -> {:ok, dirs} + [] -> :pending + end + rescue + ArgumentError -> :pending + end + @impl GenServer def init(args) do {:ok, stack_id} = Keyword.fetch(args, :stack_id) @@ -47,9 +64,12 @@ defmodule ElectricTelemetry.DiskUsage do storage_dir: storage_dir, manual_refresh: Keyword.get(args, :manual_refresh, false), update_period: update_period, + group_depth: Keyword.get(args, :group_depth), + top_n: Keyword.get(args, :top_n, @default_top_n), usage_bytes: usage_bytes, updated_at: updated_at, measurement_duration: 0, + top_dirs: nil, timer: nil } |> ets_write() @@ -77,23 +97,47 @@ defmodule ElectricTelemetry.DiskUsage do end defp read_disk_usage(state) do - {duration, bytes} = + exclude = [usage_cache_file(state.storage_dir)] + + {duration, {bytes, top_dirs}} = :timer.tc( fn -> - Disk.recursive_usage(state.storage_dir, [usage_cache_file(state.storage_dir)]) + case state.group_depth do + nil -> + {Disk.recursive_usage(state.storage_dir, exclude), nil} + + depth when is_integer(depth) -> + {total, buckets} = + Disk.recursive_usage_grouped(state.storage_dir, exclude, depth) + + {total, top_n(buckets, state.top_n)} + end end, :millisecond ) updated_at = DateTime.utc_now() - %{state | usage_bytes: bytes, updated_at: updated_at, measurement_duration: duration} + %{ + state + | usage_bytes: bytes, + updated_at: updated_at, + measurement_duration: duration, + top_dirs: top_dirs + } |> ets_write() |> save_usage!() |> cancel_timer() |> schedule_update() end + # Returns the `n` largest `{name, bytes}` buckets, sorted descending by size. + defp top_n(buckets, n) do + buckets + |> Enum.sort_by(fn {_name, bytes} -> bytes end, :desc) + |> Enum.take(n) + end + defp ets_write(state) do %{ usage_bytes: usage_bytes, @@ -103,6 +147,12 @@ defmodule ElectricTelemetry.DiskUsage do } = state :ets.insert(table, {:usage_bytes, usage_bytes, updated_at, duration}) + + case Map.get(state, :top_dirs) do + nil -> :ok + dirs -> :ets.insert(table, {:top_dirs, dirs}) + end + state end diff --git a/packages/electric-telemetry/lib/electric/telemetry/disk_usage/disk.ex b/packages/electric-telemetry/lib/electric/telemetry/disk_usage/disk.ex index 49daab9289..1fd4b06aa6 100644 --- a/packages/electric-telemetry/lib/electric/telemetry/disk_usage/disk.ex +++ b/packages/electric-telemetry/lib/electric/telemetry/disk_usage/disk.ex @@ -3,10 +3,34 @@ defmodule ElectricTelemetry.DiskUsage.Disk do Record.defrecord(:file_info, Record.extract(:file_info, from_lib: "kernel/include/file.hrl")) + @doc """ + Recursively sum the size of all regular files under `path`, excluding any + paths listed in `exclude`. + """ def recursive_usage(path, exclude) do do_recursive_usage(path, MapSet.new(exclude), 0) end + @doc """ + Like `recursive_usage/2`, but in a single traversal also returns a map of + per-directory subtotals bucketed at `group_depth`. + + The returned total is byte-identical to `recursive_usage/2` for the same + `path`/`exclude`. The bucket map is keyed by the name (not full path) of each + directory found at exactly `group_depth` levels below `path` (the root `path` + itself is depth 0), with the value being the recursive size of every regular + file under that directory. Excluded paths contribute to neither the total nor + the buckets. + + Returns `{total_bytes, %{dir_name => bytes}}`. + """ + def recursive_usage_grouped(path, exclude, group_depth) + when is_integer(group_depth) and group_depth >= 0 do + # When grouping at the root (depth 0), the root's own basename is the bucket. + initial_key = if group_depth == 0, do: Path.basename(path), else: nil + do_recursive_usage_grouped(path, initial_key, MapSet.new(exclude), group_depth, 0, {0, %{}}) + end + def do_recursive_usage(path, exclude, acc) do case stat(path) do {:ok, file_info(size: size, type: :regular)} -> @@ -33,6 +57,67 @@ defmodule ElectricTelemetry.DiskUsage.Disk do end end + # Threads the running total `acc` through the traversal exactly as + # `do_recursive_usage/3` does — including its legacy semantics where an + # unreadable/non-regular entry resets the threaded total to 0 — so the + # returned total is byte-identical. The `buckets` map is carried alongside and + # only ever grows on regular files; it is intentionally NOT reset on those + # error paths (it is a best-effort per-bucket tally, while the total preserves + # exact backwards compatibility). + # + # `bucket_key` is the name of the ancestor directory sitting at `group_depth`, + # or `nil` until that depth is reached. + defp do_recursive_usage_grouped(path, bucket_key, exclude, group_depth, depth, {acc, buckets}) do + case stat(path) do + {:ok, file_info(size: size, type: :regular)} -> + if MapSet.member?(exclude, path) do + {acc, buckets} + else + {size + acc, add_to_bucket(buckets, bucket_key, size)} + end + + {:ok, file_info(type: :directory)} -> + case ls(path) do + {:ok, files} -> + Enum.reduce(files, {acc, buckets}, fn name, {acc, bucks} -> + # `name` is a charlist from :prim_file.list_dir/1 and sits at + # `depth + 1`. The bucket key is the directory name at exactly + # `group_depth`; once established it propagates to descendants. + child_bucket_key = + cond do + not is_nil(bucket_key) -> bucket_key + depth + 1 == group_depth -> List.to_string(name) + true -> nil + end + + do_recursive_usage_grouped( + Path.join(path, name), + child_bucket_key, + exclude, + group_depth, + depth + 1, + {acc, bucks} + ) + end) + + {:error, _} -> + {0, buckets} + end + + {:ok, _} -> + {0, buckets} + + {:error, _} -> + {0, buckets} + end + end + + defp add_to_bucket(buckets, nil, _size), do: buckets + + defp add_to_bucket(buckets, key, size) do + Map.update(buckets, key, size, &(&1 + size)) + end + defdelegate stat(path), to: :prim_file, as: :read_file_info defp ls(path) do diff --git a/packages/electric-telemetry/lib/electric/telemetry/stack_telemetry.ex b/packages/electric-telemetry/lib/electric/telemetry/stack_telemetry.ex index e8d6086110..7fba19c8f5 100644 --- a/packages/electric-telemetry/lib/electric/telemetry/stack_telemetry.ex +++ b/packages/electric-telemetry/lib/electric/telemetry/stack_telemetry.ex @@ -67,9 +67,20 @@ defmodule ElectricTelemetry.StackTelemetry do ] end + # The electric shape storage layout places a single shape's directory at + # `////`, i.e. depth 4 below the + # storage root (see Electric.ShapeCache.PureFileStorage.shape_data_dir/3). + # Bucketing the disk walk at this depth yields per-shape subtotals keyed by + # shape handle, which we emit as `electric.storage.dir.bytes` for the top-N + # largest shapes. + @shape_dir_group_depth 4 + defp disk_usage_child_specs(%{stack_id: stack_id} = opts) do if storage_dir = Map.get(opts, :storage_dir) do - [{ElectricTelemetry.DiskUsage, stack_id: stack_id, storage_dir: storage_dir}] + [ + {ElectricTelemetry.DiskUsage, + stack_id: stack_id, storage_dir: storage_dir, group_depth: @shape_dir_group_depth} + ] else [] end @@ -113,6 +124,7 @@ defmodule ElectricTelemetry.StackTelemetry do distribution("electric.storage.transaction_stored.replication_lag", unit: :millisecond), last_value("electric.storage.used.bytes", unit: :byte), distribution("electric.storage.used.measurement_duration", unit: :millisecond), + last_value("electric.storage.dir.bytes", unit: :byte, tags: [:stack_id, :shape]), counter("electric.postgres.replication.transaction_received.count"), sum("electric.postgres.replication.transaction_received.bytes", unit: :byte), sum("electric.storage.transaction_stored.bytes", unit: :byte), diff --git a/packages/electric-telemetry/test/electric/telemetry/disk_usage/disk_test.exs b/packages/electric-telemetry/test/electric/telemetry/disk_usage/disk_test.exs new file mode 100644 index 0000000000..87268600fc --- /dev/null +++ b/packages/electric-telemetry/test/electric/telemetry/disk_usage/disk_test.exs @@ -0,0 +1,116 @@ +defmodule ElectricTelemetry.DiskUsage.DiskTest do + use ExUnit.Case, async: true + + alias ElectricTelemetry.DiskUsage.Disk + + @moduletag :tmp_dir + + defp write_file(path, size) do + File.mkdir_p!(Path.dirname(path)) + File.write!(path, :binary.copy("0", size), [:raw, :binary]) + size + end + + describe "recursive_usage/2" do + test "sums all regular files recursively", %{tmp_dir: dir} do + write_file(Path.join(dir, "a/b/c/1.data"), 100) + write_file(Path.join(dir, "a/b/c/2.data"), 50) + write_file(Path.join(dir, "a/x/y/z/1.data"), 25) + + assert Disk.recursive_usage(dir, []) == 175 + end + + test "excludes listed files", %{tmp_dir: dir} do + write_file(Path.join(dir, "keep.data"), 100) + excluded = Path.join(dir, "skip.data") + write_file(excluded, 999) + + assert Disk.recursive_usage(dir, [excluded]) == 100 + end + end + + describe "recursive_usage_grouped/3" do + test "total is byte-identical to recursive_usage/2 from a single walk", %{tmp_dir: dir} do + write_file(Path.join(dir, "s1/p/q/shape-a/1.data"), 100) + write_file(Path.join(dir, "s1/p/q/shape-a/2.data"), 200) + write_file(Path.join(dir, "s1/p/q/shape-b/1.data"), 50) + write_file(Path.join(dir, "loose.data"), 7) + + {total, _buckets} = Disk.recursive_usage_grouped(dir, [], 4) + assert total == Disk.recursive_usage(dir, []) + assert total == 357 + end + + test "buckets by directory name at the configured depth", %{tmp_dir: dir} do + # Layout: /////... => shape at depth 4 + write_file(Path.join(dir, "stack/aa/bb/shape-a/log/1.data"), 100) + write_file(Path.join(dir, "stack/aa/bb/shape-a/log/2.data"), 200) + write_file(Path.join(dir, "stack/aa/bb/shape-b/snap/1.data"), 50) + write_file(Path.join(dir, "stack/cc/dd/shape-c/1.data"), 10) + + {total, buckets} = Disk.recursive_usage_grouped(dir, [], 4) + + assert total == 360 + assert buckets == %{"shape-a" => 300, "shape-b" => 50, "shape-c" => 10} + end + + test "depth 0 buckets the whole tree under the root's basename", %{tmp_dir: dir} do + write_file(Path.join(dir, "x/1.data"), 5) + write_file(Path.join(dir, "y/1.data"), 5) + + {total, buckets} = Disk.recursive_usage_grouped(dir, [], 0) + assert total == 10 + assert buckets == %{Path.basename(dir) => 10} + end + + test "excluded files contribute to neither total nor buckets", %{tmp_dir: dir} do + write_file(Path.join(dir, "stack/aa/bb/shape-a/1.data"), 100) + excluded = Path.join(dir, "stack/aa/bb/shape-a/skip.data") + write_file(excluded, 999) + + {total, buckets} = Disk.recursive_usage_grouped(dir, [excluded], 4) + assert total == 100 + assert buckets == %{"shape-a" => 100} + end + + test "unreadable / missing directory is skipped and contributes 0", %{tmp_dir: dir} do + write_file(Path.join(dir, "stack/aa/bb/shape-a/1.data"), 100) + missing = Path.join(dir, "stack/aa/bb/does-not-exist") + + # A nonexistent path stat fails -> contributes 0, no crash. + {total, buckets} = Disk.recursive_usage_grouped(missing, [], 4) + assert total == 0 + assert buckets == %{} + + # And the rest of a valid tree is still counted. + {total, buckets} = Disk.recursive_usage_grouped(dir, [], 4) + assert total == 100 + assert buckets == %{"shape-a" => 100} + end + + test "total stays byte-identical to recursive_usage/2 when a dir is unreadable", + %{tmp_dir: dir} do + write_file(Path.join(dir, "stack/aa/bb/shape-a/1.data"), 100) + bad = Path.join(dir, "stack/aa/bb/shape-b") + write_file(Path.join(bad, "1.data"), 50) + File.chmod!(bad, 0o000) + + on_exit(fn -> File.chmod(bad, 0o755) end) + + legacy = Disk.recursive_usage(dir, []) + {grouped, _buckets} = Disk.recursive_usage_grouped(dir, [], 4) + + # The grouped total must match the legacy total bit-for-bit, including the + # legacy reset-to-0 behaviour on an unreadable directory. + assert grouped == legacy + end + + test "directory with no files at the bucket depth yields no bucket", %{tmp_dir: dir} do + File.mkdir_p!(Path.join(dir, "stack/aa/bb/empty-shape")) + + {total, buckets} = Disk.recursive_usage_grouped(dir, [], 4) + assert total == 0 + assert buckets == %{} + end + end +end diff --git a/packages/electric-telemetry/test/electric/telemetry/disk_usage_test.exs b/packages/electric-telemetry/test/electric/telemetry/disk_usage_test.exs index c712342bf4..cd522912d1 100644 --- a/packages/electric-telemetry/test/electric/telemetry/disk_usage_test.exs +++ b/packages/electric-telemetry/test/electric/telemetry/disk_usage_test.exs @@ -70,6 +70,85 @@ defmodule ElectricTelemetry.DiskUsageTest do :ok = DiskUsage.update(ctx.usage) end + describe "per-directory grouping (top-N)" do + # Create a shape dir at depth 4 (///) with the + # given total size spread over one file. + defp make_shape(storage_dir, handle, bytes) do + base = Path.join([storage_dir, "stack", "aa", "bb", handle]) + File.mkdir_p!(base) + File.write!(Path.join(base, "1.data"), :binary.copy("0", bytes), [:raw, :binary]) + bytes + end + + @tag start_usage: false + test "emits only the top-N largest shapes", ctx do + # 5 shapes with distinct sizes, ask for top 3. + make_shape(ctx.tmp_dir, "shape-1", 100) + make_shape(ctx.tmp_dir, "shape-2", 500) + make_shape(ctx.tmp_dir, "shape-3", 300) + make_shape(ctx.tmp_dir, "shape-4", 50) + make_shape(ctx.tmp_dir, "shape-5", 400) + + ctx = start_usage_grouped(ctx, group_depth: 4, top_n: 3) + :ok = DiskUsage.update(ctx.usage) + + assert {:ok, 1350, _} = DiskUsage.current(ctx.stack_id) + + assert {:ok, top} = DiskUsage.current_dirs(ctx.stack_id) + # Sorted desc by size, only the 3 largest, smaller ones dropped. + assert top == [{"shape-2", 500}, {"shape-5", 400}, {"shape-3", 300}] + handles = Enum.map(top, &elem(&1, 0)) + refute "shape-1" in handles + refute "shape-4" in handles + end + + @tag start_usage: false + test "tag values are the shape_handle dir names", ctx do + make_shape(ctx.tmp_dir, "abcd-1234-handle", 10) + + ctx = start_usage_grouped(ctx, group_depth: 4, top_n: 10) + :ok = DiskUsage.update(ctx.usage) + + assert {:ok, [{"abcd-1234-handle", 10}]} = DiskUsage.current_dirs(ctx.stack_id) + end + + @tag start_usage: false + test "grouping disabled by default leaves current_dirs pending", ctx do + make_shape(ctx.tmp_dir, "shape-1", 10) + ctx = start_usage(ctx) + :ok = DiskUsage.update(ctx.usage) + + assert {:ok, 10, _} = DiskUsage.current(ctx.stack_id) + assert :pending = DiskUsage.current_dirs(ctx.stack_id) + end + + @tag start_usage: false + test "total stays correct alongside grouping", ctx do + total = + make_shape(ctx.tmp_dir, "s1", 100) + + make_shape(ctx.tmp_dir, "s2", 250) + + ctx = start_usage_grouped(ctx, group_depth: 4) + :ok = DiskUsage.update(ctx.usage) + + assert {:ok, ^total, _} = DiskUsage.current(ctx.stack_id) + end + end + + defp start_usage_grouped(ctx, opts) do + {:ok, usage_pid} = + DiskUsage.start_link( + [ + stack_id: ctx.stack_id, + storage_dir: ctx.tmp_dir, + manual_refresh: true, + update_period: 1_000 + ] ++ opts + ) + + Map.put(ctx, :usage, usage_pid) + end + defp stop_usage(ctx) do ref = Process.monitor(ctx.usage) Process.unlink(ctx.usage) From 8b5a4fc2a7189cf1cfa83cacd1d88a8c6a92ca74 Mon Sep 17 00:00:00 2001 From: Oleksii Sholik Date: Tue, 23 Jun 2026 16:40:19 +0200 Subject: [PATCH 05/13] Emit per-shape disk usage metric for top-N largest shapes report_disk_usage/2 now also reads DiskUsage.current_dirs/1 and emits electric.storage.dir.bytes (tagged by shape handle) for the top-N largest shapes, reusing the existing periodic disk-usage walk and its emission cadence. Cardinality is bounded to the top-N buckets. Co-Authored-By: Claude Opus 4.8 (1M context) --- .changeset/per-shape-disk-usage-metric.md | 6 ++++++ .../electric/stack_supervisor/telemetry.ex | 21 +++++++++++++++++++ 2 files changed, 27 insertions(+) create mode 100644 .changeset/per-shape-disk-usage-metric.md diff --git a/.changeset/per-shape-disk-usage-metric.md b/.changeset/per-shape-disk-usage-metric.md new file mode 100644 index 0000000000..4f37ce8c26 --- /dev/null +++ b/.changeset/per-shape-disk-usage-metric.md @@ -0,0 +1,6 @@ +--- +"@core/electric-telemetry": patch +"@core/sync-service": patch +--- + +Export in-app BEAM allocator (`vm.alloc.*`), cgroup (`cgroup.*`), and host/process (`host.mem.*`, `host.proc.beam.*`) metrics for reconciling VM-reported memory/CPU/disk against what the kernel charges. Also emit a new `electric.storage.dir.bytes` stack-level metric reporting on-disk size for the top-N largest shapes (tagged by shape handle), computed during the existing periodic disk-usage walk so it adds no extra filesystem traversal. diff --git a/packages/sync-service/lib/electric/stack_supervisor/telemetry.ex b/packages/sync-service/lib/electric/stack_supervisor/telemetry.ex index ea35dfe3f3..30c7772a78 100644 --- a/packages/sync-service/lib/electric/stack_supervisor/telemetry.ex +++ b/packages/sync-service/lib/electric/stack_supervisor/telemetry.ex @@ -161,6 +161,27 @@ defmodule Electric.StackSupervisor.Telemetry do :pending -> :ok end + + report_per_shape_disk_usage(stack_id) + end + + # Emit `electric.storage.dir.bytes` for the top-N largest shapes only (the + # disk walk bounds cardinality by keeping just the top-N buckets). Each + # series is tagged with the shape handle. + defp report_per_shape_disk_usage(stack_id) do + case ElectricTelemetry.DiskUsage.current_dirs(stack_id) do + {:ok, top_dirs} -> + Enum.each(top_dirs, fn {shape_handle, bytes} -> + Electric.Telemetry.OpenTelemetry.execute( + [:electric, :storage, :dir], + %{bytes: bytes}, + %{stack_id: stack_id, shape: shape_handle} + ) + end) + + :pending -> + :ok + end end else def report_disk_usage(_stack_id, _telemetry_opts) do From 7a1ae9b391aba6dabf736900c2ae0573d68cf92e Mon Sep 17 00:00:00 2001 From: Oleksii Sholik Date: Wed, 24 Jun 2026 10:58:52 +0200 Subject: [PATCH 06/13] Take shape-dir group depth from caller, keep telemetry generic The shape-handle directory depth relative to the disk-usage walk root differs by deployment: standalone walks the bare storage dir with shapes nested under shapes//// (depth 5), while cloud walks the per-tenant shapes dir directly (depth 4). The previous hard-coded depth of 4 in the generic telemetry package was therefore wrong for standalone, where it bucketed the per-shape electric.storage.dir.bytes metric by the 2-char hash-shard prefix (aggregating many shapes) instead of by shape handle. StackTelemetry now reads :shape_dir_group_depth from its opts, supplied by the electric-side caller, which computes it from the path that PureFileStorage.shape_data_dir/3 actually produces for a sample handle, measured relative to the walk root. This stays correct if the storage sharding scheme changes, and passes nil through to disable grouping for storage backends without an on-disk shape layout. Cross-stack attribution is a non-issue: each stack's walk root is already its own subtree, so a stack only ever buckets its own shapes. Also document the transient full-map + O(n log n) sort in top_n/2 as a known tradeoff that is fine at ~10k shapes per stack, and use a realistic two-level-sharded fixture layout in the tests with a regression guard asserting a too-shallow depth buckets by the hash-shard prefix instead of the shape handle. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../lib/electric/telemetry/disk_usage.ex | 10 +++ .../lib/electric/telemetry/opts.ex | 4 ++ .../lib/electric/telemetry/stack_telemetry.ex | 20 +++--- .../electric/telemetry/disk_usage_test.exs | 71 +++++++++++++------ .../electric/stack_supervisor/telemetry.ex | 43 +++++++++++ packages/sync-service/mix.lock | 1 + .../stack_supervisor/telemetry_test.exs | 65 +++++++++++++++++ 7 files changed, 183 insertions(+), 31 deletions(-) diff --git a/packages/electric-telemetry/lib/electric/telemetry/disk_usage.ex b/packages/electric-telemetry/lib/electric/telemetry/disk_usage.ex index f04f242290..4b2a4ab44c 100644 --- a/packages/electric-telemetry/lib/electric/telemetry/disk_usage.ex +++ b/packages/electric-telemetry/lib/electric/telemetry/disk_usage.ex @@ -132,6 +132,16 @@ defmodule ElectricTelemetry.DiskUsage do end # Returns the `n` largest `{name, bytes}` buckets, sorted descending by size. + # + # Scale tradeoff: the full `%{dir => bytes}` map is materialized for the whole + # tree and sorted (O(k log k) in the number of buckets `k`) once per walk + # before being trimmed to the top `n`. Only the top `n` are retained in state + # and ETS, so steady-state memory stays bounded; the transient cost is the + # full map plus one sort per ~60s cycle. This is comfortable at ~10k shapes + # per stack and acceptable into the low hundreds of thousands. If a single + # stack ever holds enough shapes for that transient sort to cause real GC + # pressure, replace this with a running bounded top-N (min-heap) that never + # materializes the full sorted list. defp top_n(buckets, n) do buckets |> Enum.sort_by(fn {_name, bytes} -> bytes end, :desc) diff --git a/packages/electric-telemetry/lib/electric/telemetry/opts.ex b/packages/electric-telemetry/lib/electric/telemetry/opts.ex index fedd966578..08bb8983ff 100644 --- a/packages/electric-telemetry/lib/electric/telemetry/opts.ex +++ b/packages/electric-telemetry/lib/electric/telemetry/opts.ex @@ -5,6 +5,10 @@ defmodule ElectricTelemetry.Opts do installation_id: [type: :string], stack_id: [type: :string], storage_dir: [type: :string], + # Directory depth (relative to the disk-usage walk root) at which a single + # shape's directory sits, used to bucket the per-shape + # `electric.storage.dir.bytes` metric. `nil` disables per-shape grouping. + shape_dir_group_depth: [type: {:or, [:pos_integer, nil]}, default: nil], version: [type: :string, required: true], reporters: [ type: :keyword_list, diff --git a/packages/electric-telemetry/lib/electric/telemetry/stack_telemetry.ex b/packages/electric-telemetry/lib/electric/telemetry/stack_telemetry.ex index 7fba19c8f5..9bfd3f5b41 100644 --- a/packages/electric-telemetry/lib/electric/telemetry/stack_telemetry.ex +++ b/packages/electric-telemetry/lib/electric/telemetry/stack_telemetry.ex @@ -67,19 +67,21 @@ defmodule ElectricTelemetry.StackTelemetry do ] end - # The electric shape storage layout places a single shape's directory at - # `////`, i.e. depth 4 below the - # storage root (see Electric.ShapeCache.PureFileStorage.shape_data_dir/3). - # Bucketing the disk walk at this depth yields per-shape subtotals keyed by - # shape handle, which we emit as `electric.storage.dir.bytes` for the top-N - # largest shapes. - @shape_dir_group_depth 4 - + # `:shape_dir_group_depth` is the directory depth (relative to the disk-usage + # walk root) at which a single shape's directory sits, supplied by the caller + # since it depends on the deployment's storage layout (the generic telemetry + # package stays layout-agnostic). When set, the disk walk additionally buckets + # per-shape subtotals keyed by shape handle and emits them as + # `electric.storage.dir.bytes` for the top-N largest shapes. When `nil` (e.g. + # storage backends without an on-disk shape layout), only the aggregate total + # is measured. defp disk_usage_child_specs(%{stack_id: stack_id} = opts) do if storage_dir = Map.get(opts, :storage_dir) do [ {ElectricTelemetry.DiskUsage, - stack_id: stack_id, storage_dir: storage_dir, group_depth: @shape_dir_group_depth} + stack_id: stack_id, + storage_dir: storage_dir, + group_depth: Map.get(opts, :shape_dir_group_depth)} ] else [] diff --git a/packages/electric-telemetry/test/electric/telemetry/disk_usage_test.exs b/packages/electric-telemetry/test/electric/telemetry/disk_usage_test.exs index cd522912d1..89c81ba0b9 100644 --- a/packages/electric-telemetry/test/electric/telemetry/disk_usage_test.exs +++ b/packages/electric-telemetry/test/electric/telemetry/disk_usage_test.exs @@ -71,50 +71,77 @@ defmodule ElectricTelemetry.DiskUsageTest do end describe "per-directory grouping (top-N)" do - # Create a shape dir at depth 4 (///) with the - # given total size spread over one file. - defp make_shape(storage_dir, handle, bytes) do - base = Path.join([storage_dir, "stack", "aa", "bb", handle]) + # Mirror the real electric shape storage layout, where DiskUsage walks the + # per-stack `shapes` root and a shape lives at + # `////` (the two-level shard is + # the first two char-pairs of the handle; see + # Electric.ShapeCache.PureFileStorage.shape_data_dir/3). The shape handle + # therefore sits at depth 4 below the walk root. + @shape_dir_depth 4 + + defp make_shape(walk_root, stack_id, handle, bytes) do + <> = handle + base = Path.join([walk_root, stack_id, p1, p2, handle]) File.mkdir_p!(base) File.write!(Path.join(base, "1.data"), :binary.copy("0", bytes), [:raw, :binary]) bytes end + defp make_shape(ctx, handle, bytes) when is_map(ctx) do + make_shape(ctx.tmp_dir, ctx.stack_id, handle, bytes) + end + @tag start_usage: false test "emits only the top-N largest shapes", ctx do - # 5 shapes with distinct sizes, ask for top 3. - make_shape(ctx.tmp_dir, "shape-1", 100) - make_shape(ctx.tmp_dir, "shape-2", 500) - make_shape(ctx.tmp_dir, "shape-3", 300) - make_shape(ctx.tmp_dir, "shape-4", 50) - make_shape(ctx.tmp_dir, "shape-5", 400) - - ctx = start_usage_grouped(ctx, group_depth: 4, top_n: 3) + # 5 shapes with distinct handles and sizes, ask for top 3. + make_shape(ctx, "aaaa1111", 100) + make_shape(ctx, "bbbb2222", 500) + make_shape(ctx, "cccc3333", 300) + make_shape(ctx, "dddd4444", 50) + make_shape(ctx, "eeee5555", 400) + + ctx = start_usage_grouped(ctx, group_depth: @shape_dir_depth, top_n: 3) :ok = DiskUsage.update(ctx.usage) assert {:ok, 1350, _} = DiskUsage.current(ctx.stack_id) assert {:ok, top} = DiskUsage.current_dirs(ctx.stack_id) # Sorted desc by size, only the 3 largest, smaller ones dropped. - assert top == [{"shape-2", 500}, {"shape-5", 400}, {"shape-3", 300}] + assert top == [{"bbbb2222", 500}, {"eeee5555", 400}, {"cccc3333", 300}] handles = Enum.map(top, &elem(&1, 0)) - refute "shape-1" in handles - refute "shape-4" in handles + refute "aaaa1111" in handles + refute "dddd4444" in handles end @tag start_usage: false test "tag values are the shape_handle dir names", ctx do - make_shape(ctx.tmp_dir, "abcd-1234-handle", 10) + make_shape(ctx, "abcd1234handle", 10) + + ctx = start_usage_grouped(ctx, group_depth: @shape_dir_depth, top_n: 10) + :ok = DiskUsage.update(ctx.usage) - ctx = start_usage_grouped(ctx, group_depth: 4, top_n: 10) + assert {:ok, [{"abcd1234handle", 10}]} = DiskUsage.current_dirs(ctx.stack_id) + end + + @tag start_usage: false + test "wrong (too shallow) group_depth buckets by hash-prefix, not handle", ctx do + # Regression guard for the off-by-one bug: bucketing one level too shallow + # keys by the 2-char shard prefix ``, aggregating many shapes into one + # bucket instead of reporting per shape. Two shapes that share a shard + # prefix must collapse together at the wrong depth. + make_shape(ctx, "zzaa1111", 10) + make_shape(ctx, "zzaa2222", 20) + + # @shape_dir_depth - 1 == bucket by ("aa"), which both shapes share. + ctx = start_usage_grouped(ctx, group_depth: @shape_dir_depth - 1, top_n: 10) :ok = DiskUsage.update(ctx.usage) - assert {:ok, [{"abcd-1234-handle", 10}]} = DiskUsage.current_dirs(ctx.stack_id) + assert {:ok, [{"aa", 30}]} = DiskUsage.current_dirs(ctx.stack_id) end @tag start_usage: false test "grouping disabled by default leaves current_dirs pending", ctx do - make_shape(ctx.tmp_dir, "shape-1", 10) + make_shape(ctx, "aaaa1111", 10) ctx = start_usage(ctx) :ok = DiskUsage.update(ctx.usage) @@ -125,10 +152,10 @@ defmodule ElectricTelemetry.DiskUsageTest do @tag start_usage: false test "total stays correct alongside grouping", ctx do total = - make_shape(ctx.tmp_dir, "s1", 100) + - make_shape(ctx.tmp_dir, "s2", 250) + make_shape(ctx, "aaaa1111", 100) + + make_shape(ctx, "bbbb2222", 250) - ctx = start_usage_grouped(ctx, group_depth: 4) + ctx = start_usage_grouped(ctx, group_depth: @shape_dir_depth) :ok = DiskUsage.update(ctx.usage) assert {:ok, ^total, _} = DiskUsage.current(ctx.stack_id) diff --git a/packages/sync-service/lib/electric/stack_supervisor/telemetry.ex b/packages/sync-service/lib/electric/stack_supervisor/telemetry.ex index 30c7772a78..24875b5e6b 100644 --- a/packages/sync-service/lib/electric/stack_supervisor/telemetry.ex +++ b/packages/sync-service/lib/electric/stack_supervisor/telemetry.ex @@ -221,6 +221,7 @@ defmodule Electric.StackSupervisor.Telemetry do config.telemetry_opts |> Keyword.put(:stack_id, config.stack_id) |> Keyword.put(:storage_dir, config.storage_dir) + |> Keyword.put(:shape_dir_group_depth, shape_dir_group_depth(config)) |> Keyword.put(:otel_opts, otel_opts) # Always enable default periodic measurements in addition to the user-provided ones |> Keyword.update( @@ -239,6 +240,48 @@ defmodule Electric.StackSupervisor.Telemetry do {ElectricTelemetry.StackTelemetry, telemetry_opts} end + @doc """ + Computes the directory depth, relative to the disk-usage walk root + (`config.storage_dir`), at which a single shape's directory sits. Used to + bucket the per-shape `electric.storage.dir.bytes` metric by shape handle. + + The walk root and the shape storage root differ between deployments: + + * standalone sync-service walks the bare `ELECTRIC_STORAGE_DIR` while + PureFileStorage is configured with `/shapes`, so a shape + lives at `/shapes////` (depth 5); + * cloud (multi-tenant) walks the per-tenant `//shapes` + directly, so a shape lives at `///` relative + to that walk root (depth 4). + + Rather than hard-code either number, the depth is derived from the actual + path that `Electric.ShapeCache.PureFileStorage.shape_data_dir/3` produces for + a sample shape handle, so it stays correct if the storage sharding scheme + changes. Returns `nil` (grouping disabled) for storage backends without an + on-disk shape layout. + """ + def shape_dir_group_depth(config) do + with {Electric.ShapeCache.PureFileStorage, storage_opts} <- config.storage, + shape_storage_dir when is_binary(shape_storage_dir) <- + Keyword.get(storage_opts, :storage_dir) do + base_path = Path.join(shape_storage_dir, config.stack_id) + # Use a syntactically valid sample handle (>= 4 chars) so the real + # path-building code determines the depth, then measure it relative to + # the walk root. + sample_handle = "00000000-0000" + shape_dir = Electric.ShapeCache.PureFileStorage.shape_data_dir(base_path, sample_handle) + path_depth(shape_dir) - path_depth(config.storage_dir) + else + _ -> nil + end + end + + defp path_depth(path) do + path + |> Path.split() + |> length() + end + defp default_metrics_from_periodic_measurements do [ Telemetry.Metrics.last_value("electric.shapes.total_shapes.count"), diff --git a/packages/sync-service/mix.lock b/packages/sync-service/mix.lock index 91befd7aa7..524db607b5 100644 --- a/packages/sync-service/mix.lock +++ b/packages/sync-service/mix.lock @@ -52,6 +52,7 @@ "postgrex": {:hex, :postgrex, "0.22.2", "4aec14df2a72722aee92492566edbeeb44e233ecb86b1915d03136297ef1385d", [:mix], [{:db_connection, "~> 2.9", [hex: :db_connection, repo: "hexpm", optional: false]}, {:decimal, "~> 1.5 or ~> 2.0 or ~> 3.0", [hex: :decimal, repo: "hexpm", optional: false]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: true]}, {:table, "~> 0.1.0", [hex: :table, repo: "hexpm", optional: true]}], "hexpm", "8946382ddb06294f56026ac4278b3cc212bac8a2c82ed68b4087819ed1abc53b"}, "protobuf": {:hex, :protobuf, "0.13.0", "7a9d9aeb039f68a81717eb2efd6928fdf44f03d2c0dfdcedc7b560f5f5aae93d", [:mix], [{:jason, "~> 1.2", [hex: :jason, repo: "hexpm", optional: true]}], "hexpm", "21092a223e3c6c144c1a291ab082a7ead32821ba77073b72c68515aa51fef570"}, "protox": {:hex, :protox, "2.0.9", "5d95631413695639166ab82d3596a016cf2e974b45ce60b87fb926d6e1650977", [:mix], [], "hexpm", "d00d861815351b0fae88532f403f9dd40ade1e52d55728f7151a1f79fa61e9f1"}, + "recon": {:hex, :recon, "2.5.6", "9052588e83bfedfd9b72e1034532aee2a5369d9d9343b61aeb7fbce761010741", [:mix, :rebar3], [], "hexpm", "96c6799792d735cc0f0fd0f86267e9d351e63339cbe03df9d162010cefc26bb0"}, "remote_ip": {:hex, :remote_ip, "1.2.0", "fb078e12a44414f4cef5a75963c33008fe169b806572ccd17257c208a7bc760f", [:mix], [{:combine, "~> 0.10", [hex: :combine, repo: "hexpm", optional: false]}, {:plug, "~> 1.14", [hex: :plug, repo: "hexpm", optional: false]}], "hexpm", "2ff91de19c48149ce19ed230a81d377186e4412552a597d6a5137373e5877cb7"}, "repatch": {:hex, :repatch, "1.6.1", "e2bc55d7358de5727a7d989886d9aebd6497ccab4232b41dcae54f8fd6539c19", [:mix], [{:ex2ms, "~> 1.7.0", [hex: :ex2ms, repo: "hexpm", optional: false]}], "hexpm", "486f80b503fad097ae9ff33f3c7ad3a24c2bad85e694d76ff214e15d13cf9d25"}, "req": {:hex, :req, "0.6.1", "7b904c8b42d0e08136a5c6aba024fd12fc79a1ed8856e7a3522b0917f7e75113", [:mix], [{:brotli, "~> 0.3.1", [hex: :brotli, repo: "hexpm", optional: true]}, {:ezstd, "~> 1.0", [hex: :ezstd, repo: "hexpm", optional: true]}, {:finch, "~> 0.21.0 or ~> 0.22.0", [hex: :finch, repo: "hexpm", optional: false]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: false]}, {:mime, "~> 2.0.6 or ~> 2.1", [hex: :mime, repo: "hexpm", optional: false]}, {:nimble_csv, "~> 1.0", [hex: :nimble_csv, repo: "hexpm", optional: true]}, {:plug, "~> 1.0", [hex: :plug, repo: "hexpm", optional: true]}], "hexpm", "aaf11c9c80f2df2364630b3594e1857fe610d8ea7cb994e1ce3dcb55f204ff1c"}, diff --git a/packages/sync-service/test/electric/stack_supervisor/telemetry_test.exs b/packages/sync-service/test/electric/stack_supervisor/telemetry_test.exs index 5c543abf87..b2bc6edad1 100644 --- a/packages/sync-service/test/electric/stack_supervisor/telemetry_test.exs +++ b/packages/sync-service/test/electric/stack_supervisor/telemetry_test.exs @@ -130,5 +130,70 @@ if Electric.telemetry_enabled?() and Code.ensure_loaded?(ElectricTelemetry.Repor assert scrape =~ ~r/electric_admission_control_reject_count\{kind="shape"\} 2/ end end + + describe "shape_dir_group_depth/1" do + alias Electric.ShapeCache.PureFileStorage + + # The depth must land exactly on the shape-handle directory produced by the + # real storage code, for both deployment layouts. We derive the expected + # depth from PureFileStorage.shape_data_dir/3 rather than hard-coding it, so + # config/sharding drift can't silently re-introduce the off-by-one bug. + defp expected_depth(walk_root, shape_storage_dir, stack_id) do + base_path = Path.join(shape_storage_dir, stack_id) + shape_dir = PureFileStorage.shape_data_dir(base_path, "00000000-0000") + length(Path.split(shape_dir)) - length(Path.split(walk_root)) + end + + test "standalone layout: walk root is the bare storage dir, shapes nested under shapes/" do + # ELECTRIC_STORAGE_DIR walked directly; PureFileStorage at /shapes. + walk_root = "/var/electric" + shape_storage_dir = Path.join(walk_root, "shapes") + stack_id = "single-stack" + + config = %{ + stack_id: stack_id, + storage_dir: walk_root, + storage: {PureFileStorage, storage_dir: shape_storage_dir} + } + + depth = Telemetry.shape_dir_group_depth(config) + + # /shapes//// => depth 5. + assert depth == 5 + assert depth == expected_depth(walk_root, shape_storage_dir, stack_id) + # Guard the exact bug: bucketing at 4 would key by the shard prefix. + refute depth == 4 + end + + test "cloud layout: walk root is the per-tenant shapes dir" do + # Cloud passes the per-tenant //shapes as BOTH the walk + # root and the PureFileStorage storage_dir. + tenant_id = "tenant-abc" + walk_root = Path.join(["/var/electric", tenant_id, "shapes"]) + shape_storage_dir = walk_root + + config = %{ + stack_id: tenant_id, + storage_dir: walk_root, + storage: {PureFileStorage, storage_dir: shape_storage_dir} + } + + depth = Telemetry.shape_dir_group_depth(config) + + # //// => depth 4. + assert depth == 4 + assert depth == expected_depth(walk_root, shape_storage_dir, tenant_id) + end + + test "non-file storage backends disable grouping" do + config = %{ + stack_id: "s", + storage_dir: "/var/electric", + storage: {Electric.ShapeCache.InMemoryStorage, []} + } + + assert Telemetry.shape_dir_group_depth(config) == nil + end + end end end From 7a5f7f1804325cde445854efe2e837493d742209 Mon Sep 17 00:00:00 2001 From: Oleksii Sholik Date: Sun, 5 Jul 2026 00:32:17 +0200 Subject: [PATCH 07/13] Assert system metrics in otel-export integration test Extend the otel-export.lux test to assert that the new SystemMetrics families reach the collector end-to-end: vm.alloc.* (allocator carriers), host.mem.* / host.proc.beam.* (/proc), and cgroup.memory.current. Co-Authored-By: Claude Opus 4.8 (1M context) --- integration-tests/tests/otel-export.lux | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/integration-tests/tests/otel-export.lux b/integration-tests/tests/otel-export.lux index b325c34912..98eb23e5d6 100644 --- a/integration-tests/tests/otel-export.lux +++ b/integration-tests/tests/otel-export.lux @@ -101,6 +101,22 @@ # Verify the presence of process.memory.total metric [invoke grep_otel_collector_output "Metric process\.memory\.total .*process_type=A_memory_hog.*Value=[0-9]+"] + # Verify BEAM allocator metrics (vm.alloc.*) from SystemMetrics. These come from + # :recon_alloc and are cross-platform; they are sampled every poll tick. + [invoke grep_otel_collector_output "Metric vm\.alloc\.allocated .*Value=[0-9]+"] + [invoke grep_otel_collector_output "Metric vm\.alloc\.used .*Value=[0-9]+"] + [invoke grep_otel_collector_output "Metric vm\.alloc\.unused .*Value=[0-9]+"] + [invoke grep_otel_collector_output "Metric vm\.alloc\.carrier_usage .*Value=[.0-9]+"] + + # Verify host /proc metrics (host.mem.* / host.proc.beam.*) from SystemMetrics. + # These read /proc and are populated on Linux (the CI runner). + [invoke grep_otel_collector_output "Metric host\.mem\.total .*Value=[0-9]+"] + [invoke grep_otel_collector_output "Metric host\.proc\.beam\.vm_rss .*Value=[0-9]+"] + + # Verify cgroup accounting metrics (cgroup.*) from SystemMetrics. Populated when a + # cgroup is detected — cgroup v2 on the CI runner. + [invoke grep_otel_collector_output "Metric cgroup\.memory\.current .*Value=[0-9]+"] + # Verify that LSN metrics are exported [invoke grep_otel_collector_output "Metric electric\.postgres\.replication\.pg_wal_offset .*stack_id=single_stack"] [invoke grep_otel_collector_output "Metric electric\.postgres\.replication\.slot_confirmed_flush_lsn_lag .*stack_id=single_stack"] From 53d64b5b444fb0f246368184422be67a65a9a0c9 Mon Sep 17 00:00:00 2001 From: Oleksii Sholik Date: Tue, 7 Jul 2026 12:20:38 +0200 Subject: [PATCH 08/13] Drop cgroup v1 support Electric Cloud runs on cgroup v2 and the v1 path was a parallel read/emit implementation (per-controller subdirs, rss/cache key mapping, ns->us conversion, unlimited-limit sentinel) that doubled the reader's surface for hosts we no longer target. cgroup.* metrics are now v2-only; v1 hosts cleanly no-op like non-Linux platforms. This also lets cgroup version detection drop the `stat -fc` shell-out in favour of checking for /sys/fs/cgroup/cgroup.controllers, the canonical v2 marker, and memory.max no longer needs a sentinel filter (v2 spells unlimited as the literal "max", which already parses to nil and is skipped). Co-Authored-By: Claude Fable 5 --- .../lib/electric/telemetry/system_metrics.ex | 28 +---- .../telemetry/system_metrics/cgroup.ex | 117 +++--------------- .../telemetry/system_metrics/cgroup_test.exs | 95 -------------- 3 files changed, 25 insertions(+), 215 deletions(-) diff --git a/packages/electric-telemetry/lib/electric/telemetry/system_metrics.ex b/packages/electric-telemetry/lib/electric/telemetry/system_metrics.ex index 303867f782..deaffe0086 100644 --- a/packages/electric-telemetry/lib/electric/telemetry/system_metrics.ex +++ b/packages/electric-telemetry/lib/electric/telemetry/system_metrics.ex @@ -51,15 +51,10 @@ defmodule ElectricTelemetry.SystemMetrics do @doc """ Boot-time platform/cgroup detection, computed once and cached in `:persistent_term`. - Returns a map of the form `%{os: {family, name}, cgroup_version: :v1 | :v2 | :none}`. - - Cgroup version is detected by stat-ing the filesystem mounted at `/sys/fs/cgroup`: - a `cgroup2fs` filesystem indicates v2; `tmpfs`/`cgroup` indicates v1; anything else - (including non-Linux platforms or a missing mount) is reported as `:none`. - - Later tasks (cgroup/host readers) reuse this detection. + Returns a map of the form `%{os: {family, name}, cgroup_version: :v2 | :none}`. + Cgroup v1 hosts report `:none` — the `cgroup.*` metrics are v2-only. """ - @spec system_info() :: %{os: {atom(), atom()}, cgroup_version: :v1 | :v2 | :none} + @spec system_info() :: %{os: {atom(), atom()}, cgroup_version: :v2 | :none} def system_info do case :persistent_term.get(@system_info_key, :undefined) do :undefined -> @@ -77,22 +72,9 @@ defmodule ElectricTelemetry.SystemMetrics do %{os: os, cgroup_version: detect_cgroup_version(os)} end + # The controllers list at the cgroup fs root is the canonical v2 marker. defp detect_cgroup_version({:unix, :linux}) do - # `stat -fc %T` prints the filesystem type of the mount backing the path. - case System.cmd("stat", ["-fc", "%T", "/sys/fs/cgroup"], stderr_to_stdout: true) do - {output, 0} -> - case String.trim(output) do - "cgroup2fs" -> :v2 - "tmpfs" -> :v1 - "cgroup" -> :v1 - _ -> :none - end - - _ -> - :none - end - rescue - _ -> :none + if File.exists?("/sys/fs/cgroup/cgroup.controllers"), do: :v2, else: :none end defp detect_cgroup_version(_non_linux), do: :none diff --git a/packages/electric-telemetry/lib/electric/telemetry/system_metrics/cgroup.ex b/packages/electric-telemetry/lib/electric/telemetry/system_metrics/cgroup.ex index c20d1bbe62..abb8f24361 100644 --- a/packages/electric-telemetry/lib/electric/telemetry/system_metrics/cgroup.ex +++ b/packages/electric-telemetry/lib/electric/telemetry/system_metrics/cgroup.ex @@ -1,29 +1,16 @@ defmodule ElectricTelemetry.SystemMetrics.Cgroup do @moduledoc """ - Reads cgroup (v1 and v2) accounting files and emits `cgroup.*` telemetry. - - Electric Cloud runs on cgroup v2 (container at the root of its own cgroup - namespace), so v2 controllers are read directly from `/sys/fs/cgroup/*`. v1 is - the legacy/fallback layout (Fargate and other hosts) where controllers live - under per-controller subdirs (`/sys/fs/cgroup/memory/`, `/sys/fs/cgroup/cpu/`, - …). + Reads cgroup v2 accounting files from `/sys/fs/cgroup/*` and emits `cgroup.*` + telemetry. All reads are defensive: a missing file, a permission error, or unexpected content results in the corresponding metric being skipped, never a crash. The cgroup version is taken from `ElectricTelemetry.SystemMetrics.system_info/0` - (cached at boot) so we never stat the filesystem on the hot path. When the - version is `:none` (including all non-Linux platforms) the measurement is a - clean no-op. - - Units: - - * memory metrics are bytes - * cpu usage / throttled time are emitted in **microseconds**. v1 reports - these in nanoseconds (`cpuacct.usage`, `cpu.stat` `throttled_time`), so we - convert to microseconds for a unit consistent with v2's `usage_usec` / - `throttled_usec`. - * io metrics are bytes - * PSI `full avg10` is a stall percentage (float). PSI exists on v2 only. + (cached at boot) so we never stat the filesystem on the hot path. Anything + other than v2 (cgroup v1 hosts, non-Linux platforms) is a clean no-op. + + Units: memory and io metrics are bytes, cpu usage / throttled time are + microseconds, PSI `full avg10` is a stall percentage (float). """ alias ElectricTelemetry.SystemMetrics @@ -45,35 +32,25 @@ defmodule ElectricTelemetry.SystemMetrics.Cgroup do * `:cgroup_root` — override the cgroup filesystem root (default `#{@default_root}`); used by tests to point at fixture trees. - * `:cgroup_version` — override the detected cgroup version (`:v1`/`:v2`/ - `:none`); used by tests. Defaults to the cached - `SystemMetrics.system_info/0` value. + * `:cgroup_version` — override the detected cgroup version (`:v2`/`:none`); + used by tests. Defaults to the cached `SystemMetrics.system_info/0` value. """ @spec measurement(map(), keyword()) :: :ok def measurement(_telemetry_opts, opts \\ []) do version = Keyword.get_lazy(opts, :cgroup_version, fn -> SystemMetrics.system_info().cgroup_version end) - root = Keyword.get(opts, :cgroup_root, @default_root) - - case version do - :v2 -> read_v2(root) - :v1 -> read_v1(root) - :none -> :ok + if version == :v2 do + root = Keyword.get(opts, :cgroup_root, @default_root) + emit_memory(root) + emit_cpu(root) + emit_io(root) end :ok end - ## cgroup v2 ------------------------------------------------------------- - - defp read_v2(root) do - emit_v2_memory(root) - emit_v2_cpu(root) - emit_v2_io(root) - end - - defp emit_v2_memory(root) do + defp emit_memory(root) do current = read_int_file(Path.join(root, "memory.current")) stat = read_stat_file(Path.join(root, "memory.stat")) @@ -81,12 +58,14 @@ defmodule ElectricTelemetry.SystemMetrics.Cgroup do emit(:memory, :anon, Map.get(stat, "anon")) emit(:memory, :file, Map.get(stat, "file")) emit(:memory, :working_set, working_set(current, Map.get(stat, "inactive_file"))) - emit(:memory, :max, real_limit(read_raw_file(Path.join(root, "memory.max")))) + # An unlimited memory.max is the literal "max", which parses to nil and is + # skipped (the host-RAM ceiling comes from the /proc reader). + emit(:memory, :max, read_int_file(Path.join(root, "memory.max"))) emit_pressure(Path.join(root, "memory.pressure"), :memory) end - defp emit_v2_cpu(root) do + defp emit_cpu(root) do stat = read_stat_file(Path.join(root, "cpu.stat")) emit(:cpu, :usage_usec, Map.get(stat, "usage_usec")) @@ -96,52 +75,13 @@ defmodule ElectricTelemetry.SystemMetrics.Cgroup do emit_pressure(Path.join(root, "cpu.pressure"), :cpu) end - defp emit_v2_io(root) do + defp emit_io(root) do io = parse_io_stat(read_raw_file(Path.join(root, "io.stat"))) emit(:io, :rbytes, io.rbytes) emit(:io, :wbytes, io.wbytes) end - ## cgroup v1 ------------------------------------------------------------- - - defp read_v1(root) do - emit_v1_memory(root) - emit_v1_cpu(root) - # NOTE: v1 blkio accounting (blkio.throttle.io_service_bytes) is awkward and - # not present on the hosts we care about (Electric Cloud is v2). IO metrics - # are emitted on v2 only; v1 deliberately skips them. - end - - defp emit_v1_memory(root) do - current = read_int_file(Path.join(root, "memory/memory.usage_in_bytes")) - stat = read_stat_file(Path.join(root, "memory/memory.stat")) - - emit(:memory, :current, current) - emit(:memory, :anon, Map.get(stat, "rss")) - emit(:memory, :file, Map.get(stat, "cache")) - emit(:memory, :working_set, working_set(current, Map.get(stat, "inactive_file"))) - - emit( - :memory, - :max, - real_limit(read_raw_file(Path.join(root, "memory/memory.limit_in_bytes"))) - ) - - # v1 has no PSI; skip cgroup.memory.pressure.full.avg10. - end - - defp emit_v1_cpu(root) do - usage_ns = read_int_file(Path.join(root, "cpuacct/cpuacct.usage")) - emit(:cpu, :usage_usec, ns_to_us(usage_ns)) - - stat = read_stat_file(Path.join(root, "cpu/cpu.stat")) - emit(:cpu, :nr_throttled, Map.get(stat, "nr_throttled")) - emit(:cpu, :throttled_usec, ns_to_us(Map.get(stat, "throttled_time"))) - - # v1 has no PSI; skip cgroup.cpu.pressure.full.avg10. - end - ## emitting -------------------------------------------------------------- # Emit a `[:cgroup, plane]` event carrying a single keyed measurement, unless @@ -172,23 +112,6 @@ defmodule ElectricTelemetry.SystemMetrics.Cgroup do defp working_set(_current, nil), do: nil defp working_set(current, inactive_file), do: max(current - inactive_file, 0) - # A memory limit is only meaningful when it's a real number. v2 unlimited is - # the literal string "max"; v1 unlimited is a huge sentinel (~9.2e18). In both - # cases skip the metric (the host-RAM ceiling comes from /proc readers). - @v1_unlimited_threshold 0x7000_0000_0000_0000 - defp real_limit(nil), do: nil - - defp real_limit(raw) do - case parse_int(raw) do - nil -> nil - value when value >= @v1_unlimited_threshold -> nil - value -> value - end - end - - defp ns_to_us(nil), do: nil - defp ns_to_us(ns), do: div(ns, 1000) - # Parse a flat `key value` stat file (memory.stat, cpu.stat) into a map of # binary key -> integer value. Lines that don't parse are dropped. defp read_stat_file(path) do diff --git a/packages/electric-telemetry/test/electric/telemetry/system_metrics/cgroup_test.exs b/packages/electric-telemetry/test/electric/telemetry/system_metrics/cgroup_test.exs index b61bb52fcd..344697822a 100644 --- a/packages/electric-telemetry/test/electric/telemetry/system_metrics/cgroup_test.exs +++ b/packages/electric-telemetry/test/electric/telemetry/system_metrics/cgroup_test.exs @@ -197,101 +197,6 @@ defmodule ElectricTelemetry.SystemMetrics.CgroupTest do end end - describe "cgroup v1" do - setup %{tmp_dir: tmp_dir} do - root = Path.join(tmp_dir, "v1") - - write_file(root, "memory/memory.usage_in_bytes", "2097152\n") - - write_file(root, "memory/memory.stat", """ - rss 1048576 - cache 524288 - inactive_file 262144 - mapped_file 4096 - """) - - # unlimited sentinel - write_file(root, "memory/memory.limit_in_bytes", "9223372036854771712\n") - - # cpuacct.usage is in nanoseconds - write_file(root, "cpuacct/cpuacct.usage", "9000000000\n") - - write_file(root, "cpu/cpu.stat", """ - nr_periods 100 - nr_throttled 7 - throttled_time 250000000 - """) - - %{root: root} - end - - test "emits parsed memory metrics with v1 key mapping", %{root: root} do - m = - collect_cgroup_metrics(fn -> - Cgroup.measurement(%{}, cgroup_version: :v1, cgroup_root: root) - end) - - assert m["cgroup.memory.current"] == 2_097_152 - # v1 anon <- rss, file <- cache - assert m["cgroup.memory.anon"] == 1_048_576 - assert m["cgroup.memory.file"] == 524_288 - assert m["cgroup.memory.working_set"] == 2_097_152 - 262_144 - end - - test "skips memory.max when v1 unlimited sentinel", %{root: root} do - m = - collect_cgroup_metrics(fn -> - Cgroup.measurement(%{}, cgroup_version: :v1, cgroup_root: root) - end) - - refute Map.has_key?(m, "cgroup.memory.max") - end - - test "emits a real numeric v1 memory limit", %{root: root} do - write_file(root, "memory/memory.limit_in_bytes", "1073741824\n") - - m = - collect_cgroup_metrics(fn -> - Cgroup.measurement(%{}, cgroup_version: :v1, cgroup_root: root) - end) - - assert m["cgroup.memory.max"] == 1_073_741_824 - end - - test "converts cpu usage / throttled time from ns to us", %{root: root} do - m = - collect_cgroup_metrics(fn -> - Cgroup.measurement(%{}, cgroup_version: :v1, cgroup_root: root) - end) - - # 9_000_000_000 ns -> 9_000_000 us - assert m["cgroup.cpu.usage_usec"] == 9_000_000 - assert m["cgroup.cpu.nr_throttled"] == 7 - # 250_000_000 ns -> 250_000 us - assert m["cgroup.cpu.throttled_usec"] == 250_000 - end - - test "does not emit PSI on v1", %{root: root} do - m = - collect_cgroup_metrics(fn -> - Cgroup.measurement(%{}, cgroup_version: :v1, cgroup_root: root) - end) - - refute Map.has_key?(m, "cgroup.memory.pressure.full.avg10") - refute Map.has_key?(m, "cgroup.cpu.pressure.full.avg10") - end - - test "does not emit io metrics on v1", %{root: root} do - m = - collect_cgroup_metrics(fn -> - Cgroup.measurement(%{}, cgroup_version: :v1, cgroup_root: root) - end) - - refute Map.has_key?(m, "cgroup.io.rbytes") - refute Map.has_key?(m, "cgroup.io.wbytes") - end - end - describe ":none / non-Linux" do test "no-ops and emits nothing", %{tmp_dir: tmp_dir} do m = From bee669188b88bf6d90f6c3f72edec66963be35a1 Mon Sep 17 00:00:00 2001 From: Oleksii Sholik Date: Tue, 7 Jul 2026 12:24:38 +0200 Subject: [PATCH 09/13] Simplify system metrics readers around shared helpers Extract the patterns the cgroup and /proc readers had each grown a private copy of: - SystemMetrics.emit/2 drops nil measurements and fires a single :telemetry.execute per event. The readers previously emitted one single-key event per scalar (~19 executes per poll tick); each plane now emits one multi-key event (6 executes), matching the package convention (cf. get_system_memory_usage) with no metric-definition changes. - ProcfsParse.read_kv_file/1 replaces the three byte-identical read_meminfo/read_status/read_proc_io wrappers and Cgroup's own read_stat_file: one whitespace-tolerant "key[:] value [unit]" parser covers meminfo, status, io, memory.stat and cpu.stat. parse_float moves here next to parse_int, and read_int_file is shared too. Also: - Register Cgroup.measurement / Proc.measurement MFAs directly with the poller instead of going through thin delegators on SystemMetrics (the poller accepts any MFA; the extra layer bought nothing). - recon_alloc_measurement reads :allocated and :used once and derives unused/carrier_usage locally instead of triggering four full allocator sweeps per tick; the fragmentation reduce no longer computes unused_bytes twice per instance. - Fold the duplicated persistent_term get-or-compute code into a memoized/2 helper, drop the unused default_root/0 public functions, sum io.stat rbytes/wbytes in a single pass, and trim comment essays down to the load-bearing points. Co-Authored-By: Claude Fable 5 --- .../telemetry/application_telemetry.ex | 18 +- .../lib/electric/telemetry/system_metrics.ex | 169 +++++----------- .../telemetry/system_metrics/cgroup.ex | 188 +++++------------- .../electric/telemetry/system_metrics/proc.ex | 124 ++++-------- .../telemetry/system_metrics/procfs_parse.ex | 105 ++++------ 5 files changed, 181 insertions(+), 423 deletions(-) diff --git a/packages/electric-telemetry/lib/electric/telemetry/application_telemetry.ex b/packages/electric-telemetry/lib/electric/telemetry/application_telemetry.ex index 979421168d..892747e111 100644 --- a/packages/electric-telemetry/lib/electric/telemetry/application_telemetry.ex +++ b/packages/electric-telemetry/lib/electric/telemetry/application_telemetry.ex @@ -88,17 +88,13 @@ defmodule ElectricTelemetry.ApplicationTelemetry do &{__MODULE__, &1, [telemetry_opts]} ) ++ [ - # BEAM allocator metrics (vm.alloc.*) live in ElectricTelemetry.SystemMetrics. - # The cheap aggregate view runs every tick; the expensive per-allocator - # fragmentation breakdown is internally gated to ~once a minute. + # System metrics (vm.alloc.*, cgroup.*, host.*); each measurement no-ops + # where unsupported (no cgroup v2, non-Linux). The per-allocator + # fragmentation breakdown internally gates itself to ~once a minute. {ElectricTelemetry.SystemMetrics, :recon_alloc_measurement, [telemetry_opts]}, {ElectricTelemetry.SystemMetrics, :allocator_fragmentation_measurement, [telemetry_opts]}, - # cgroup (v1/v2) accounting metrics (cgroup.*) live in - # ElectricTelemetry.SystemMetrics.Cgroup; no-op when no cgroup is detected. - {ElectricTelemetry.SystemMetrics, :cgroup_measurement, [telemetry_opts]}, - # host/process /proc metrics (host.mem.*, host.proc.beam.*) live in - # ElectricTelemetry.SystemMetrics.Proc; no-op on non-Linux. - {ElectricTelemetry.SystemMetrics, :proc_measurement, [telemetry_opts]} + {ElectricTelemetry.SystemMetrics.Cgroup, :measurement, [telemetry_opts]}, + {ElectricTelemetry.SystemMetrics.Proc, :measurement, [telemetry_opts]} ] end @@ -136,7 +132,7 @@ defmodule ElectricTelemetry.ApplicationTelemetry do last_value("vm.alloc.unused", unit: :byte), last_value("vm.alloc.carrier_usage"), last_value("vm.alloc.fragmentation.unused", tags: [:allocator], unit: :byte), - # cgroup (v1/v2) accounting metrics (emitted by + # cgroup v2 accounting metrics (emitted by # ElectricTelemetry.SystemMetrics.Cgroup). last_value("cgroup.memory.current", unit: :byte), last_value("cgroup.memory.anon", unit: :byte), @@ -145,8 +141,6 @@ defmodule ElectricTelemetry.ApplicationTelemetry do last_value("cgroup.memory.max", unit: :byte), # PSI "full avg10" stall percentage (v2 only). last_value("cgroup.memory.pressure.full.avg10"), - # cpu usage / throttled time already in microseconds (v1 ns values are - # converted in the reader); :unit is a label only, no conversion. last_value("cgroup.cpu.usage_usec", unit: :microsecond), last_value("cgroup.cpu.nr_throttled"), last_value("cgroup.cpu.throttled_usec", unit: :microsecond), diff --git a/packages/electric-telemetry/lib/electric/telemetry/system_metrics.ex b/packages/electric-telemetry/lib/electric/telemetry/system_metrics.ex index deaffe0086..3982817609 100644 --- a/packages/electric-telemetry/lib/electric/telemetry/system_metrics.ex +++ b/packages/electric-telemetry/lib/electric/telemetry/system_metrics.ex @@ -3,43 +3,25 @@ defmodule ElectricTelemetry.SystemMetrics do Periodic measurement functions for low-level system/runtime metrics that complement the application-level VM stats in `ElectricTelemetry.ApplicationTelemetry`. - These functions are not a process; they are invoked by `ElectricTelemetry.Poller` - (via `telemetry_poller`) as `{__MODULE__, fun, [telemetry_opts]}` MFA tuples. The - poller wraps every invocation in `ElectricTelemetry.Poller.safe_invoke/3`, so a - crash here is logged and swallowed rather than removing the measurement. - - This module currently exposes: - - * BEAM allocator metrics (`vm.alloc.*`), derived from `:recon_alloc`. These are - cross-platform (recon works everywhere) and sit alongside the existing - `vm.memory.*` metrics. The cheap aggregate view is sampled every poll tick; - the expensive per-allocator fragmentation breakdown is gated to run roughly - once a minute. - - * A cached platform/cgroup detection helper (`system_info/0`). Only the OS and - cgroup version are computed here; later tasks build cgroup/host readers on top - of this scaffolding. + These functions are not a process; `ElectricTelemetry.Poller` invokes them + (and the `SystemMetrics.Cgroup` / `SystemMetrics.Proc` readers) as MFA tuples, + wrapping every invocation in `safe_invoke/3` so a crash here is logged and + swallowed rather than removing the measurement. + + This module holds the BEAM allocator metrics (`vm.alloc.*`, derived from + `:recon_alloc`), the cached platform/cgroup detection (`system_info/0`), and + the shared `emit/2` helper used by the cgroup//proc readers. """ @system_info_key {__MODULE__, :system_info} @fragmentation_gate_key {__MODULE__, :fragmentation_gate} - # The poller runs all measurements at a single ~5s period. The per-allocator - # fragmentation breakdown (`:recon_alloc.fragmentation/1`) is O(carriers) and too - # expensive to run every tick, so we gate it to run roughly once a minute. With a - # 5s poll interval, every 12th tick is ~60s. - # - # We gate with an atomic `:counters` ref rather than a second `:telemetry_poller` - # child: the poller's period is shared across all measurements (see - # `ElectricTelemetry.Poller.child_spec/2`), so there is no per-measurement period - # to configure, and a counter-gate keeps this self-contained. - # - # The `:counters` ref is created once and cached in `:persistent_term` (a single - # put per gate key, ever). We deliberately avoid bumping the tick count via - # `:persistent_term.put` on every poll: each `:persistent_term.put` triggers a - # global scan of all process heaps for GC, which is exactly the per-tick overhead a - # cheap telemetry module must avoid. `:counters.add/3` is a lock-free atomic with no - # such cost and no read-modify-write race. + # The poller runs all measurements at a single ~5s period, and + # `:recon_alloc.fragmentation/1` is O(carriers) — too expensive for every + # tick — so it self-gates to every 12th tick (~once a minute). The gate is an + # atomic `:counters` ref cached in `:persistent_term`: `:counters.add/3` is a + # lock-free bump, whereas a `:persistent_term.put` per tick would trigger a + # global GC scan of all process heaps. @fragmentation_interval_ticks 12 @doc """ @@ -48,6 +30,18 @@ defmodule ElectricTelemetry.SystemMetrics do @spec allocator_fragmentation_interval_ticks() :: pos_integer() def allocator_fragmentation_interval_ticks, do: @fragmentation_interval_ticks + @doc """ + Emit a telemetry event carrying the non-nil `measurements` in a single + `:telemetry.execute/2` call, or nothing when every value is nil (file + missing / unreadable / metric intentionally skipped). + """ + @spec emit(:telemetry.event_name(), %{optional(atom()) => number() | nil}) :: :ok + def emit(event, measurements) do + measurements = Map.reject(measurements, fn {_key, value} -> is_nil(value) end) + if map_size(measurements) > 0, do: :telemetry.execute(event, measurements) + :ok + end + @doc """ Boot-time platform/cgroup detection, computed once and cached in `:persistent_term`. @@ -56,20 +50,10 @@ defmodule ElectricTelemetry.SystemMetrics do """ @spec system_info() :: %{os: {atom(), atom()}, cgroup_version: :v2 | :none} def system_info do - case :persistent_term.get(@system_info_key, :undefined) do - :undefined -> - info = compute_system_info() - :persistent_term.put(@system_info_key, info) - info - - info -> - info - end - end - - defp compute_system_info do - os = :os.type() - %{os: os, cgroup_version: detect_cgroup_version(os)} + memoized(@system_info_key, fn -> + os = :os.type() + %{os: os, cgroup_version: detect_cgroup_version(os)} + end) end # The controllers list at the cgroup fs root is the canonical v2 marker. @@ -88,42 +72,32 @@ defmodule ElectricTelemetry.SystemMetrics do * `:used` — bytes actually in use by blocks * `:unused` — `allocated - used`, i.e. fragmentation/headroom held in carriers * `:carrier_usage` — `used / allocated`, the carrier usage ratio in `0..1` - - These map to the `vm.alloc.allocated`, `vm.alloc.unused`, and - `vm.alloc.carrier_usage` metrics (plus `vm.alloc.used`). """ @spec recon_alloc_measurement(map()) :: :ok def recon_alloc_measurement(_telemetry_opts) do + # Each :recon_alloc.memory/1 call sweeps every allocator instance, so read + # the two base numbers once and derive the rest locally. allocated = :recon_alloc.memory(:allocated) used = :recon_alloc.memory(:used) - unused = max(allocated - used, 0) - carrier_usage = :recon_alloc.memory(:usage) - :telemetry.execute([:vm, :alloc], %{ + emit([:vm, :alloc], %{ allocated: allocated, used: used, - unused: unused, - carrier_usage: carrier_usage + unused: max(allocated - used, 0), + carrier_usage: if(allocated > 0, do: used / allocated) }) - - :ok end @doc """ - Per-allocator fragmentation breakdown (slow tier, ~once a minute). - - Uses `:recon_alloc.fragmentation(:current)` (an O(carriers) call). It keys results - by `{type, instance}` (e.g. `{:eheap_alloc, 3}`); we compute unused bytes held in - carriers (`sbcs_carriers_size + mbcs_carriers_size - sbcs_block_size - mbcs_block_size`) - and sum them per allocator *type*. One `[:vm, :alloc, :fragmentation]` event is - emitted per type, tagged by `allocator` (the type name as a string). + Per-allocator fragmentation breakdown (slow tier, ~once a minute — see + `@fragmentation_interval_ticks`). - Because the poller runs every measurement at one ~5s period, this is gated by a - counter in `:persistent_term` and early-returns on non-due ticks (see - `@fragmentation_interval_ticks`). Pass `force: true` to bypass the gate (tests). + Sums unused carrier bytes from `:recon_alloc.fragmentation(:current)` per + allocator *type* and emits one `[:vm, :alloc, :fragmentation]` event per + type, tagged by `allocator`. Options: - * `:force` — when `true`, always run regardless of the gate + * `:force` — when `true`, always run regardless of the gate (tests) * `:gate_key` — override the `:persistent_term` key holding the tick counter (used by tests to avoid contending with the live poller's counter) """ @@ -132,7 +106,8 @@ defmodule ElectricTelemetry.SystemMetrics do if due?(opts) do :recon_alloc.fragmentation(:current) |> Enum.reduce(%{}, fn {{type, _instance}, info}, acc -> - Map.update(acc, type, unused_bytes(info), &(&1 + unused_bytes(info))) + unused = unused_bytes(info) + Map.update(acc, type, unused, &(&1 + unused)) end) |> Enum.each(fn {type, unused} -> :telemetry.execute( @@ -146,68 +121,30 @@ defmodule ElectricTelemetry.SystemMetrics do :ok end - @doc """ - cgroup (v1/v2) accounting metrics, sampled every poll tick. - - Reads the container's cgroup files and emits the `cgroup.*` family (memory, - cpu, io, and — on v2 — PSI pressure). Parsing lives in - `ElectricTelemetry.SystemMetrics.Cgroup`; this is a thin delegator so it - registers like the other measurements. No-ops when no cgroup is detected. - - Options are forwarded to `Cgroup.measurement/2` (`:cgroup_root` and - `:cgroup_version` overrides for tests). - """ - @spec cgroup_measurement(map(), keyword()) :: :ok - def cgroup_measurement(telemetry_opts, opts \\ []) do - ElectricTelemetry.SystemMetrics.Cgroup.measurement(telemetry_opts, opts) - end - - @doc """ - Host/process `/proc` metrics, sampled every poll tick. - - Reads `/proc/meminfo` and the BEAM's own `/proc//status` and - `/proc//io` and emits the `host.mem.*` and `host.proc.beam.*` families. - Parsing lives in `ElectricTelemetry.SystemMetrics.Proc`; this is a thin - delegator so it registers like the other measurements. No-ops on non-Linux. - - Options are forwarded to `Proc.measurement/2` (`:proc_root`, `:pid` and `:os` - overrides for tests). - """ - @spec proc_measurement(map(), keyword()) :: :ok - def proc_measurement(telemetry_opts, opts \\ []) do - ElectricTelemetry.SystemMetrics.Proc.measurement(telemetry_opts, opts) - end - defp due?(opts) do if Keyword.get(opts, :force, false) do true else gate_key = Keyword.get(opts, :gate_key, @fragmentation_gate_key) - ref = gate_counter(gate_key) + ref = memoized(gate_key, fn -> :counters.new(1, [:write_concurrency]) end) :counters.add(ref, 1, 1) # The first tick reads 1, so the gate fires on the Nth tick (not on boot). rem(:counters.get(ref, 1), @fragmentation_interval_ticks) == 0 end end - # Returns a cached single-slot `:counters` ref for the gate key, creating and - # caching it on first use. The `:persistent_term.put` here happens at most once per - # gate key (on first call), never on the per-tick hot path. - defp gate_counter(gate_key) do - case :persistent_term.get(gate_key, :undefined) do - :undefined -> - ref = :counters.new(1, [:write_concurrency]) - :persistent_term.put(gate_key, ref) - ref - - ref -> - ref + # Get-or-compute a `:persistent_term` entry. The put happens at most once per + # key (on first call), never on the per-tick hot path. + defp memoized(key, fun) do + with :undefined <- :persistent_term.get(key, :undefined) do + tap(fun.(), &:persistent_term.put(key, &1)) end end - # `:recon_alloc.fragmentation/1` returns a proplist per allocator with carrier and - # block sizes for both single-block (sbcs) and multi-block (mbcs) carriers. Unused - # bytes are the carrier bytes the OS gave us minus the bytes actually filled by blocks. + # `:recon_alloc.fragmentation/1` returns a proplist per allocator with carrier + # and block sizes for both single-block (sbcs) and multi-block (mbcs) + # carriers. Unused bytes are the carrier bytes the OS gave us minus the bytes + # actually filled by blocks. defp unused_bytes(info) do carriers = Keyword.get(info, :sbcs_carriers_size, 0) + Keyword.get(info, :mbcs_carriers_size, 0) diff --git a/packages/electric-telemetry/lib/electric/telemetry/system_metrics/cgroup.ex b/packages/electric-telemetry/lib/electric/telemetry/system_metrics/cgroup.ex index abb8f24361..cb165be567 100644 --- a/packages/electric-telemetry/lib/electric/telemetry/system_metrics/cgroup.ex +++ b/packages/electric-telemetry/lib/electric/telemetry/system_metrics/cgroup.ex @@ -14,16 +14,13 @@ defmodule ElectricTelemetry.SystemMetrics.Cgroup do """ alias ElectricTelemetry.SystemMetrics - alias ElectricTelemetry.SystemMetrics.ProcfsParse - @default_root "/sys/fs/cgroup" + import ElectricTelemetry.SystemMetrics, only: [emit: 2] - @doc """ - The default cgroup filesystem root. Tests override this via the `:cgroup_root` - option on `measurement/2`. - """ - @spec default_root() :: String.t() - def default_root, do: @default_root + import ElectricTelemetry.SystemMetrics.ProcfsParse, + only: [read_kv_file: 1, read_int_file: 1, read_raw_file: 1, parse_int: 1, parse_float: 1] + + @default_root "/sys/fs/cgroup" @doc """ Read the cgroup accounting files and emit `cgroup.*` telemetry events. @@ -44,7 +41,7 @@ defmodule ElectricTelemetry.SystemMetrics.Cgroup do root = Keyword.get(opts, :cgroup_root, @default_root) emit_memory(root) emit_cpu(root) - emit_io(root) + emit([:cgroup, :io], io_totals(read_raw_file(Path.join(root, "io.stat")))) end :ok @@ -52,127 +49,62 @@ defmodule ElectricTelemetry.SystemMetrics.Cgroup do defp emit_memory(root) do current = read_int_file(Path.join(root, "memory.current")) - stat = read_stat_file(Path.join(root, "memory.stat")) - - emit(:memory, :current, current) - emit(:memory, :anon, Map.get(stat, "anon")) - emit(:memory, :file, Map.get(stat, "file")) - emit(:memory, :working_set, working_set(current, Map.get(stat, "inactive_file"))) - # An unlimited memory.max is the literal "max", which parses to nil and is - # skipped (the host-RAM ceiling comes from the /proc reader). - emit(:memory, :max, read_int_file(Path.join(root, "memory.max"))) - - emit_pressure(Path.join(root, "memory.pressure"), :memory) + stat = read_kv_file(Path.join(root, "memory.stat")) + inactive_file = stat["inactive_file"] + + emit([:cgroup, :memory], %{ + current: current, + anon: stat["anon"], + file: stat["file"], + working_set: current && inactive_file && max(current - inactive_file, 0), + # An unlimited memory.max is the literal "max", which parses to nil and is + # skipped (the host-RAM ceiling comes from the /proc reader). + max: read_int_file(Path.join(root, "memory.max")) + }) + + emit_pressure(root, "memory.pressure", :memory) end defp emit_cpu(root) do - stat = read_stat_file(Path.join(root, "cpu.stat")) - - emit(:cpu, :usage_usec, Map.get(stat, "usage_usec")) - emit(:cpu, :nr_throttled, Map.get(stat, "nr_throttled")) - emit(:cpu, :throttled_usec, Map.get(stat, "throttled_usec")) - - emit_pressure(Path.join(root, "cpu.pressure"), :cpu) - end - - defp emit_io(root) do - io = parse_io_stat(read_raw_file(Path.join(root, "io.stat"))) - - emit(:io, :rbytes, io.rbytes) - emit(:io, :wbytes, io.wbytes) - end - - ## emitting -------------------------------------------------------------- - - # Emit a `[:cgroup, plane]` event carrying a single keyed measurement, unless - # the value is nil (file missing / unparseable / intentionally skipped). - defp emit(_plane, _key, nil), do: :ok - - defp emit(plane, key, value) do - :telemetry.execute([:cgroup, plane], %{key => value}) - :ok - end + stat = read_kv_file(Path.join(root, "cpu.stat")) - # PSI files may be absent even on v2 (kernel built without PSI). Skip on any - # read/parse failure. - defp emit_pressure(path, plane) do - case psi_full_avg10(read_raw_file(path)) do - nil -> :ok - avg10 -> :telemetry.execute([:cgroup, plane, :pressure, :full], %{avg10: avg10}) - end + emit([:cgroup, :cpu], %{ + usage_usec: stat["usage_usec"], + nr_throttled: stat["nr_throttled"], + throttled_usec: stat["throttled_usec"] + }) - :ok + emit_pressure(root, "cpu.pressure", :cpu) end - ## parsing helpers ------------------------------------------------------- - - # working_set = current - inactive_file. If either input is missing, skip the - # metric rather than emit a wrong number. - defp working_set(nil, _inactive_file), do: nil - defp working_set(_current, nil), do: nil - defp working_set(current, inactive_file), do: max(current - inactive_file, 0) - - # Parse a flat `key value` stat file (memory.stat, cpu.stat) into a map of - # binary key -> integer value. Lines that don't parse are dropped. - defp read_stat_file(path) do - case read_raw_file(path) do - nil -> - %{} - - content -> - content - |> String.split("\n", trim: true) - |> Enum.reduce(%{}, fn line, acc -> - case String.split(line, " ", trim: true) do - [key, value] -> - case parse_int(value) do - nil -> acc - int -> Map.put(acc, key, int) - end - - _ -> - acc - end - end) - end + # PSI files may be absent even on v2 (kernel built without PSI); a nil avg10 + # is dropped by emit/2, skipping the event. + defp emit_pressure(root, file, plane) do + avg10 = psi_full_avg10(read_raw_file(Path.join(root, file))) + emit([:cgroup, plane, :pressure, :full], %{avg10: avg10}) end # io.stat is one line per device: "MAJ:MIN rbytes=… wbytes=… rios=… …". - # Sum rbytes/wbytes across all devices. - defp parse_io_stat(nil), do: %{rbytes: nil, wbytes: nil} - - defp parse_io_stat(content) do - {rbytes, wbytes, any?} = - content - |> String.split("\n", trim: true) - |> Enum.reduce({0, 0, false}, fn line, {r, w, any?} -> - fields = String.split(line, " ", trim: true) - - line_r = io_field(fields, "rbytes=") - line_w = io_field(fields, "wbytes=") - - {r + (line_r || 0), w + (line_w || 0), any? or line_r != nil or line_w != nil} - end) - - if any? do - %{rbytes: rbytes, wbytes: wbytes} - else - %{rbytes: nil, wbytes: nil} + # Sum rbytes/wbytes across all devices; a key never seen stays absent. + defp io_totals(nil), do: %{} + + defp io_totals(content) do + for field <- String.split(content), reduce: %{} do + acc -> + case field do + "rbytes=" <> value -> add_field(acc, :rbytes, parse_int(value)) + "wbytes=" <> value -> add_field(acc, :wbytes, parse_int(value)) + _ -> acc + end end end - defp io_field(fields, prefix) do - Enum.find_value(fields, fn field -> - case field do - ^prefix <> rest -> parse_int(rest) - _ -> nil - end - end) - end + defp add_field(acc, _key, nil), do: acc + defp add_field(acc, key, value), do: Map.update(acc, key, value, &(&1 + value)) # PSI file format (one "some"/"full" line each): # some avg10=0.00 avg60=0.00 avg300=0.00 total=12345 - # full avg10=0.00 avg60=0.00 avg300=0.00 total=6789 + # full avg10=0.00 avg60=0.00 avg300=0.05 total=6789 # Extract avg10 from the "full" line as a float. defp psi_full_avg10(nil), do: nil @@ -182,11 +114,9 @@ defmodule ElectricTelemetry.SystemMetrics.Cgroup do |> Enum.find_value(fn line -> case String.split(line, " ", trim: true) do ["full" | fields] -> - Enum.find_value(fields, fn field -> - case field do - "avg10=" <> rest -> parse_float(rest) - _ -> nil - end + Enum.find_value(fields, fn + "avg10=" <> rest -> parse_float(rest) + _ -> nil end) _ -> @@ -194,24 +124,4 @@ defmodule ElectricTelemetry.SystemMetrics.Cgroup do end end) end - - ## low-level file/number helpers ----------------------------------------- - - # The format-agnostic file/integer primitives are shared with the /proc - # reader (ProcfsParse); the colon- vs space-delimited stat parsers above stay - # separate because they parse genuinely different file layouts. - - # Read a single-integer file (memory.current, usage_in_bytes, cpuacct.usage). - defp read_int_file(path), do: parse_int(read_raw_file(path)) - - defp read_raw_file(path), do: ProcfsParse.read_raw_file(path) - - defp parse_int(value), do: ProcfsParse.parse_int(value) - - defp parse_float(str) do - case Float.parse(String.trim(str)) do - {float, ""} -> float - _ -> nil - end - end end diff --git a/packages/electric-telemetry/lib/electric/telemetry/system_metrics/proc.ex b/packages/electric-telemetry/lib/electric/telemetry/system_metrics/proc.ex index c83b53e7c5..395c9b48eb 100644 --- a/packages/electric-telemetry/lib/electric/telemetry/system_metrics/proc.ex +++ b/packages/electric-telemetry/lib/electric/telemetry/system_metrics/proc.ex @@ -1,46 +1,31 @@ defmodule ElectricTelemetry.SystemMetrics.Proc do @moduledoc """ - Reads Linux `/proc` accounting files and emits `host.mem.*` and - `host.proc.beam.*` telemetry. + Reads Linux `/proc` accounting files and emits `host.mem.*` (from + `/proc/meminfo`) and `host.proc.beam.*` (from the BEAM's own + `/proc//status` and `/proc//io`) telemetry. - This bridges the cgroup plane and the BEAM allocator plane: - - * `host.mem.*` (from `/proc/meminfo`) gives the host-RAM ceiling and cache - picture. On Electric Cloud there is one task per host (EC2), so host ≈ - container. - * `host.proc.beam.*` (from `/proc//status` and - `/proc//io`) gives the per-process RSS breakdown (anon vs file - vs shmem) and IO byte counters for *this* BEAM, so cgroup `anon` can be - shown to track the BEAM's anonymous footprint. + This bridges the cgroup plane and the BEAM allocator plane: the per-process + RSS breakdown (anon vs file vs shmem) shows cgroup `anon` tracking the BEAM + footprint, and meminfo gives the host-RAM ceiling/cache picture (one task per + host on EC2, so host ≈ container). All reads are defensive: a missing file, a permission error (`/proc//io` can be EACCES-restricted even for self in some sandboxes), or unexpected content results in the corresponding metric being skipped, never a crash. + `/proc` only exists on Linux, so the measurement is a clean no-op everywhere + else. - `/proc` only exists on Linux, so the measurement is a clean no-op on every - other platform. The OS is taken from - `ElectricTelemetry.SystemMetrics.system_info/0` (cached at boot) so we never - re-detect on the hot path. - - Units: - - * `/proc/meminfo` values and `/proc//status` `Rss*`/`VmRSS` are in kB - and converted to bytes (× 1024). - * `/proc//io` `read_bytes`/`write_bytes` are already bytes and emitted - as-is. + meminfo values and status `Rss*`/`VmRSS` are in kB and converted to bytes; + `/proc//io` `read_bytes`/`write_bytes` are already bytes and emitted + as-is. """ alias ElectricTelemetry.SystemMetrics - alias ElectricTelemetry.SystemMetrics.ProcfsParse - @default_root "/proc" + import ElectricTelemetry.SystemMetrics, only: [emit: 2] + import ElectricTelemetry.SystemMetrics.ProcfsParse, only: [read_kv_file: 1] - @doc """ - The default `/proc` filesystem root. Tests override this via the `:proc_root` - option on `measurement/2`. - """ - @spec default_root() :: String.t() - def default_root, do: @default_root + @default_root "/proc" @doc """ Read the `/proc` accounting files and emit `host.mem.*` / `host.proc.beam.*` @@ -51,7 +36,7 @@ defmodule ElectricTelemetry.SystemMetrics.Proc do * `:proc_root` — override the `/proc` filesystem root (default `#{@default_root}`); used by tests to point at fixture trees. * `:pid` — override the OS pid of the BEAM whose per-process files are read - (default `:os.getpid()`); used by tests to point at `//`. + (default `:os.getpid()`); used by tests. * `:os` — override the detected OS (`{family, name}` tuple); used by tests. Defaults to the cached `SystemMetrics.system_info/0` value. """ @@ -59,71 +44,38 @@ defmodule ElectricTelemetry.SystemMetrics.Proc do def measurement(_telemetry_opts, opts \\ []) do os = Keyword.get_lazy(opts, :os, fn -> SystemMetrics.system_info().os end) - case os do - {:unix, :linux} -> - root = Keyword.get(opts, :proc_root, @default_root) - pid = Keyword.get_lazy(opts, :pid, fn -> :os.getpid() end) |> to_string() - - emit_host_mem(root) - emit_beam_status(root, pid) - emit_beam_io(root, pid) + if os == {:unix, :linux} do + root = Keyword.get(opts, :proc_root, @default_root) + pid = opts |> Keyword.get_lazy(:pid, fn -> :os.getpid() end) |> to_string() - _non_linux -> - :ok - end + meminfo = read_kv_file(Path.join(root, "meminfo")) - :ok - end + emit([:host, :mem], %{ + total: kb_to_bytes(meminfo["MemTotal"]), + free: kb_to_bytes(meminfo["MemFree"]), + cached: kb_to_bytes(meminfo["Cached"]) + }) - ## host memory ----------------------------------------------------------- + status = read_kv_file(Path.join([root, pid, "status"])) - # /proc/meminfo is a flat ": kB" file. Read the keys we want and - # convert kB -> bytes. - defp emit_host_mem(root) do - info = ProcfsParse.read_meminfo(Path.join(root, "meminfo")) + emit([:host, :proc, :beam], %{ + rss_anon: kb_to_bytes(status["RssAnon"]), + rss_file: kb_to_bytes(status["RssFile"]), + rss_shmem: kb_to_bytes(status["RssShmem"]), + vm_rss: kb_to_bytes(status["VmRSS"]) + }) - emit([:host, :mem], :total, kb_to_bytes(Map.get(info, "MemTotal"))) - emit([:host, :mem], :free, kb_to_bytes(Map.get(info, "MemFree"))) - emit([:host, :mem], :cached, kb_to_bytes(Map.get(info, "Cached"))) - end + io = read_kv_file(Path.join([root, pid, "io"])) - ## BEAM process status --------------------------------------------------- - - # /proc//status is a flat ":\t[ kB]" file. The Rss*/VmRSS - # values are in kB; convert to bytes. - defp emit_beam_status(root, pid) do - status = ProcfsParse.read_status(Path.join([root, pid, "status"])) - - emit([:host, :proc, :beam], :rss_anon, kb_to_bytes(Map.get(status, "RssAnon"))) - emit([:host, :proc, :beam], :rss_file, kb_to_bytes(Map.get(status, "RssFile"))) - emit([:host, :proc, :beam], :rss_shmem, kb_to_bytes(Map.get(status, "RssShmem"))) - emit([:host, :proc, :beam], :vm_rss, kb_to_bytes(Map.get(status, "VmRSS"))) - end - - ## BEAM process IO ------------------------------------------------------- - - # /proc//io is a flat ": " file with byte counters. May be - # EACCES-restricted; on any read failure we simply emit nothing. - defp emit_beam_io(root, pid) do - io = ProcfsParse.read_proc_io(Path.join([root, pid, "io"])) - - emit([:host, :proc, :beam, :io], :read_bytes, Map.get(io, "read_bytes")) - emit([:host, :proc, :beam, :io], :write_bytes, Map.get(io, "write_bytes")) - end - - ## emitting -------------------------------------------------------------- - - # Emit `event` carrying a single keyed measurement, unless the value is nil - # (file missing / unreadable / unparseable). - defp emit(_event, _key, nil), do: :ok + emit([:host, :proc, :beam, :io], %{ + read_bytes: io["read_bytes"], + write_bytes: io["write_bytes"] + }) + end - defp emit(event, key, value) do - :telemetry.execute(event, %{key => value}) :ok end - ## unit helpers ---------------------------------------------------------- - defp kb_to_bytes(nil), do: nil defp kb_to_bytes(kb), do: kb * 1024 end diff --git a/packages/electric-telemetry/lib/electric/telemetry/system_metrics/procfs_parse.ex b/packages/electric-telemetry/lib/electric/telemetry/system_metrics/procfs_parse.ex index bdd7bedb81..a3e3e54397 100644 --- a/packages/electric-telemetry/lib/electric/telemetry/system_metrics/procfs_parse.ex +++ b/packages/electric-telemetry/lib/electric/telemetry/system_metrics/procfs_parse.ex @@ -1,7 +1,7 @@ defmodule ElectricTelemetry.SystemMetrics.ProcfsParse do @moduledoc """ - Small, exhaustively defensive parsers for the flat `/proc` text files read by - `ElectricTelemetry.SystemMetrics.Proc`. + Small, exhaustively defensive parsers for the flat text files exposed by + procfs and cgroupfs. Every function degrades to an empty map (or `nil` for scalars) on a missing file, a permission error, or malformed content — it never raises. Parsing @@ -13,80 +13,31 @@ defmodule ElectricTelemetry.SystemMetrics.ProcfsParse do """ @doc """ - Parse `/proc/meminfo` into a map of `binary key => integer kB value`. + Parse a flat key/value file — `/proc/meminfo`, `/proc//status`, + `/proc//io`, cgroup `memory.stat`/`cpu.stat` — into a map of + `binary key => integer value`. - Lines look like `MemTotal: 16384000 kB`. The trailing `kB` unit (and any - line that doesn't match `: [kB]`) is handled gracefully; values are - returned in their native kB (the caller converts to bytes). + Handles both the `key value` and `Key:\twhitespace-padded value [unit]` line + layouts: the key is the first whitespace-delimited token with any trailing + `:` stripped, the value is the second token, and any trailing unit (e.g. + `kB`) is ignored. Lines whose value doesn't parse as an integer are dropped. + Values keep their native unit (the caller converts, e.g. kB -> bytes). """ - @spec read_meminfo(Path.t()) :: %{optional(binary()) => integer()} - def read_meminfo(path) do - case read_raw_file(path) do - nil -> %{} - content -> parse_keyed_kv(content) - end + @spec read_kv_file(Path.t()) :: %{optional(binary()) => integer()} + def read_kv_file(path) do + for line <- String.split(read_raw_file(path) || "", "\n", trim: true), + [key, value | _] <- [String.split(line)], + int = parse_int(value), + into: %{}, + do: {String.trim_trailing(key, ":"), int} end @doc """ - Parse `/proc//status` into a map of `binary key => integer kB value` for - the memory keys only. - - Lines look like `RssAnon:\t 12345 kB` or `VmRSS:\t 67890 kB`. Only lines - whose value parses as an integer are kept; non-numeric keys (e.g. `Name`, - `State`) are simply dropped. Values are returned in their native kB (the - caller converts to bytes). + Read a single-integer file (e.g. cgroup `memory.current`), returning `nil` + on any read or parse error. """ - @spec read_status(Path.t()) :: %{optional(binary()) => integer()} - def read_status(path) do - case read_raw_file(path) do - nil -> %{} - content -> parse_keyed_kv(content) - end - end - - @doc """ - Parse `/proc//io` into a map of `binary key => integer byte value`. - - Lines look like `read_bytes: 12345`. This file can be permission-restricted - (EACCES) even for the reading process itself in some sandboxes; that read - failure degrades to an empty map. Values are already in bytes. - """ - @spec read_proc_io(Path.t()) :: %{optional(binary()) => integer()} - def read_proc_io(path) do - case read_raw_file(path) do - nil -> %{} - content -> parse_keyed_kv(content) - end - end - - # Parse a ":[ ]" file (status, io) into a map of - # binary key -> integer. The text after the colon is trimmed and split on - # spaces; the first token is taken as the value and any trailing unit (e.g. - # "kB") is ignored. Lines whose value doesn't parse as an integer are dropped. - defp parse_keyed_kv(content) do - content - |> String.split("\n", trim: true) - |> Enum.reduce(%{}, fn line, acc -> - case String.split(line, ":", parts: 2) do - [key, rest] -> - case rest |> String.trim() |> String.split(" ", trim: true) do - [value | _] -> - case parse_int(value) do - nil -> acc - int -> Map.put(acc, key, int) - end - - _ -> - acc - end - - _ -> - acc - end - end) - end - - ## low-level file/number helpers ----------------------------------------- + @spec read_int_file(Path.t()) :: integer() | nil + def read_int_file(path), do: parse_int(read_raw_file(path)) @doc """ Read a file, returning its trimmed content or `nil` on any error (missing @@ -113,4 +64,18 @@ defmodule ElectricTelemetry.SystemMetrics.ProcfsParse do _ -> nil end end + + @doc """ + Parse a string as a float, returning `nil` on `nil` input or any non-float + content. + """ + @spec parse_float(binary() | nil) :: float() | nil + def parse_float(nil), do: nil + + def parse_float(str) do + case Float.parse(String.trim(str)) do + {float, ""} -> float + _ -> nil + end + end end From 41783856553356f0444751f2cb1159a3b3bddaba Mon Sep 17 00:00:00 2001 From: Oleksii Sholik Date: Tue, 7 Jul 2026 12:30:01 +0200 Subject: [PATCH 10/13] Unify the grouped and ungrouped disk usage walks Disk.recursive_usage/2 and recursive_usage_grouped/3 were two parallel copies of the same traversal whose totals had to be kept byte-identical by hand (with a comment and a test enforcing it). The grouped walk with a nil group_depth never establishes a bucket key, so it already IS the ungrouped walk: recursive_usage/2 is now a thin wrapper over it and the duplicate walker is deleted, making the byte-identical contract true by construction. This in turn collapses DiskUsage.read_disk_usage's group_depth branching into a single call. Also share the defensive ETS lookup between current/1 and current_dirs/1, and rename the telemetry opt shape_dir_group_depth -> disk_usage_group_depth: the generic package buckets directories at a depth and knows nothing about shapes, so the shape vocabulary now stays in sync-service where the depth is derived. Co-Authored-By: Claude Fable 5 --- .../lib/electric/telemetry/disk_usage.ex | 75 ++++++------------- .../lib/electric/telemetry/disk_usage/disk.ex | 66 +++++----------- .../lib/electric/telemetry/opts.ex | 7 +- .../lib/electric/telemetry/stack_telemetry.ex | 16 ++-- .../electric/stack_supervisor/telemetry.ex | 2 +- 5 files changed, 51 insertions(+), 115 deletions(-) diff --git a/packages/electric-telemetry/lib/electric/telemetry/disk_usage.ex b/packages/electric-telemetry/lib/electric/telemetry/disk_usage.ex index 4b2a4ab44c..143d53b812 100644 --- a/packages/electric-telemetry/lib/electric/telemetry/disk_usage.ex +++ b/packages/electric-telemetry/lib/electric/telemetry/disk_usage.ex @@ -16,15 +16,10 @@ defmodule ElectricTelemetry.DiskUsage do end def current(stack_id) do - table = name(stack_id) - - case :ets.lookup(table, :usage_bytes) do - [] -> :pending - [{:usage_bytes, _bytes, nil, _duration}] -> :pending + case lookup(stack_id, :usage_bytes) do [{:usage_bytes, bytes, %DateTime{}, duration}] -> {:ok, bytes, duration} + _ -> :pending end - rescue - ArgumentError -> :pending end @doc """ @@ -33,14 +28,17 @@ defmodule ElectricTelemetry.DiskUsage do measurement with grouping enabled has completed yet. """ def current_dirs(stack_id) do - table = name(stack_id) - - case :ets.lookup(table, :top_dirs) do + case lookup(stack_id, :top_dirs) do [{:top_dirs, dirs}] -> {:ok, dirs} - [] -> :pending + _ -> :pending end + end + + # The table doesn't exist until this stack's DiskUsage server has started. + defp lookup(stack_id, key) do + :ets.lookup(name(stack_id), key) rescue - ArgumentError -> :pending + ArgumentError -> [] end @impl GenServer @@ -99,31 +97,18 @@ defmodule ElectricTelemetry.DiskUsage do defp read_disk_usage(state) do exclude = [usage_cache_file(state.storage_dir)] - {duration, {bytes, top_dirs}} = + {duration, {bytes, buckets}} = :timer.tc( - fn -> - case state.group_depth do - nil -> - {Disk.recursive_usage(state.storage_dir, exclude), nil} - - depth when is_integer(depth) -> - {total, buckets} = - Disk.recursive_usage_grouped(state.storage_dir, exclude, depth) - - {total, top_n(buckets, state.top_n)} - end - end, + fn -> Disk.recursive_usage_grouped(state.storage_dir, exclude, state.group_depth) end, :millisecond ) - updated_at = DateTime.utc_now() - %{ state | usage_bytes: bytes, - updated_at: updated_at, + updated_at: DateTime.utc_now(), measurement_duration: duration, - top_dirs: top_dirs + top_dirs: state.group_depth && top_n(buckets, state.top_n) } |> ets_write() |> save_usage!() @@ -131,17 +116,10 @@ defmodule ElectricTelemetry.DiskUsage do |> schedule_update() end - # Returns the `n` largest `{name, bytes}` buckets, sorted descending by size. - # - # Scale tradeoff: the full `%{dir => bytes}` map is materialized for the whole - # tree and sorted (O(k log k) in the number of buckets `k`) once per walk - # before being trimmed to the top `n`. Only the top `n` are retained in state - # and ETS, so steady-state memory stays bounded; the transient cost is the - # full map plus one sort per ~60s cycle. This is comfortable at ~10k shapes - # per stack and acceptable into the low hundreds of thousands. If a single - # stack ever holds enough shapes for that transient sort to cause real GC - # pressure, replace this with a running bounded top-N (min-heap) that never - # materializes the full sorted list. + # The `n` largest `{name, bytes}` buckets, sorted descending by size. The + # full bucket map is materialized and sorted once per (~60s) walk before + # being trimmed; fine at ~10k shapes per stack — replace with a bounded + # min-heap if a stack ever holds hundreds of thousands. defp top_n(buckets, n) do buckets |> Enum.sort_by(fn {_name, bytes} -> bytes end, :desc) @@ -149,19 +127,12 @@ defmodule ElectricTelemetry.DiskUsage do end defp ets_write(state) do - %{ - usage_bytes: usage_bytes, - updated_at: updated_at, - table: table, - measurement_duration: duration - } = state + :ets.insert( + state.table, + {:usage_bytes, state.usage_bytes, state.updated_at, state.measurement_duration} + ) - :ets.insert(table, {:usage_bytes, usage_bytes, updated_at, duration}) - - case Map.get(state, :top_dirs) do - nil -> :ok - dirs -> :ets.insert(table, {:top_dirs, dirs}) - end + if dirs = state.top_dirs, do: :ets.insert(state.table, {:top_dirs, dirs}) state end diff --git a/packages/electric-telemetry/lib/electric/telemetry/disk_usage/disk.ex b/packages/electric-telemetry/lib/electric/telemetry/disk_usage/disk.ex index 1fd4b06aa6..6cad150f3b 100644 --- a/packages/electric-telemetry/lib/electric/telemetry/disk_usage/disk.ex +++ b/packages/electric-telemetry/lib/electric/telemetry/disk_usage/disk.ex @@ -8,66 +8,34 @@ defmodule ElectricTelemetry.DiskUsage.Disk do paths listed in `exclude`. """ def recursive_usage(path, exclude) do - do_recursive_usage(path, MapSet.new(exclude), 0) + {total, _buckets} = recursive_usage_grouped(path, exclude, nil) + total end @doc """ - Like `recursive_usage/2`, but in a single traversal also returns a map of - per-directory subtotals bucketed at `group_depth`. + Like `recursive_usage/2`, but in the same single traversal also returns a map + of per-directory subtotals bucketed at `group_depth` (`nil` disables + bucketing and yields an empty map). - The returned total is byte-identical to `recursive_usage/2` for the same - `path`/`exclude`. The bucket map is keyed by the name (not full path) of each - directory found at exactly `group_depth` levels below `path` (the root `path` - itself is depth 0), with the value being the recursive size of every regular - file under that directory. Excluded paths contribute to neither the total nor - the buckets. + The bucket map is keyed by the name (not full path) of each directory found + at exactly `group_depth` levels below `path` (the root `path` itself is depth + 0), with the value being the recursive size of every regular file under that + directory. Excluded paths contribute to neither the total nor the buckets. + The buckets are a best-effort tally that only ever grows, while the total + keeps the legacy behaviour of resetting to 0 on an unreadable entry. Returns `{total_bytes, %{dir_name => bytes}}`. """ def recursive_usage_grouped(path, exclude, group_depth) - when is_integer(group_depth) and group_depth >= 0 do + when is_nil(group_depth) or (is_integer(group_depth) and group_depth >= 0) do # When grouping at the root (depth 0), the root's own basename is the bucket. initial_key = if group_depth == 0, do: Path.basename(path), else: nil - do_recursive_usage_grouped(path, initial_key, MapSet.new(exclude), group_depth, 0, {0, %{}}) + walk(path, initial_key, MapSet.new(exclude), group_depth, 0, {0, %{}}) end - def do_recursive_usage(path, exclude, acc) do - case stat(path) do - {:ok, file_info(size: size, type: :regular)} -> - if MapSet.member?(exclude, path) do - acc - else - size + acc - end - - {:ok, file_info(type: :directory)} -> - case ls(path) do - {:ok, files} -> - Enum.reduce(files, acc, &do_recursive_usage(Path.join(path, &1), exclude, &2)) - - {:error, _} -> - 0 - end - - {:ok, _} -> - 0 - - {:error, _} -> - 0 - end - end - - # Threads the running total `acc` through the traversal exactly as - # `do_recursive_usage/3` does — including its legacy semantics where an - # unreadable/non-regular entry resets the threaded total to 0 — so the - # returned total is byte-identical. The `buckets` map is carried alongside and - # only ever grows on regular files; it is intentionally NOT reset on those - # error paths (it is a best-effort per-bucket tally, while the total preserves - # exact backwards compatibility). - # # `bucket_key` is the name of the ancestor directory sitting at `group_depth`, # or `nil` until that depth is reached. - defp do_recursive_usage_grouped(path, bucket_key, exclude, group_depth, depth, {acc, buckets}) do + defp walk(path, bucket_key, exclude, group_depth, depth, {acc, buckets}) do case stat(path) do {:ok, file_info(size: size, type: :regular)} -> if MapSet.member?(exclude, path) do @@ -81,8 +49,8 @@ defmodule ElectricTelemetry.DiskUsage.Disk do {:ok, files} -> Enum.reduce(files, {acc, buckets}, fn name, {acc, bucks} -> # `name` is a charlist from :prim_file.list_dir/1 and sits at - # `depth + 1`. The bucket key is the directory name at exactly - # `group_depth`; once established it propagates to descendants. + # `depth + 1`. Once established, the bucket key propagates to all + # descendants. child_bucket_key = cond do not is_nil(bucket_key) -> bucket_key @@ -90,7 +58,7 @@ defmodule ElectricTelemetry.DiskUsage.Disk do true -> nil end - do_recursive_usage_grouped( + walk( Path.join(path, name), child_bucket_key, exclude, diff --git a/packages/electric-telemetry/lib/electric/telemetry/opts.ex b/packages/electric-telemetry/lib/electric/telemetry/opts.ex index 08bb8983ff..7082c1bfa0 100644 --- a/packages/electric-telemetry/lib/electric/telemetry/opts.ex +++ b/packages/electric-telemetry/lib/electric/telemetry/opts.ex @@ -5,10 +5,9 @@ defmodule ElectricTelemetry.Opts do installation_id: [type: :string], stack_id: [type: :string], storage_dir: [type: :string], - # Directory depth (relative to the disk-usage walk root) at which a single - # shape's directory sits, used to bucket the per-shape - # `electric.storage.dir.bytes` metric. `nil` disables per-shape grouping. - shape_dir_group_depth: [type: {:or, [:pos_integer, nil]}, default: nil], + # Directory depth (relative to the disk-usage walk root) at which to + # bucket per-directory disk usage subtotals; `nil` disables bucketing. + disk_usage_group_depth: [type: {:or, [:pos_integer, nil]}, default: nil], version: [type: :string, required: true], reporters: [ type: :keyword_list, diff --git a/packages/electric-telemetry/lib/electric/telemetry/stack_telemetry.ex b/packages/electric-telemetry/lib/electric/telemetry/stack_telemetry.ex index 9bfd3f5b41..91884bceb1 100644 --- a/packages/electric-telemetry/lib/electric/telemetry/stack_telemetry.ex +++ b/packages/electric-telemetry/lib/electric/telemetry/stack_telemetry.ex @@ -67,21 +67,19 @@ defmodule ElectricTelemetry.StackTelemetry do ] end - # `:shape_dir_group_depth` is the directory depth (relative to the disk-usage - # walk root) at which a single shape's directory sits, supplied by the caller - # since it depends on the deployment's storage layout (the generic telemetry - # package stays layout-agnostic). When set, the disk walk additionally buckets - # per-shape subtotals keyed by shape handle and emits them as - # `electric.storage.dir.bytes` for the top-N largest shapes. When `nil` (e.g. - # storage backends without an on-disk shape layout), only the aggregate total - # is measured. + # `:disk_usage_group_depth` is supplied by the caller since it depends on the + # deployment's storage layout (sync-service derives it from the real shape + # storage path, keeping this package layout-agnostic). When set, the disk walk + # additionally buckets per-directory subtotals at that depth — for Electric + # these are shape dirs, emitted as `electric.storage.dir.bytes` for the top-N + # largest. When `nil`, only the aggregate total is measured. defp disk_usage_child_specs(%{stack_id: stack_id} = opts) do if storage_dir = Map.get(opts, :storage_dir) do [ {ElectricTelemetry.DiskUsage, stack_id: stack_id, storage_dir: storage_dir, - group_depth: Map.get(opts, :shape_dir_group_depth)} + group_depth: Map.get(opts, :disk_usage_group_depth)} ] else [] diff --git a/packages/sync-service/lib/electric/stack_supervisor/telemetry.ex b/packages/sync-service/lib/electric/stack_supervisor/telemetry.ex index 24875b5e6b..daf19b6cdc 100644 --- a/packages/sync-service/lib/electric/stack_supervisor/telemetry.ex +++ b/packages/sync-service/lib/electric/stack_supervisor/telemetry.ex @@ -221,7 +221,7 @@ defmodule Electric.StackSupervisor.Telemetry do config.telemetry_opts |> Keyword.put(:stack_id, config.stack_id) |> Keyword.put(:storage_dir, config.storage_dir) - |> Keyword.put(:shape_dir_group_depth, shape_dir_group_depth(config)) + |> Keyword.put(:disk_usage_group_depth, shape_dir_group_depth(config)) |> Keyword.put(:otel_opts, otel_opts) # Always enable default periodic measurements in addition to the user-provided ones |> Keyword.update( From 27b67c0ca83938dbfce9e018322f655710139530 Mon Sep 17 00:00:00 2001 From: Oleksii Sholik Date: Tue, 7 Jul 2026 12:48:14 +0200 Subject: [PATCH 11/13] Remove host.mem.* metrics as duplicates of system.memory.* MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ApplicationTelemetry already reads host memory via :memsup.get_system_memory_data() (meminfo-backed on Linux) and emits it as the [:system, :memory] event, so the meminfo-derived host.mem.total/ free/cached family added on this branch was a second reader for the same three numbers. Drop it; the Proc reader now covers only the genuinely new data — the BEAM's own /proc//status RSS breakdown and /proc//io counters (host.proc.beam.*). Co-Authored-By: Claude Fable 5 --- .changeset/per-shape-disk-usage-metric.md | 2 +- integration-tests/tests/otel-export.lux | 3 +- .../telemetry/application_telemetry.ex | 5 +- .../electric/telemetry/system_metrics/proc.ex | 24 ++------ .../telemetry/system_metrics/proc_test.exs | 59 ------------------- 5 files changed, 9 insertions(+), 84 deletions(-) diff --git a/.changeset/per-shape-disk-usage-metric.md b/.changeset/per-shape-disk-usage-metric.md index 4f37ce8c26..015f9fc34c 100644 --- a/.changeset/per-shape-disk-usage-metric.md +++ b/.changeset/per-shape-disk-usage-metric.md @@ -3,4 +3,4 @@ "@core/sync-service": patch --- -Export in-app BEAM allocator (`vm.alloc.*`), cgroup (`cgroup.*`), and host/process (`host.mem.*`, `host.proc.beam.*`) metrics for reconciling VM-reported memory/CPU/disk against what the kernel charges. Also emit a new `electric.storage.dir.bytes` stack-level metric reporting on-disk size for the top-N largest shapes (tagged by shape handle), computed during the existing periodic disk-usage walk so it adds no extra filesystem traversal. +Export in-app BEAM allocator (`vm.alloc.*`), cgroup v2 (`cgroup.*`), and per-process (`host.proc.beam.*`) metrics for reconciling VM-reported memory/CPU/disk against what the kernel charges. Also emit a new `electric.storage.dir.bytes` stack-level metric reporting on-disk size for the top-N largest shapes (tagged by shape handle), computed during the existing periodic disk-usage walk so it adds no extra filesystem traversal. diff --git a/integration-tests/tests/otel-export.lux b/integration-tests/tests/otel-export.lux index 98eb23e5d6..978e9c34df 100644 --- a/integration-tests/tests/otel-export.lux +++ b/integration-tests/tests/otel-export.lux @@ -108,9 +108,8 @@ [invoke grep_otel_collector_output "Metric vm\.alloc\.unused .*Value=[0-9]+"] [invoke grep_otel_collector_output "Metric vm\.alloc\.carrier_usage .*Value=[.0-9]+"] - # Verify host /proc metrics (host.mem.* / host.proc.beam.*) from SystemMetrics. + # Verify per-process /proc metrics (host.proc.beam.*) from SystemMetrics. # These read /proc and are populated on Linux (the CI runner). - [invoke grep_otel_collector_output "Metric host\.mem\.total .*Value=[0-9]+"] [invoke grep_otel_collector_output "Metric host\.proc\.beam\.vm_rss .*Value=[0-9]+"] # Verify cgroup accounting metrics (cgroup.*) from SystemMetrics. Populated when a diff --git a/packages/electric-telemetry/lib/electric/telemetry/application_telemetry.ex b/packages/electric-telemetry/lib/electric/telemetry/application_telemetry.ex index 892747e111..b949f1b243 100644 --- a/packages/electric-telemetry/lib/electric/telemetry/application_telemetry.ex +++ b/packages/electric-telemetry/lib/electric/telemetry/application_telemetry.ex @@ -147,11 +147,8 @@ defmodule ElectricTelemetry.ApplicationTelemetry do last_value("cgroup.cpu.pressure.full.avg10"), last_value("cgroup.io.rbytes", unit: :byte), last_value("cgroup.io.wbytes", unit: :byte), - # host/process /proc metrics (emitted by + # per-process /proc metrics (emitted by # ElectricTelemetry.SystemMetrics.Proc). - last_value("host.mem.total", unit: :byte), - last_value("host.mem.free", unit: :byte), - last_value("host.mem.cached", unit: :byte), last_value("host.proc.beam.rss_anon", unit: :byte), last_value("host.proc.beam.rss_file", unit: :byte), last_value("host.proc.beam.rss_shmem", unit: :byte), diff --git a/packages/electric-telemetry/lib/electric/telemetry/system_metrics/proc.ex b/packages/electric-telemetry/lib/electric/telemetry/system_metrics/proc.ex index 395c9b48eb..ac9f1babd5 100644 --- a/packages/electric-telemetry/lib/electric/telemetry/system_metrics/proc.ex +++ b/packages/electric-telemetry/lib/electric/telemetry/system_metrics/proc.ex @@ -1,13 +1,9 @@ defmodule ElectricTelemetry.SystemMetrics.Proc do @moduledoc """ - Reads Linux `/proc` accounting files and emits `host.mem.*` (from - `/proc/meminfo`) and `host.proc.beam.*` (from the BEAM's own - `/proc//status` and `/proc//io`) telemetry. - - This bridges the cgroup plane and the BEAM allocator plane: the per-process - RSS breakdown (anon vs file vs shmem) shows cgroup `anon` tracking the BEAM - footprint, and meminfo gives the host-RAM ceiling/cache picture (one task per - host on EC2, so host ≈ container). + Reads the BEAM's own `/proc//status` and `/proc//io` and emits + `host.proc.beam.*` telemetry: the per-process RSS breakdown (anon vs file vs + shmem) shows cgroup `anon` tracking the BEAM's anonymous footprint, and the + IO byte counters give the kernel's view of this process's disk traffic. All reads are defensive: a missing file, a permission error (`/proc//io` can be EACCES-restricted even for self in some sandboxes), or unexpected @@ -15,7 +11,7 @@ defmodule ElectricTelemetry.SystemMetrics.Proc do `/proc` only exists on Linux, so the measurement is a clean no-op everywhere else. - meminfo values and status `Rss*`/`VmRSS` are in kB and converted to bytes; + Status `Rss*`/`VmRSS` values are in kB and converted to bytes; `/proc//io` `read_bytes`/`write_bytes` are already bytes and emitted as-is. """ @@ -28,7 +24,7 @@ defmodule ElectricTelemetry.SystemMetrics.Proc do @default_root "/proc" @doc """ - Read the `/proc` accounting files and emit `host.mem.*` / `host.proc.beam.*` + Read the BEAM's `/proc/` accounting files and emit `host.proc.beam.*` telemetry events. Reads every tick (no slow-tier gating). Options: @@ -48,14 +44,6 @@ defmodule ElectricTelemetry.SystemMetrics.Proc do root = Keyword.get(opts, :proc_root, @default_root) pid = opts |> Keyword.get_lazy(:pid, fn -> :os.getpid() end) |> to_string() - meminfo = read_kv_file(Path.join(root, "meminfo")) - - emit([:host, :mem], %{ - total: kb_to_bytes(meminfo["MemTotal"]), - free: kb_to_bytes(meminfo["MemFree"]), - cached: kb_to_bytes(meminfo["Cached"]) - }) - status = read_kv_file(Path.join([root, pid, "status"])) emit([:host, :proc, :beam], %{ diff --git a/packages/electric-telemetry/test/electric/telemetry/system_metrics/proc_test.exs b/packages/electric-telemetry/test/electric/telemetry/system_metrics/proc_test.exs index 928dbab1ef..f0ec394cb9 100644 --- a/packages/electric-telemetry/test/electric/telemetry/system_metrics/proc_test.exs +++ b/packages/electric-telemetry/test/electric/telemetry/system_metrics/proc_test.exs @@ -15,7 +15,6 @@ defmodule ElectricTelemetry.SystemMetrics.ProcTest do ref = make_ref() events = [ - [:host, :mem], [:host, :proc, :beam], [:host, :proc, :beam, :io] ] @@ -62,17 +61,6 @@ defmodule ElectricTelemetry.SystemMetrics.ProcTest do File.write!(path, content) end - defp meminfo do - """ - MemTotal: 16384000 kB - MemFree: 2048000 kB - MemAvailable: 8192000 kB - Buffers: 102400 kB - Cached: 4096000 kB - SwapTotal: 0 kB - """ - end - defp status do """ Name:\tbeam.smp @@ -103,24 +91,12 @@ defmodule ElectricTelemetry.SystemMetrics.ProcTest do setup %{tmp_dir: tmp_dir} do root = Path.join(tmp_dir, "proc") - write_file(root, "meminfo", meminfo()) write_file(root, "#{@pid}/status", status()) write_file(root, "#{@pid}/io", proc_io()) %{root: root} end - test "emits host memory metrics in bytes (kB * 1024)", %{root: root} do - m = - collect_host_metrics(fn -> - Proc.measurement(%{}, os: {:unix, :linux}, proc_root: root, pid: @pid) - end) - - assert m["host.mem.total"] == 16_384_000 * 1024 - assert m["host.mem.free"] == 2_048_000 * 1024 - assert m["host.mem.cached"] == 4_096_000 * 1024 - end - test "emits BEAM Rss*/VmRSS in bytes (kB * 1024)", %{root: root} do m = collect_host_metrics(fn -> @@ -156,7 +132,6 @@ defmodule ElectricTelemetry.SystemMetrics.ProcTest do describe "non-linux guard" do test "no-ops and emits nothing on macOS", %{tmp_dir: tmp_dir} do root = Path.join(tmp_dir, "proc") - write_file(root, "meminfo", meminfo()) write_file(root, "#{@pid}/status", status()) write_file(root, "#{@pid}/io", proc_io()) @@ -171,26 +146,8 @@ defmodule ElectricTelemetry.SystemMetrics.ProcTest do end describe "missing / malformed files" do - test "missing meminfo skips host.mem.* but still emits process metrics", %{tmp_dir: tmp_dir} do - root = Path.join(tmp_dir, "proc") - write_file(root, "#{@pid}/status", status()) - write_file(root, "#{@pid}/io", proc_io()) - - m = - collect_host_metrics(fn -> - assert :ok = Proc.measurement(%{}, os: {:unix, :linux}, proc_root: root, pid: @pid) - end) - - refute Map.has_key?(m, "host.mem.total") - refute Map.has_key?(m, "host.mem.free") - refute Map.has_key?(m, "host.mem.cached") - assert m["host.proc.beam.rss_anon"] == 300_000 * 1024 - assert m["host.proc.beam.io.read_bytes"] == 65_536 - end - test "missing status file skips beam.rss_* but other metrics still emit", %{tmp_dir: tmp_dir} do root = Path.join(tmp_dir, "proc") - write_file(root, "meminfo", meminfo()) write_file(root, "#{@pid}/io", proc_io()) m = @@ -200,7 +157,6 @@ defmodule ElectricTelemetry.SystemMetrics.ProcTest do refute Map.has_key?(m, "host.proc.beam.rss_anon") refute Map.has_key?(m, "host.proc.beam.vm_rss") - assert m["host.mem.total"] == 16_384_000 * 1024 assert m["host.proc.beam.io.read_bytes"] == 65_536 end @@ -208,7 +164,6 @@ defmodule ElectricTelemetry.SystemMetrics.ProcTest do tmp_dir: tmp_dir } do root = Path.join(tmp_dir, "proc") - write_file(root, "meminfo", meminfo()) write_file(root, "#{@pid}/status", status()) # no io file written @@ -219,19 +174,12 @@ defmodule ElectricTelemetry.SystemMetrics.ProcTest do refute Map.has_key?(m, "host.proc.beam.io.read_bytes") refute Map.has_key?(m, "host.proc.beam.io.write_bytes") - assert m["host.mem.total"] == 16_384_000 * 1024 assert m["host.proc.beam.rss_anon"] == 300_000 * 1024 end test "malformed value skips that metric, others still emitted", %{tmp_dir: tmp_dir} do root = Path.join(tmp_dir, "proc") - write_file(root, "meminfo", """ - MemTotal: notanumber kB - MemFree: 2048000 kB - Cached: 4096000 kB - """) - write_file(root, "#{@pid}/status", """ VmRSS:\t garbage kB RssAnon:\t 300000 kB @@ -247,10 +195,6 @@ defmodule ElectricTelemetry.SystemMetrics.ProcTest do assert :ok = Proc.measurement(%{}, os: {:unix, :linux}, proc_root: root, pid: @pid) end) - refute Map.has_key?(m, "host.mem.total") - assert m["host.mem.free"] == 2_048_000 * 1024 - assert m["host.mem.cached"] == 4_096_000 * 1024 - refute Map.has_key?(m, "host.proc.beam.vm_rss") assert m["host.proc.beam.rss_anon"] == 300_000 * 1024 @@ -276,9 +220,6 @@ defmodule ElectricTelemetry.SystemMetrics.ProcTest do ElectricTelemetry.ApplicationTelemetry.metrics(%{}) |> Enum.map(& &1.name) - assert [:host, :mem, :total] in names - assert [:host, :mem, :free] in names - assert [:host, :mem, :cached] in names assert [:host, :proc, :beam, :rss_anon] in names assert [:host, :proc, :beam, :rss_file] in names assert [:host, :proc, :beam, :rss_shmem] in names From 9e6eb1f61e68634434c52380fa104d3300a960cf Mon Sep 17 00:00:00 2001 From: Oleksii Sholik Date: Tue, 7 Jul 2026 12:50:19 +0200 Subject: [PATCH 12/13] Sample allocator fragmentation from SystemMonitor's own timer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The per-allocator fragmentation breakdown is O(carriers) and was registered with the shared ~5s poller, self-gating to every 12th tick through a hand-rolled :counters ref cached in :persistent_term — plus :force/:gate_key test-only options and a public interval accessor to make the gate testable. SystemMonitor is already a long-lived GenServer doing periodic work (the hourly full GC), so give it a one-minute timer that calls the now ungated allocator_fragmentation_measurement/0 directly. This deletes the whole gate: the counter, the persistent_term entry, both test-only options, and the accessor. The sampling call is wrapped in a rescue so a metrics hiccup can't take down the system monitor. Co-Authored-By: Claude Fable 5 --- .../telemetry/application_telemetry.ex | 6 +- .../lib/electric/telemetry/system_metrics.ex | 78 ++++++------------- .../lib/electric/telemetry/system_monitor.ex | 28 +++++++ .../telemetry/system_metrics_test.exs | 47 ++--------- 4 files changed, 61 insertions(+), 98 deletions(-) diff --git a/packages/electric-telemetry/lib/electric/telemetry/application_telemetry.ex b/packages/electric-telemetry/lib/electric/telemetry/application_telemetry.ex index b949f1b243..708bcc7526 100644 --- a/packages/electric-telemetry/lib/electric/telemetry/application_telemetry.ex +++ b/packages/electric-telemetry/lib/electric/telemetry/application_telemetry.ex @@ -89,10 +89,10 @@ defmodule ElectricTelemetry.ApplicationTelemetry do ) ++ [ # System metrics (vm.alloc.*, cgroup.*, host.*); each measurement no-ops - # where unsupported (no cgroup v2, non-Linux). The per-allocator - # fragmentation breakdown internally gates itself to ~once a minute. + # where unsupported (no cgroup v2, non-Linux). The expensive + # vm.alloc.fragmentation.* breakdown is sampled by SystemMonitor on a + # one-minute timer instead of this poller. {ElectricTelemetry.SystemMetrics, :recon_alloc_measurement, [telemetry_opts]}, - {ElectricTelemetry.SystemMetrics, :allocator_fragmentation_measurement, [telemetry_opts]}, {ElectricTelemetry.SystemMetrics.Cgroup, :measurement, [telemetry_opts]}, {ElectricTelemetry.SystemMetrics.Proc, :measurement, [telemetry_opts]} ] diff --git a/packages/electric-telemetry/lib/electric/telemetry/system_metrics.ex b/packages/electric-telemetry/lib/electric/telemetry/system_metrics.ex index 3982817609..4ab59e1903 100644 --- a/packages/electric-telemetry/lib/electric/telemetry/system_metrics.ex +++ b/packages/electric-telemetry/lib/electric/telemetry/system_metrics.ex @@ -4,9 +4,12 @@ defmodule ElectricTelemetry.SystemMetrics do complement the application-level VM stats in `ElectricTelemetry.ApplicationTelemetry`. These functions are not a process; `ElectricTelemetry.Poller` invokes them - (and the `SystemMetrics.Cgroup` / `SystemMetrics.Proc` readers) as MFA tuples, - wrapping every invocation in `safe_invoke/3` so a crash here is logged and - swallowed rather than removing the measurement. + (and the `SystemMetrics.Cgroup` / `SystemMetrics.Proc` readers) as MFA tuples + on its ~5s tick, wrapping every invocation in `safe_invoke/3` so a crash here + is logged and swallowed rather than removing the measurement. The one + exception is `allocator_fragmentation_measurement/0`, which is too expensive + for that tick and is instead sampled by `ElectricTelemetry.SystemMonitor` on + a one-minute timer. This module holds the BEAM allocator metrics (`vm.alloc.*`, derived from `:recon_alloc`), the cached platform/cgroup detection (`system_info/0`), and @@ -14,21 +17,6 @@ defmodule ElectricTelemetry.SystemMetrics do """ @system_info_key {__MODULE__, :system_info} - @fragmentation_gate_key {__MODULE__, :fragmentation_gate} - - # The poller runs all measurements at a single ~5s period, and - # `:recon_alloc.fragmentation/1` is O(carriers) — too expensive for every - # tick — so it self-gates to every 12th tick (~once a minute). The gate is an - # atomic `:counters` ref cached in `:persistent_term`: `:counters.add/3` is a - # lock-free bump, whereas a `:persistent_term.put` per tick would trigger a - # global GC scan of all process heaps. - @fragmentation_interval_ticks 12 - - @doc """ - Number of poll ticks between per-allocator fragmentation samples. - """ - @spec allocator_fragmentation_interval_ticks() :: pos_integer() - def allocator_fragmentation_interval_ticks, do: @fragmentation_interval_ticks @doc """ Emit a telemetry event carrying the non-nil `measurements` in a single @@ -89,50 +77,32 @@ defmodule ElectricTelemetry.SystemMetrics do end @doc """ - Per-allocator fragmentation breakdown (slow tier, ~once a minute — see - `@fragmentation_interval_ticks`). + Per-allocator fragmentation breakdown. Sums unused carrier bytes from `:recon_alloc.fragmentation(:current)` per allocator *type* and emits one `[:vm, :alloc, :fragmentation]` event per - type, tagged by `allocator`. - - Options: - * `:force` — when `true`, always run regardless of the gate (tests) - * `:gate_key` — override the `:persistent_term` key holding the tick counter - (used by tests to avoid contending with the live poller's counter) + type, tagged by `allocator`. This is O(carriers) — too expensive for the + poller's ~5s tick — so `ElectricTelemetry.SystemMonitor` samples it on its + own one-minute timer instead. """ - @spec allocator_fragmentation_measurement(map(), keyword()) :: :ok - def allocator_fragmentation_measurement(_telemetry_opts, opts \\ []) do - if due?(opts) do - :recon_alloc.fragmentation(:current) - |> Enum.reduce(%{}, fn {{type, _instance}, info}, acc -> - unused = unused_bytes(info) - Map.update(acc, type, unused, &(&1 + unused)) - end) - |> Enum.each(fn {type, unused} -> - :telemetry.execute( - [:vm, :alloc, :fragmentation], - %{unused: unused}, - %{allocator: to_string(type)} - ) - end) - end + @spec allocator_fragmentation_measurement() :: :ok + def allocator_fragmentation_measurement do + :recon_alloc.fragmentation(:current) + |> Enum.reduce(%{}, fn {{type, _instance}, info}, acc -> + unused = unused_bytes(info) + Map.update(acc, type, unused, &(&1 + unused)) + end) + |> Enum.each(fn {type, unused} -> + :telemetry.execute( + [:vm, :alloc, :fragmentation], + %{unused: unused}, + %{allocator: to_string(type)} + ) + end) :ok end - defp due?(opts) do - if Keyword.get(opts, :force, false) do - true - else - gate_key = Keyword.get(opts, :gate_key, @fragmentation_gate_key) - ref = memoized(gate_key, fn -> :counters.new(1, [:write_concurrency]) end) - :counters.add(ref, 1, 1) - # The first tick reads 1, so the gate fires on the Nth tick (not on boot). - rem(:counters.get(ref, 1), @fragmentation_interval_ticks) == 0 - end - end - # Get-or-compute a `:persistent_term` entry. The put happens at most once per # key (on first call), never on the per-tick hot path. defp memoized(key, fun) do diff --git a/packages/electric-telemetry/lib/electric/telemetry/system_monitor.ex b/packages/electric-telemetry/lib/electric/telemetry/system_monitor.ex index a1b3d82160..34a956da05 100644 --- a/packages/electric-telemetry/lib/electric/telemetry/system_monitor.ex +++ b/packages/electric-telemetry/lib/electric/telemetry/system_monitor.ex @@ -8,6 +8,9 @@ defmodule ElectricTelemetry.SystemMonitor do - long_schedule - long_message_queue + It also hosts slow periodic work that doesn't fit the poller's single ~5s + period: the per-allocator fragmentation sampling (`vm.alloc.fragmentation.*`) + runs on its own one-minute timer here. """ use GenServer @@ -21,6 +24,8 @@ defmodule ElectricTelemetry.SystemMonitor do @vm_monitor_long_message_queue [:vm, :monitor, :long_message_queue] @garbage_collect_interval :timer.hours(1) @garbage_collect_message :periodic_garbage_collect + @allocator_fragmentation_interval :timer.minutes(1) + @allocator_fragmentation_message :sample_allocator_fragmentation def start_link(opts) do GenServer.start_link(__MODULE__, opts.intervals_and_thresholds, name: __MODULE__) @@ -40,6 +45,8 @@ defmodule ElectricTelemetry.SystemMonitor do garbage_collect_timer: nil } + schedule_allocator_fragmentation() + {:ok, schedule_garbage_collect(state)} end @@ -142,6 +149,19 @@ defmodule ElectricTelemetry.SystemMonitor do {:noreply, %{state | garbage_collect_timer: nil} |> schedule_garbage_collect()} end + def handle_info(@allocator_fragmentation_message, state) do + # Don't let a metrics hiccup take down the system monitor. + try do + ElectricTelemetry.SystemMetrics.allocator_fragmentation_measurement() + rescue + error -> Logger.warning("Allocator fragmentation sampling failed: #{inspect(error)}") + end + + schedule_allocator_fragmentation() + + {:noreply, state} + end + defp log_long_message_queue_event(pid, type) do with {:message_queue_len, queue_len} <- Process.info(pid, :message_queue_len) do :telemetry.execute(@vm_monitor_long_message_queue, %{length: queue_len}, %{ @@ -163,6 +183,14 @@ defmodule ElectricTelemetry.SystemMonitor do defp maybe_start_long_message_queue_timer(state), do: state + defp schedule_allocator_fragmentation do + Process.send_after( + self(), + @allocator_fragmentation_message, + @allocator_fragmentation_interval + ) + end + defp schedule_garbage_collect(%{garbage_collect_timer: nil} = state) do %{ state diff --git a/packages/electric-telemetry/test/electric/telemetry/system_metrics_test.exs b/packages/electric-telemetry/test/electric/telemetry/system_metrics_test.exs index 207f3552ac..d2553ce162 100644 --- a/packages/electric-telemetry/test/electric/telemetry/system_metrics_test.exs +++ b/packages/electric-telemetry/test/electric/telemetry/system_metrics_test.exs @@ -66,8 +66,8 @@ defmodule ElectricTelemetry.SystemMetricsTest do end end - describe "allocator_fragmentation_measurement/1" do - test "emits per-allocator unused bytes when due" do + describe "allocator_fragmentation_measurement/0" do + test "emits unused bytes per allocator type" do ref = make_ref() handler_id = {__MODULE__, ref} @@ -84,55 +84,20 @@ defmodule ElectricTelemetry.SystemMetricsTest do on_exit(fn -> :telemetry.detach(handler_id) end) - # Force a due tick regardless of the persistent counter. - assert :ok = SystemMetrics.allocator_fragmentation_measurement(%{}, force: true) + assert :ok = SystemMetrics.allocator_fragmentation_measurement() results = :ets.tab2list(events) assert results != [] + # One event per distinct allocator *type*, not per scheduler instance. + assert length(results) == length(Enum.uniq_by(results, fn {allocator, _} -> allocator end)) + for {allocator, unused} <- results do assert is_binary(allocator) assert is_integer(unused) assert unused >= 0 end end - - test "gating only fires every Nth tick" do - ref = make_ref() - handler_id = {__MODULE__, ref} - - counter = :counters.new(1, []) - - :telemetry.attach( - handler_id, - [:vm, :alloc, :fragmentation], - fn _event, _measurements, _meta, _ -> :counters.add(counter, 1, 1) end, - nil - ) - - on_exit(fn -> :telemetry.detach(handler_id) end) - - # Use an isolated gate key so we don't interfere with the real poller's counter. - gate_key = {:test_gate, ref} - every = SystemMetrics.allocator_fragmentation_interval_ticks() - - # Run exactly `every` ticks; exactly one of them should be due. - for _ <- 1..every do - SystemMetrics.allocator_fragmentation_measurement(%{}, gate_key: gate_key) - end - - # Exactly one tick in `every` is due; on that tick we emit one event per - # distinct allocator *type*. - fired = :counters.get(counter, 1) - - type_count = - :recon_alloc.fragmentation(:current) - |> Enum.map(fn {{type, _instance}, _info} -> type end) - |> Enum.uniq() - |> length() - - assert fired == type_count - end end describe "metric definitions" do From dbd1b8535f7f25511375bb0414ba1974315e60cd Mon Sep 17 00:00:00 2001 From: Oleksii Sholik Date: Tue, 7 Jul 2026 12:52:41 +0200 Subject: [PATCH 13/13] Cut per-tick parsing and path-building in the procfs readers Two once-ever answers were being recomputed on every ~5s poll tick: - read_kv_file/2 now takes a set of wanted keys and skips value parsing for every other line. /proc//status is ~60 lines of which we consume 4 and cgroup memory.stat is ~40 of which we consume 3, so the bulk of the per-tick Integer.parse/Map.put work disappears. - The readers' file paths are constant for the life of the VM. Cgroup precomputes the default-root paths at compile time (tests passing :cgroup_root still build them dynamically); Proc resolves the BEAM pid and joins its two paths once, cached via SystemMetrics.memoized/2 (made public for this). Co-Authored-By: Claude Fable 5 --- .../lib/electric/telemetry/system_metrics.ex | 9 ++-- .../telemetry/system_metrics/cgroup.ex | 50 +++++++++++++------ .../electric/telemetry/system_metrics/proc.ex | 33 ++++++++++-- .../telemetry/system_metrics/procfs_parse.ex | 15 +++--- 4 files changed, 77 insertions(+), 30 deletions(-) diff --git a/packages/electric-telemetry/lib/electric/telemetry/system_metrics.ex b/packages/electric-telemetry/lib/electric/telemetry/system_metrics.ex index 4ab59e1903..7489a4bfe9 100644 --- a/packages/electric-telemetry/lib/electric/telemetry/system_metrics.ex +++ b/packages/electric-telemetry/lib/electric/telemetry/system_metrics.ex @@ -103,9 +103,12 @@ defmodule ElectricTelemetry.SystemMetrics do :ok end - # Get-or-compute a `:persistent_term` entry. The put happens at most once per - # key (on first call), never on the per-tick hot path. - defp memoized(key, fun) do + @doc """ + Get-or-compute a `:persistent_term` entry. The put happens at most once per + key (on first call), never on a per-tick hot path. + """ + @spec memoized(term(), (-> value)) :: value when value: term() + def memoized(key, fun) do with :undefined <- :persistent_term.get(key, :undefined) do tap(fun.(), &:persistent_term.put(key, &1)) end diff --git a/packages/electric-telemetry/lib/electric/telemetry/system_metrics/cgroup.ex b/packages/electric-telemetry/lib/electric/telemetry/system_metrics/cgroup.ex index cb165be567..5e8d6e6813 100644 --- a/packages/electric-telemetry/lib/electric/telemetry/system_metrics/cgroup.ex +++ b/packages/electric-telemetry/lib/electric/telemetry/system_metrics/cgroup.ex @@ -18,10 +18,24 @@ defmodule ElectricTelemetry.SystemMetrics.Cgroup do import ElectricTelemetry.SystemMetrics, only: [emit: 2] import ElectricTelemetry.SystemMetrics.ProcfsParse, - only: [read_kv_file: 1, read_int_file: 1, read_raw_file: 1, parse_int: 1, parse_float: 1] + only: [read_kv_file: 2, read_int_file: 1, read_raw_file: 1, parse_int: 1, parse_float: 1] @default_root "/sys/fs/cgroup" + @files %{ + memory_current: "memory.current", + memory_stat: "memory.stat", + memory_max: "memory.max", + memory_pressure: "memory.pressure", + cpu_stat: "cpu.stat", + cpu_pressure: "cpu.pressure", + io_stat: "io.stat" + } + @default_paths Map.new(@files, fn {key, file} -> {key, Path.join(@default_root, file)} end) + + @memory_stat_keys MapSet.new(~w(anon file inactive_file)) + @cpu_stat_keys MapSet.new(~w(usage_usec nr_throttled throttled_usec)) + @doc """ Read the cgroup accounting files and emit `cgroup.*` telemetry events. @@ -38,18 +52,23 @@ defmodule ElectricTelemetry.SystemMetrics.Cgroup do Keyword.get_lazy(opts, :cgroup_version, fn -> SystemMetrics.system_info().cgroup_version end) if version == :v2 do - root = Keyword.get(opts, :cgroup_root, @default_root) - emit_memory(root) - emit_cpu(root) - emit([:cgroup, :io], io_totals(read_raw_file(Path.join(root, "io.stat")))) + paths = opts |> Keyword.get(:cgroup_root, @default_root) |> paths() + emit_memory(paths) + emit_cpu(paths) + emit([:cgroup, :io], io_totals(read_raw_file(paths.io_stat))) end :ok end - defp emit_memory(root) do - current = read_int_file(Path.join(root, "memory.current")) - stat = read_kv_file(Path.join(root, "memory.stat")) + # The production file paths are compile-time constants; only tests pass a + # different root. + defp paths(@default_root), do: @default_paths + defp paths(root), do: Map.new(@files, fn {key, file} -> {key, Path.join(root, file)} end) + + defp emit_memory(paths) do + current = read_int_file(paths.memory_current) + stat = read_kv_file(paths.memory_stat, @memory_stat_keys) inactive_file = stat["inactive_file"] emit([:cgroup, :memory], %{ @@ -59,14 +78,14 @@ defmodule ElectricTelemetry.SystemMetrics.Cgroup do working_set: current && inactive_file && max(current - inactive_file, 0), # An unlimited memory.max is the literal "max", which parses to nil and is # skipped (the host-RAM ceiling comes from the /proc reader). - max: read_int_file(Path.join(root, "memory.max")) + max: read_int_file(paths.memory_max) }) - emit_pressure(root, "memory.pressure", :memory) + emit_pressure(paths.memory_pressure, :memory) end - defp emit_cpu(root) do - stat = read_kv_file(Path.join(root, "cpu.stat")) + defp emit_cpu(paths) do + stat = read_kv_file(paths.cpu_stat, @cpu_stat_keys) emit([:cgroup, :cpu], %{ usage_usec: stat["usage_usec"], @@ -74,14 +93,13 @@ defmodule ElectricTelemetry.SystemMetrics.Cgroup do throttled_usec: stat["throttled_usec"] }) - emit_pressure(root, "cpu.pressure", :cpu) + emit_pressure(paths.cpu_pressure, :cpu) end # PSI files may be absent even on v2 (kernel built without PSI); a nil avg10 # is dropped by emit/2, skipping the event. - defp emit_pressure(root, file, plane) do - avg10 = psi_full_avg10(read_raw_file(Path.join(root, file))) - emit([:cgroup, plane, :pressure, :full], %{avg10: avg10}) + defp emit_pressure(path, plane) do + emit([:cgroup, plane, :pressure, :full], %{avg10: psi_full_avg10(read_raw_file(path))}) end # io.stat is one line per device: "MAJ:MIN rbytes=… wbytes=… rios=… …". diff --git a/packages/electric-telemetry/lib/electric/telemetry/system_metrics/proc.ex b/packages/electric-telemetry/lib/electric/telemetry/system_metrics/proc.ex index ac9f1babd5..727e709b82 100644 --- a/packages/electric-telemetry/lib/electric/telemetry/system_metrics/proc.ex +++ b/packages/electric-telemetry/lib/electric/telemetry/system_metrics/proc.ex @@ -19,10 +19,13 @@ defmodule ElectricTelemetry.SystemMetrics.Proc do alias ElectricTelemetry.SystemMetrics import ElectricTelemetry.SystemMetrics, only: [emit: 2] - import ElectricTelemetry.SystemMetrics.ProcfsParse, only: [read_kv_file: 1] + import ElectricTelemetry.SystemMetrics.ProcfsParse, only: [read_kv_file: 2] @default_root "/proc" + @status_keys MapSet.new(~w(RssAnon RssFile RssShmem VmRSS)) + @io_keys MapSet.new(~w(read_bytes write_bytes)) + @doc """ Read the BEAM's `/proc/` accounting files and emit `host.proc.beam.*` telemetry events. @@ -41,10 +44,9 @@ defmodule ElectricTelemetry.SystemMetrics.Proc do os = Keyword.get_lazy(opts, :os, fn -> SystemMetrics.system_info().os end) if os == {:unix, :linux} do - root = Keyword.get(opts, :proc_root, @default_root) - pid = opts |> Keyword.get_lazy(:pid, fn -> :os.getpid() end) |> to_string() + %{status: status_path, io: io_path} = paths(opts) - status = read_kv_file(Path.join([root, pid, "status"])) + status = read_kv_file(status_path, @status_keys) emit([:host, :proc, :beam], %{ rss_anon: kb_to_bytes(status["RssAnon"]), @@ -53,7 +55,7 @@ defmodule ElectricTelemetry.SystemMetrics.Proc do vm_rss: kb_to_bytes(status["VmRSS"]) }) - io = read_kv_file(Path.join([root, pid, "io"])) + io = read_kv_file(io_path, @io_keys) emit([:host, :proc, :beam, :io], %{ read_bytes: io["read_bytes"], @@ -64,6 +66,27 @@ defmodule ElectricTelemetry.SystemMetrics.Proc do :ok end + # In production (no opts) the BEAM pid and the file paths never change for + # the life of the VM, so resolve them once and cache in :persistent_term; + # test overrides recompute. + defp paths([]) do + SystemMetrics.memoized({__MODULE__, :paths}, fn -> + build_paths(@default_root, :os.getpid()) + end) + end + + defp paths(opts) do + build_paths( + Keyword.get(opts, :proc_root, @default_root), + Keyword.get_lazy(opts, :pid, fn -> :os.getpid() end) + ) + end + + defp build_paths(root, pid) do + dir = Path.join(root, to_string(pid)) + %{status: Path.join(dir, "status"), io: Path.join(dir, "io")} + end + defp kb_to_bytes(nil), do: nil defp kb_to_bytes(kb), do: kb * 1024 end diff --git a/packages/electric-telemetry/lib/electric/telemetry/system_metrics/procfs_parse.ex b/packages/electric-telemetry/lib/electric/telemetry/system_metrics/procfs_parse.ex index a3e3e54397..c03542b256 100644 --- a/packages/electric-telemetry/lib/electric/telemetry/system_metrics/procfs_parse.ex +++ b/packages/electric-telemetry/lib/electric/telemetry/system_metrics/procfs_parse.ex @@ -13,9 +13,10 @@ defmodule ElectricTelemetry.SystemMetrics.ProcfsParse do """ @doc """ - Parse a flat key/value file — `/proc/meminfo`, `/proc//status`, - `/proc//io`, cgroup `memory.stat`/`cpu.stat` — into a map of - `binary key => integer value`. + Parse a flat key/value file — `/proc//status`, `/proc//io`, cgroup + `memory.stat`/`cpu.stat` — into a map of `binary key => integer value`, + keeping only the keys in the `keys` set. These files carry dozens of lines of + which a caller consumes a handful; non-wanted lines skip value parsing. Handles both the `key value` and `Key:\twhitespace-padded value [unit]` line layouts: the key is the first whitespace-delimited token with any trailing @@ -23,13 +24,15 @@ defmodule ElectricTelemetry.SystemMetrics.ProcfsParse do `kB`) is ignored. Lines whose value doesn't parse as an integer are dropped. Values keep their native unit (the caller converts, e.g. kB -> bytes). """ - @spec read_kv_file(Path.t()) :: %{optional(binary()) => integer()} - def read_kv_file(path) do + @spec read_kv_file(Path.t(), MapSet.t(binary())) :: %{optional(binary()) => integer()} + def read_kv_file(path, keys) do for line <- String.split(read_raw_file(path) || "", "\n", trim: true), [key, value | _] <- [String.split(line)], + key = String.trim_trailing(key, ":"), + MapSet.member?(keys, key), int = parse_int(value), into: %{}, - do: {String.trim_trailing(key, ":"), int} + do: {key, int} end @doc """