Skip to content

feat(logs): real GetLogGroupFields, ListLogGroupsForQuery, StartLiveTail#761

Merged
vieiralucas merged 2 commits intomainfrom
worktree-batch5-logs-gap-fill
Apr 25, 2026
Merged

feat(logs): real GetLogGroupFields, ListLogGroupsForQuery, StartLiveTail#761
vieiralucas merged 2 commits intomainfrom
worktree-batch5-logs-gap-fill

Conversation

@vieiralucas
Copy link
Copy Markdown
Member

@vieiralucas vieiralucas commented Apr 25, 2026

Summary

  • GetLogGroupFields: walks every event in every stream of the target log group, parses each as JSON, tallies how often each top-level key appears. Synthetic @timestamp / @message / @logStream always surfaced; response uses the documented name + percent shape (not the prior fieldName)
  • StartQuery: accepts the AWS-documented logGroupNames and logGroupIdentifiers arrays in addition to the single logGroupName. QueryInfo captures every group/identifier referenced
  • ListLogGroupsForQuery: looks up the captured identifiers by queryId and returns them instead of an empty array
  • StartLiveTail: echoes the requested logGroupIdentifiers back in sessionStart

Out of scope (documented limitations)

  • GetLogObject, GetLogFields, GetLogRecord and anomaly analysis still require Logs Insights query result generation to be wired through the event store; deferred
  • Array-style metric filter patterns ([w1, w2 = "ERROR", w3]) — separate parser change

Test plan

  • Unit: get_log_group_fields_returns_synthetic_fields_when_no_events, get_log_group_fields_extracts_top_level_keys_from_json_events, list_log_groups_for_query_returns_started_groups, list_log_groups_for_query_returns_array_form_groups
  • All 256 existing fakecloud-logs unit tests still green
  • cargo clippy --workspace --all-targets -- -D warnings clean
  • cargo fmt --check clean

Summary by cubic

Replace stubbed CloudWatch Logs APIs with real behavior and fix multi‑group query handling. GetLogGroupFields, ListLogGroupsForQuery, StartLiveTail, and GetQueryResults now read recorded events and validate inputs.

  • New Features

    • GetLogGroupFields: counts top-level JSON keys across events; always includes @timestamp, @message, @logStream; returns name + percent.
    • StartQuery: accepts logGroupNames and logGroupIdentifiers; queries scan all referenced groups (ARNs resolved, duplicates deduped).
    • ListLogGroupsForQuery: returns captured identifiers for the queryId; unknown queryId now returns ResourceNotFoundException.
    • StartLiveTail: validates logGroupIdentifiers is an array of strings and echoes them in sessionStart.
  • Migration

    • GetLogGroupFields response uses name instead of fieldName. Update consumers accordingly.

Written for commit ad110ec. Summary will update on new commits.

Three CloudWatch Logs operations were returning hardcoded stubs even
though the data they need is already in fakecloud's state. They now
return real values derived from recorded events / queries.

- GetLogGroupFields walks every event in every stream of the target
  log group, parses each as JSON, and tallies how often each top-level
  key appears. Always-present fields (@timestamp, @message, @logstream)
  are still surfaced for empty groups. The response shape uses AWS's
  documented `name` / `percent` keys (not the prior `fieldName`).
- StartQuery now accepts the AWS-documented `logGroupNames` and
  `logGroupIdentifiers` arrays in addition to the single `logGroupName`.
  The QueryInfo state captures every group/identifier the query
  referenced.
- ListLogGroupsForQuery looks up the QueryInfo by `queryId` and returns
  the captured identifiers instead of an empty array.
- StartLiveTail echoes the requested `logGroupIdentifiers` back in
  `sessionStart` instead of an empty list.

The stubs in misc.rs for `get_log_object`, `get_log_fields`, and
`get_log_record` (plus anomaly analysis) require Logs Insights query
result generation to be wired through the event store and remain
deferred — they are documented as known limitations elsewhere.
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 25, 2026

Codecov Report

❌ Patch coverage is 91.66667% with 15 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
crates/fakecloud-logs/src/service/queries.rs 90.90% 9 Missing ⚠️
crates/fakecloud-logs/src/service/misc.rs 89.58% 5 Missing ⚠️
crates/fakecloud-logs/src/service/groups.rs 96.87% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

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

3 issues found across 4 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="crates/fakecloud-logs/src/service/misc.rs">

<violation number="1" location="crates/fakecloud-logs/src/service/misc.rs:579">
P2: `StartLiveTail` silently ignores invalid `logGroupIdentifiers` values instead of returning a validation error.</violation>
</file>

<file name="crates/fakecloud-logs/src/service/queries.rs">

<violation number="1" location="crates/fakecloud-logs/src/service/queries.rs:92">
P1: Multi-group StartQuery inputs are reduced to one `log_group_name`, so `GetQueryResults` only scans a single group instead of all requested groups.</violation>

<violation number="2" location="crates/fakecloud-logs/src/service/queries.rs:429">
P2: Unknown `queryId` is silently treated as success with an empty list; return a not-found error instead.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

Comment thread crates/fakecloud-logs/src/service/queries.rs
Comment thread crates/fakecloud-logs/src/service/misc.rs Outdated
Comment thread crates/fakecloud-logs/src/service/queries.rs Outdated
- GetQueryResults: walk every log group/identifier captured by StartQuery
  (not just `log_group_name`). ARNs resolve to bare group names; duplicate
  references deduplicate. Multi-group queries now actually scan all groups.
- ListLogGroupsForQuery: return ResourceNotFoundException when the queryId
  is unknown instead of silently returning an empty array.
- StartLiveTail: validate that `logGroupIdentifiers` is an array of strings
  and reject non-string entries with InvalidParameterException, instead of
  silently dropping them via filter_map.
- extract_log_group_from_arn promoted to pub(crate) so queries.rs can reuse
  it for ARN -> group-name resolution.
@vieiralucas vieiralucas merged commit 1eed8d2 into main Apr 25, 2026
20 checks passed
@vieiralucas vieiralucas deleted the worktree-batch5-logs-gap-fill branch April 25, 2026 20:35
vieiralucas added a commit that referenced this pull request Apr 25, 2026
Three pre-existing test bugs that the protocol-map fix surfaced because
this branch is the first to actually run the full conformance + e2e
suites against post-#761 main:

- crates/fakecloud-conformance/tests/logs.rs::logs_list_log_groups_for_query
- crates/fakecloud-e2e/tests/logs.rs::logs_misc_stubs

Both tests called `ListLogGroupsForQuery` with a synthetic queryId. PR
#761's Cubic round (commit ad110ec) tightened the impl to return
`ResourceNotFoundException` for unknown queryIds — matching real AWS —
but the test sites still expected the previous "silently empty" answer.
Fix: create a real query via `StartQuery` first, then look it up.

- crates/fakecloud-e2e/tests/stepfunctions.rs::wait_for_execution

50 polls * 50ms = 2.5s. The PR-#757 SNS RSA-SHA256 signing + post-#760
SES-MIME work can push first-touch latency past that budget on
GitHub-hosted runners under parallel load. Bump the budget to 10s.
Locally executions still complete in <2s.

The conformance probe-map change in this PR exposed these because
`paths-filter` only triggers conformance + e2e when the conformance
crate changes, so the failing tests rode in unnoticed.
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.

1 participant