fix: logs command multi-topic search and SLS field query syntax#151
fix: logs command multi-topic search and SLS field query syntax#151rsonghuster merged 3 commits intomasterfrom
Conversation
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds computed SLS ChangesSLS Topic Filter Enhancement
Developer & Miscellaneous
Sequence Diagram(s)sequenceDiagram
participant CLI as CLI
participant Logs as Logs subcommand
participant SLS as SLS API / slsClient
CLI->>Logs: invoke `s logs` (function, --instance-id, --qualifier, --request-id)
Logs->>Logs: getInputs() -> compute `topicFilter` (FCLogs:<name> [or FCInstanceEvents:/<name>])
Logs->>Logs: getSlsQuery(search, qualifier, requestId, instanceId, topicFilter)
Logs->>SLS: slsClient.getLogs(params (query includes topicFilter), topic removed)
SLS-->>Logs: returns streaming or history logs
Logs-->>CLI: display logs
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
- Search FCInstanceEvents topic alongside FCLogs when --instance-id is specified - Use SLS field-specific query syntax: qualifier: "value", instanceID: "value", requestId: "value" - Stop unsetting LATEST qualifier (was preventing LATEST version log searches) - Support --request-id and --instance-id filters in realtime (--tail) mode - Update docs and help text for the new behavior Change-Id: Id05f45be1e1597dd89682a2ebab336b68735ca89 Co-developed-by: Claude <noreply@anthropic.com>
Change-Id: I2e55f6cfb2ac1a741f26bf8db8f959eb5570c637 Co-developed-by: Claude <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/subCommands/logs/index.ts (1)
14-27: 💤 Low value
topicfield is now dead code acrossIGetLogsandIRealtime
IGetLogs.topic(line 19) is never set by any caller in the new code (realtime,_realtimeOnce, andhistoryall buildIGetLogswithout atopicfield), makingdelete slsParams.topicingetLogsan always-no-op. Similarly,IRealtime.topic(line 26) is still required in the interface and still set bygetInputs()(line 189), but none of the methods that acceptIRealtime(realtime,_realtimeOnce,history) destructure or consume it.♻️ Suggested cleanup
interface IGetLogs { projectName: string; logStoreName: string; from: string | number; to: string | number; - topic?: string; query: string; } interface IRealtime { projectName: string; logStoreName: string; - topic: string; topicFilter: string; ... }And in
getLogs:- // Topic filtering is handled by __topic__ in query, remove topic from SLS request params - const slsParams: any = { ...requestParams }; - delete slsParams.topic; + const slsParams: any = { ...requestParams };And in
getInputs()return value:- topic, topicFilter,🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/subCommands/logs/index.ts` around lines 14 - 27, The topic property is dead code: update the IGetLogs and IRealtime interfaces to remove the unused topic field, stop setting/returning topic in getInputs(), and remove the unnecessary delete slsParams.topic in getLogs; alternatively, if topic is intended to be used, wire it through the callers (realtime, _realtimeOnce, history) and consume it where slsParams is constructed. Locate and change the symbols IGetLogs, IRealtime, getInputs, getLogs, realtime, _realtimeOnce, history and the reference to slsParams.topic to keep the interfaces and callers consistent (either remove topic everywhere or pass and use it end-to-end).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/subCommands/logs/index.ts`:
- Around line 14-27: The topic property is dead code: update the IGetLogs and
IRealtime interfaces to remove the unused topic field, stop setting/returning
topic in getInputs(), and remove the unnecessary delete slsParams.topic in
getLogs; alternatively, if topic is intended to be used, wire it through the
callers (realtime, _realtimeOnce, history) and consume it where slsParams is
constructed. Locate and change the symbols IGetLogs, IRealtime, getInputs,
getLogs, realtime, _realtimeOnce, history and the reference to slsParams.topic
to keep the interfaces and callers consistent (either remove topic everywhere or
pass and use it end-to-end).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2e0c3e16-a825-4ce6-abb3-0b2b6d3139b3
📒 Files selected for processing (7)
CLAUDE.md__tests__/ut/commands/logs_test.tsdocs/architecture.mddocs/technical-documentation.mdsrc/commands-help/logs.tssrc/index.tssrc/subCommands/logs/index.ts
✅ Files skipped from review due to trivial changes (5)
- src/index.ts
- docs/architecture.md
- docs/technical-documentation.md
- CLAUDE.md
- src/commands-help/logs.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/ut/commands/logs_test.ts
… on macOS runners setup-java@v1 fails to correctly set JAVA_HOME on newer macOS runners. Temurin doesn't support Java 8 on macOS, so use zulu distribution instead. Also upgrade checkout@v2→v4 and setup-node@v2→v4. Change-Id: Ibb3997e6df9cb3c744a1a35fbd90160b276f80b1 Co-developed-by: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ci_with_docker_linux.yaml:
- Line 16: The workflow uses an outdated action version
docker/setup-buildx-action@v2 (referenced in the docker-ci-standard and
docker-ci-custom jobs); update both occurrences to docker/setup-buildx-action@v4
so actionlint no longer flags the runner incompatibility and the workflow
matches the other upgraded Docker actions.
- Line 16: The workflow uses an outdated action tag
"docker/setup-buildx-action@v2"; replace both occurrences of this action string
in the CI workflow with the current stable tag "docker/setup-buildx-action@v4"
(or "v4.0.0") so the job definitions that reference setup-buildx-action use the
v4 release; locate the two occurrences of "docker/setup-buildx-action@v2" in the
file and update them to "docker/setup-buildx-action@v4".
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 285a7a11-a91b-4313-91eb-1e698b9ad87d
📒 Files selected for processing (2)
.github/workflows/ci_node16.yaml.github/workflows/ci_with_docker_linux.yaml
| steps: | ||
| - uses: actions/checkout@v2 | ||
| - uses: actions/checkout@v4 | ||
| - uses: docker/setup-buildx-action@v2 |
There was a problem hiding this comment.
Upgrade docker/setup-buildx-action@v2 to @v4 — actionlint reports it as too old.
Actionlint flags this action version as incompatible with the current GitHub Actions runner. The latest version is docker/setup-buildx-action@v4, and this should be applied at both Line 16 (docker-ci-standard) and Line 73 (docker-ci-custom). This PR already upgraded the other action versions — docker/setup-buildx-action should follow suit.
🔧 Proposed fix (apply to both Line 16 and Line 73)
- - uses: docker/setup-buildx-action@v2
+ - uses: docker/setup-buildx-action@v4📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - uses: docker/setup-buildx-action@v2 | |
| - uses: docker/setup-buildx-action@v4 |
🧰 Tools
🪛 actionlint (1.7.12)
[error] 16-16: the runner of "docker/setup-buildx-action@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/ci_with_docker_linux.yaml at line 16, The workflow uses an
outdated action version docker/setup-buildx-action@v2 (referenced in the
docker-ci-standard and docker-ci-custom jobs); update both occurrences to
docker/setup-buildx-action@v4 so actionlint no longer flags the runner
incompatibility and the workflow matches the other upgraded Docker actions.
🧩 Analysis chain
🌐 Web query:
What is the latest stable version of docker/setup-buildx-action GitHub Action?
💡 Result:
The latest stable version of the GitHub Action docker/setup-buildx-action is v4.0.0 (published 2026-03-05). [1][2] This is reflected by the repository’s “Latest release” being v4.0.0 and the dedicated release tag for v4.0.0. [1][2] Practical usage (stable pin): uses: docker/setup-buildx-action@v4.0.0 (or @v4 to track the v4 major). [1][2]
Citations:
- 1: https://github.com/docker/setup-buildx-action
- 2: https://github.com/marketplace/actions/docker-setup-buildx
🏁 Script executed:
cat -n .github/workflows/ci_with_docker_linux.yaml | sed -n '14,18p; 71,75p'Repository: devsapp/fc3
Length of output: 403
docker/setup-buildx-action@v2 is outdated and should be upgraded to v4.
Actionlint flags this action version as incompatible with the current GitHub Actions runner. The latest stable version is v4.0.0. Update both occurrences at line 16 and line 73 in the docker-ci workflow jobs.
🔧 Proposed fix
- - uses: docker/setup-buildx-action@v2
+ - uses: docker/setup-buildx-action@v4Apply the same change at line 73.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - uses: docker/setup-buildx-action@v2 | |
| - uses: docker/setup-buildx-action@v4 |
🧰 Tools
🪛 actionlint (1.7.12)
[error] 16-16: the runner of "docker/setup-buildx-action@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/ci_with_docker_linux.yaml at line 16, The workflow uses an
outdated action tag "docker/setup-buildx-action@v2"; replace both occurrences of
this action string in the CI workflow with the current stable tag
"docker/setup-buildx-action@v4" (or "v4.0.0") so the job definitions that
reference setup-buildx-action use the v4 release; locate the two occurrences of
"docker/setup-buildx-action@v2" in the file and update them to
"docker/setup-buildx-action@v4".
rsonghuster
force-pushed
the
fix/logs-multi-topic-and-query-syntax
branch
from
29a1b40 to
f634129
Compare
1 participant
Change-Id: Id05f45be1e1597dd89682a2ebab336b68735ca89
Co-developed-by: Claude noreply@anthropic.com
Summary by CodeRabbit
New Features
s logsnow supports multi-topic searching (includes FCInstanceEvents when using --instance-id) and preserves explicit qualifier LATEST.Documentation
s logsdocs with examples for --instance-id, --request-id, --qualifier and SLS topic types.--instance-idupdated.Tests
Chores