From e20d3d1394a2043ea66750ac5be4a5cebce324a0 Mon Sep 17 00:00:00 2001 From: rob Date: Sun, 28 Jun 2026 09:03:28 +0100 Subject: [PATCH 1/3] fix(electric-telemetry): bound process_type cardinality for proxy-supplied request ids parse_binary_label/1 only stripped the request id from process labels when it was exactly 20 bytes (the length Plug.RequestId self-generates). A proxy- or load-balancer-supplied x-request-id (UUID, Envoy/nginx trace id, etc.) has a different length, so the raw per-request id leaked into process_type, adding a permanent new series to telemetry events like vm.monitor.long_gc and vm.monitor.long_schedule for every distinct id. Keep the fast fixed-width match for the common 20-byte case and add a slow path that splits on the " - " delimiter regardless of request-id length, collapsing all requests to a route back to a single process_type. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../process-type-request-id-cardinality.md | 5 ++ .../lib/electric/telemetry/processes.ex | 47 +++++++++++++------ .../electric/telemetry/processes_test.exs | 21 +++++++++ 3 files changed, 58 insertions(+), 15 deletions(-) create mode 100644 .changeset/process-type-request-id-cardinality.md diff --git a/.changeset/process-type-request-id-cardinality.md b/.changeset/process-type-request-id-cardinality.md new file mode 100644 index 0000000000..0df93f8774 --- /dev/null +++ b/.changeset/process-type-request-id-cardinality.md @@ -0,0 +1,5 @@ +--- +"@core/electric-telemetry": patch +--- + +Fix unbounded `process_type` cardinality on request-handling processes. `parse_binary_label/1` only stripped the request id from process labels when it was exactly 20 bytes (the length `Plug.RequestId` self-generates). A proxy- or load-balancer-supplied `x-request-id` (UUID, Envoy/nginx trace id, etc.) has a different length, so the raw per-request id leaked into `process_type`, adding a permanent new series to telemetry events like `vm.monitor.long_gc` and `vm.monitor.long_schedule` for every distinct id. The label is now parsed by splitting on the `" - "` delimiter regardless of request-id length, collapsing all requests to a route back to a single `process_type` (e.g. `GET /v1/shape`). diff --git a/packages/electric-telemetry/lib/electric/telemetry/processes.ex b/packages/electric-telemetry/lib/electric/telemetry/processes.ex index 457f6b55c6..ab1e2fb213 100644 --- a/packages/electric-telemetry/lib/electric/telemetry/processes.ex +++ b/packages/electric-telemetry/lib/electric/telemetry/processes.ex @@ -262,26 +262,43 @@ defmodule ElectricTelemetry.Processes do defp parse_binary_label(label) do case label do "Request " <> <<_req_id::binary-20, " - ", rest::binary>> -> + # Fast path: the request id is exactly 20 bytes, as generated by Plug.RequestId. # This is a request process with the label assigned by Electric.Plug.LabelProcessPlug. # We group all requests together to be able to see their aggregated mem footprint. + parse_request_label(rest) + + "Request " <> rest -> + # Slow path: the request id has a non-default length (e.g. a UUID or a proxy-supplied + # x-request-id), so the fixed-width match above failed. Split on the " - " delimiter + # instead so all requests to a route collapse to a single label regardless of id length. + case :binary.split(rest, " - ") do + [_req_id, request] -> parse_request_label(request) + _ -> truncate_label(label) + end - # Cut off the URL query part, leaving only the ` ` prefix. - part_len = - case :binary.match(rest, "?") do - {pos, _} -> - pos + _ -> + truncate_label(label) + end + end - :nomatch -> - # No query string means only path is present. It is either a request to the health endpoint or something unexpected. - byte_size(rest) - end + defp parse_request_label(request) do + # Cut off the URL query part, leaving only the ` ` prefix. + part_len = + case :binary.match(request, "?") do + {pos, _} -> + pos - :binary.part(rest, 0, part_len) + :nomatch -> + # No query string means only path is present. It is either a request to the health endpoint or something unexpected. + byte_size(request) + end - _ -> - # Opportunistic grouping by truncating long labels. - part_len = min(20, byte_size(label)) - :binary.part(label, 0, part_len) - end + :binary.part(request, 0, part_len) + end + + defp truncate_label(label) do + # Opportunistic grouping by truncating long labels. + part_len = min(20, byte_size(label)) + :binary.part(label, 0, part_len) end end diff --git a/packages/electric-telemetry/test/electric/telemetry/processes_test.exs b/packages/electric-telemetry/test/electric/telemetry/processes_test.exs index 559b2617e5..bbf8db4c61 100644 --- a/packages/electric-telemetry/test/electric/telemetry/processes_test.exs +++ b/packages/electric-telemetry/test/electric/telemetry/processes_test.exs @@ -16,6 +16,27 @@ defmodule ElectricTelemetry.ProcessesTest do assert "GET /v1/health" = proc_type(pid) end + test "groups request labels with a non-default-length request id (e.g. a proxy-supplied UUID)" do + pid = + spawn_with_label( + "Request 123e4567-e89b-12d3-a456-426614174000 - GET /v1/shape?table=users&offset=-1" + ) + + assert "GET /v1/shape" = proc_type(pid) + end + + test "groups request labels with a shorter-than-default request id" do + pid = spawn_with_label("Request abc123 - GET /v1/health") + + assert "GET /v1/health" = proc_type(pid) + end + + test "request label with no \" - \" delimiter is truncated" do + pid = spawn_with_label("Request something-without-delimiter") + + assert "Request something-wi" = proc_type(pid) + end + test "non-request binary labels are truncated to 20 chars" do pid = spawn_with_label("some_long_label_that_exceeds_twenty_characters") From 592c3aa6d710b6a87d07a10425cf902dea2fbd8e Mon Sep 17 00:00:00 2001 From: rob Date: Tue, 30 Jun 2026 11:25:56 +0100 Subject: [PATCH 2/3] test(sync-service): fix reversed wait_until/2 args in shape_cache_test The `start_consumer_for_handle/2 starts a consumer plus dependencies` test called `wait_until(1000, fn -> ... end)`, but #4635 changed the signature to `wait_until(fun, timeout_ms \\ 500)` (function first). The new call site was added by #4651 using the old arg order; the two merged cleanly in git but not in meaning, so `1000` was bound to `fun`, failed the `is_function(fun, 0)` guard, and raised FunctionClauseError whenever the slow suite ran on main. Swap the arguments to match the current signature. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../sync-service/test/electric/shape_cache_test.exs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/packages/sync-service/test/electric/shape_cache_test.exs b/packages/sync-service/test/electric/shape_cache_test.exs index 11d17d1622..38701c1bd5 100644 --- a/packages/sync-service/test/electric/shape_cache_test.exs +++ b/packages/sync-service/test/electric/shape_cache_test.exs @@ -1579,10 +1579,13 @@ defmodule Electric.ShapeCacheTest do # explicit `start_consumer_for_handle/2` call is required. The # continue runs asynchronously after `start_supervised!` returns, # so we wait for the registry to populate. - assert wait_until(1000, fn -> - not is_nil(Electric.Shapes.ConsumerRegistry.whereis(stack_id, shape_handle)) and - not is_nil(Electric.Shapes.ConsumerRegistry.whereis(stack_id, dep_handle)) - end) + assert wait_until( + fn -> + not is_nil(Electric.Shapes.ConsumerRegistry.whereis(stack_id, shape_handle)) and + not is_nil(Electric.Shapes.ConsumerRegistry.whereis(stack_id, dep_handle)) + end, + 1000 + ) # Materializer should be started assert Process.alive?( From 767b1170b274cc2f4275a4ba79378fa951057bb4 Mon Sep 17 00:00:00 2001 From: Rob A'Court Date: Tue, 30 Jun 2026 10:57:16 +0100 Subject: [PATCH 3/3] Update changeset text --- .changeset/process-type-request-id-cardinality.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/process-type-request-id-cardinality.md b/.changeset/process-type-request-id-cardinality.md index 0df93f8774..b0f8c280cb 100644 --- a/.changeset/process-type-request-id-cardinality.md +++ b/.changeset/process-type-request-id-cardinality.md @@ -2,4 +2,4 @@ "@core/electric-telemetry": patch --- -Fix unbounded `process_type` cardinality on request-handling processes. `parse_binary_label/1` only stripped the request id from process labels when it was exactly 20 bytes (the length `Plug.RequestId` self-generates). A proxy- or load-balancer-supplied `x-request-id` (UUID, Envoy/nginx trace id, etc.) has a different length, so the raw per-request id leaked into `process_type`, adding a permanent new series to telemetry events like `vm.monitor.long_gc` and `vm.monitor.long_schedule` for every distinct id. The label is now parsed by splitting on the `" - "` delimiter regardless of request-id length, collapsing all requests to a route back to a single `process_type` (e.g. `GET /v1/shape`). +Fix `process_type` parsing for request processes that have a request id that is not the standard 20 bytes.