Skip to content

[BUG] One container's stats timeout discards the whole resource-sampling round of its instance #270

Open

Description

@LarsLaskowski

Summary

Resource statistics are sampled per running container, one stats request at a time. A container whose stats request answers non-success is simply left out of the round, but a container whose stats request times out (or breaks with a transport/JSON error) aborts the whole collection for the instance: no container sample and no instance sample is recorded for that round. A single hung container (a frequent case for a container under heavy load or in a broken state) thereby blanks the resource charts of every container on that host. A per-container timeout must only drop the affected container.

Area

  • docs/areas/docker-instances.md (Resource statistics, Limits and errors)
  • Security area: None

Current behavior

src/DockerUpdateGuard/Docker/DockerInstanceClient.cs:1052-1135 (CollectContainerResourceUsageAsync): the container list and all per-container stats requests run inside one try:

using var statsTimeoutSource = CreateRequestTimeoutCancellationTokenSource(instanceOptions.RequestTimeoutSeconds, cancellationToken);
using var statsResponse = await httpClient.GetAsync($"v1.41/containers/{...}/stats?stream=false", statsTimeoutSource.Token)...;

if (statsResponse.IsSuccessStatusCode == false)
{
    continue;                       // non-success: only this container is dropped (line 1098-1101)
}
...
catch (OperationCanceledException)  // per-container timeout lands here (line 1127-1130)
{
    return ...Failed($"Docker container resource sampling timed out for '{instanceOptions.Name}' after {instanceOptions.RequestTimeoutSeconds} seconds");
}
catch (Exception exception)          // transport error or malformed stats JSON (line 1131-1134)
{
    return ...Failed($"Docker container resource sampling failed for '{instanceOptions.Name}': {exception.Message}");
}

src/DockerUpdateGuard/Images/ResourceStatisticsCollector.cs:122-135 then logs a warning and returns without writing anything for the instance — neither the samples already collected for the other containers nor the instance sample.

Failure scenario: an instance runs 20 containers; container #7 does not answer its stats request within RequestTimeoutSeconds (default 15). The 6 samples already read and the 13 remaining containers are discarded, and the instance chart gets a gap. If container #7 stays hung, the instance never gets a sample again.

Expected behavior

Decision (product manager): a timeout drops only the affected container.

  • A stats request of one container that times out, fails at the transport level, or returns a body that cannot be parsed leaves only that container out of the round, exactly like a non-success answer today. Sampling continues with the next container.
  • Each dropped container is logged (warning, with instance name, container name/ID and the reason); the log level and wording must not flood the log for a permanently hung container more than once per round.
  • The instance sample is recorded from the containers that were sampled.
  • Edge case: when the container list contained at least one running container but no container could be sampled, the round is treated as failed for the instance (no instance sample, warning as today). Otherwise an engine that answers the list but hangs on all stats calls would record misleading samples with container count 0, CPU 0 and memory 0.
  • Unchanged: a failed or timed-out container list request still fails the round for that instance; cancellation of the calling job (shutdown) still stops the operation and is not reported as a failure; other instances are never affected.

Acceptance criteria

  • With three running containers where the second one's stats request exceeds the timeout, samples for the first and third container and one instance sample (count 2) are recorded.
  • Same for a stats request that throws HttpRequestException and for a stats body that is not valid JSON.
  • With all stats requests timing out, no samples are recorded for the instance and a warning is logged.
  • A cancelled job token still propagates OperationCanceledException (no sample, no failure log).
  • A failing container list still records no samples for the instance.
  • Unit tests (MSTest, naming per docs/UNIT_TESTS.md), e.g. DockerInstanceClientCollectContainerResourceUsageWithOneStatsTimeoutSkipsOnlyThatContainer, …WithAllStatsTimeoutsReturnsFailed, …WhenCallerCancelsThrowsOperationCanceled; and for the collector (no ResourceStatisticsCollectorTests exist yet — add the class) a test that a partial result writes container and instance samples. Timeouts are simulated through a stub HttpMessageHandler/cancellation, never by real waiting.
  • The area document docs/areas/docker-instances.md describes the new behavior (Resource statistics and Limits and errors) and its Known gaps entry for this issue (currently Open questions item 4) is removed.

Implementation notes

  • Move the per-container stats call into its own try/catch inside the loop (or a private helper returning RuntimeContainerResourceDescriptor? plus a reason). Distinguish the caller's cancellation (cancellationToken.IsCancellationRequested → rethrow) from the per-request timeout (statsTimeoutSource cancelled → skip container).
  • To implement the "nothing could be sampled" rule, the client has to report how many running containers were listed versus sampled (e.g. return failed when listed > 0 and sampled == 0, or extend the result data). Recommendation: keep it inside DockerInstanceClient and return failed with a message such as "no container stats could be read for ''", so ResourceStatisticsCollector needs no change for that case.
  • Consider the total duration: N hung containers × RequestTimeoutSeconds can now make one round long (before, the first timeout ended it). Collection rounds do not overlap (docs/areas/scan-engine.md), so this delays the next round but is acceptable; mention it in the area document.
  • ResourceStatisticsCollector logs with _logger.LogWarning directly (ResourceStatisticsCollector.cs:129); new log entries should follow the source-generated logging pattern used elsewhere (DockerInstanceClientLogging.cs).
  • No configuration, README or migration impact. Must not change: integration clients return outcomes instead of throwing (.squad/project.md, Guarantees).

Related


Generated by Claude Code

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions