Skip to content

Record batch machine command timings#1236

Open
karenychen wants to merge 4 commits into
Azure:mainfrom
karenychen:codex/batch-command-execution-times
Open

Record batch machine command timings#1236
karenychen wants to merge 4 commits into
Azure:mainfrom
karenychen:codex/batch-command-execution-times

Conversation

@karenychen

Copy link
Copy Markdown
Contributor

Summary

  • record per-batch BatchPutMachine command timings in AKSMachineClient
  • emit batch_command_execution_times as scale_machine operation metadata
  • cover metric capture and metadata emission in machine-client tests

Tests

  • PYTHONPATH=modules/python python3 -m pytest modules/python/tests/test_aks_machine_client.py modules/python/tests/test_machine_crud.py modules/python/tests/test_crud_main.py
  • git diff --check

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR extends the AKS Machine scale-out path to capture per-batch BatchPutMachine timing metrics and emit them as operation metadata, with accompanying test updates to validate metric capture and metadata emission.

Changes:

  • Record per-batch start/end timestamps and execution duration in _create_batch_machines.
  • Emit batch_command_execution_times as scale_machine operation metadata when use_batch_api=True.
  • Update machine-client tests to cover timing metric capture and metadata emission.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
modules/python/clients/aks_machine_client.py Captures per-batch timing metrics and emits them as scale_machine operation metadata.
modules/python/tests/test_aks_machine_client.py Extends tests to assert timing metrics are captured and metadata is emitted for the batch path.
Comments suppressed due to low confidence (1)

modules/python/clients/aks_machine_client.py:921

  • execution_time_seconds is derived from datetime.now() deltas, which can go negative or be skewed if the system clock adjusts (e.g., NTP). For durations, use a monotonic timer (time.perf_counter() / time.monotonic()) and keep datetime only for the wall-clock timestamps.
        start_time = datetime.now(timezone.utc)
        self._make_batch_request(
            "PUT",
            url,
            body,

Comment on lines +935 to +940
metric = {
"start_time": start_time.isoformat().replace("+00:00", "Z"),
"end_time": end_time.isoformat().replace("+00:00", "Z"),
"execution_time_seconds": execution_time_seconds,
"total_machines_in_batch": len(chunk),
}
@karenychen

Copy link
Copy Markdown
Contributor Author

Code review

No issues found. Checked for bugs and AGENTS.md compliance.

@xuexu6666

Copy link
Copy Markdown
Contributor

Code review

No issues found. Checked for bugs and CLAUDE.md compliance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants