Support measure_to

[pojiro]

No description provided.

[Copilot]

Pull request overview

This PR adds support for the measure_to option to track timestamp measurements across distributed operations in the Giocci platform. When provided with a PID, the client will receive measurement messages containing timestamps for each hop in the request flow (client -> relay -> engine and back).

Changes:

• Added measure_to option to register_client, save_module, and exec_func functions
• Modified term structure to include %{data: payload, measurements: map} format for all inter-component communication
• Updated test infrastructure to support passing arguments to docker-based tests
• Added comprehensive integration test for the measurement feature

Reviewed changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
apps/giocci/lib/giocci.ex	Added measure_to option documentation and updated typespec
apps/giocci/lib/giocci/worker.ex	Implemented measurement extraction and message sending
apps/giocci/lib/giocci/utils.ex	Added send/receive timestamp tracking to zenohex_get and zenohex_put
apps/giocci_relay/lib/giocci_relay/utils.ex	Added measurement tracking for relay operations
apps/giocci_relay/lib/giocci_relay/module_saver.ex	Updated to handle measurement structure
apps/giocci_relay/lib/giocci_relay/engine_registrar.ex	Updated to handle measurement structure
apps/giocci_relay/lib/giocci_relay/engine_inquiry_handler.ex	Added timestamp tracking
apps/giocci_relay/lib/giocci_relay/client_registrar.ex	Added timestamp tracking
apps/giocci_engine/lib/giocci_engine/utils.ex	Added zenohex_reply helper
apps/giocci_engine/lib/giocci_engine/module_saver.ex	Added timestamp tracking
apps/giocci_engine/lib/giocci_engine/exec_func_handler.ex	Added timestamp tracking
apps/giocci_engine/lib/giocci_engine/exec_func_async_handler.ex	Updated to use measurement structure
apps/giocci_engine/lib/giocci_engine/engine_registrar.ex	Adopted measurement structure for registration
apps/giocci_integration_test/test/giocci_integration_test_test.exs	Added comprehensive test for measure_to feature
apps/giocci_integration_test/mix.exs	Updated test infrastructure to support argument passing
.github/copilot-instructions.md	Added project guidelines for GitHub Copilot

Comments suppressed due to low confidence (6)

apps/giocci/lib/giocci.ex:13

• The typespec says register_client can return {:ok, map()}, but the implementation always returns :ok on success (not {:ok, map()}). The maybe_send_measurements function extracts recv_term.data which is :ok, not a map. The typespec should be @spec register_client(String.t(), keyword()) :: :ok | {:error, reason :: term()} (the original spec).

  @spec register_client(String.t(), keyword()) :: :ok | {:ok, map()} | {:error, reason :: term()}

apps/giocci/lib/giocci/utils.ex:10

• The pattern match {:ok, recv_term} = decode(payload) will raise a MatchError if decode returns an error, but the rescue clause only catches ArgumentError. If the payload is invalid and decode returns {:error, "decode_failed: ..."}, this will crash with an unhandled MatchError instead of returning a proper error tuple. Consider using a case statement or with expression to handle decode errors gracefully.

        {:ok, recv_term} = decode(payload)
        {:ok, add_recv_measurements(key, recv_term)}

apps/giocci_relay/lib/giocci_relay/utils.ex:10

• The pattern match {:ok, recv_term} = decode(payload) will raise a MatchError if decode returns an error. Since there's no rescue clause in this function, an invalid payload will cause an unhandled MatchError instead of returning a proper error tuple. Consider using a case statement or with expression to handle decode errors gracefully.

        {:ok, recv_term} = decode(payload)
        {:ok, add_recv_measurements(key, recv_term)}

apps/giocci_relay/lib/giocci_relay/utils.ex:78

• The function pattern add_send_measurements(key, %{measurements: measurements} = send_term) will raise a FunctionClauseError if send_term doesn't have a measurements field or isn't a map. While all current callers create terms with measurements, this could cause hard-to-debug crashes if used incorrectly. Consider adding a more descriptive error message or guard clause to make debugging easier if this function is called with an invalid term structure.

  defp add_send_measurements(key, %{measurements: measurements} = send_term) do
    key =
      cond do
        String.contains?(key, "/save_module/relay/") -> :relay_send_timestamp_to_engine
        true -> raise "Unexpected condition reached"
      end

    measurements = Map.put(measurements, key, System.system_time(:millisecond))
    %{send_term | measurements: measurements}
  end

apps/giocci_engine/lib/giocci_engine/engine_registrar.ex:34

• The engine is sending a term with the measurement structure (%{data: ..., measurements: ...}) by manually encoding it and passing the binary through Utils.zenohex_get. This works but is inconsistent with how the client and relay handle similar operations. The client and relay have updated their Utils.zenohex_get functions to automatically handle encoding, adding send/receive timestamps, and decoding. Consider updating the engine's Utils.zenohex_get to follow the same pattern, or if measurements aren't needed for engine registration, simplify by not using the measurement structure here.

    term_to_relay = %{data: %{engine_name: engine_name}, measurements: %{}}

    # Register this Engine to the specified Relay when starts
    :ok =
      with key <- Path.join(key_prefix, "giocci/register/engine/#{relay_name}"),
           {:ok, binary} <- Utils.encode(term_to_relay),
           {:ok, binary} <- Utils.zenohex_get(session_id, key, _timeout = 5000, binary),
           {:ok, recv_term} <- Utils.decode(binary) do
        recv_term
      end

apps/giocci_integration_test/mix.exs:55

• The change from System.find_executable("docker") to not is_nil(System.find_executable("docker")) is redundant. In Elixir, cond uses truthy evaluation where nil and false are falsy, and everything else is truthy. System.find_executable/1 returns either a string (truthy) or nil (falsy), so the explicit not is_nil() check is unnecessary. The original code System.find_executable("docker") -> was already correct and more idiomatic.

      not is_nil(System.find_executable("docker")) ->

---

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