diff --git a/.changeset/process-type-request-id-cardinality.md b/.changeset/process-type-request-id-cardinality.md new file mode 100644 index 0000000000..b0f8c280cb --- /dev/null +++ b/.changeset/process-type-request-id-cardinality.md @@ -0,0 +1,5 @@ +--- +"@core/electric-telemetry": patch +--- + +Fix `process_type` parsing for request processes that have a request id that is not the standard 20 bytes. 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") 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?(