feat: support project aware logging in runtime - #2271
Conversation
|
Claude Security Review: no high-confidence findings. (run) |
There was a problem hiding this comment.
AgentCore Harness Review
Verdict: Looks good
Clean refactor + feature. Highlights I checked:
src/core/observability.tsis split intoobservability/{runtime,insights,client,traces,types}.ts, and the oldObservabilityClient.resolveDeployedRuntime+handlers/runtime/resolveRuntimeTarget.tsare deleted with no remaining callers (git grep resolveDeployedRuntime|resolveRuntimeTarget|DeployedRuntimeonpr2271is empty). TheCoreObservabilityClientinterface moved tocore/observability/types.tsand is re-exported fromcore/observability/index.ts, andTestObservabilityClientwas trimmed to match — no dangling references.- The deleted
observability.test.tscoverage is preserved:runInsightsQueryandsanitizeQueryValueincore/observability/insights.test.ts,runtimeLogGroupincore/observability/runtime.test.ts, andlistTraces/getTracebehavior viahandlers/runtime/traces/traces.test.tsx. project log runtimeuses the sameprojectManager.resolveDeployedResourceseam asproject invoke, and pins the region off the resolved target rather than the ambient one.selectProjectResourcewas extended with anoperationstring and moved up tohandlers/project/selection.ts, and both invoke callers pass "invoke" — no stale imports.- The new
handlers/project/log/runtime.test.tsxsticks to the recommended pattern (real tempdir +ProjectSpecSchema.parse+ fakeProjectBackendat the domain seam, no fs mocks). Telemetry is emitted by the router'scli.command_runfor all commands, so the new subtree is instrumented automatically.
Nothing needs to change before merge.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## refactor #2271 +/- ##
============================================
- Coverage 97.06% 96.94% -0.12%
============================================
Files 569 570 +1
Lines 39322 39200 -122
============================================
- Hits 38167 38004 -163
- Misses 1155 1196 +41 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
3201f55 to
b8b8877
Compare
|
Claude Security Review: no high-confidence findings. (run) |
Hweinstock
left a comment
There was a problem hiding this comment.
one testing comment, otherwise lgtm.
| agentcore runtime endpoint list --id <runtimeId> --max-results 20 | ||
|
|
||
| # Follow a Runtime's logs live (Ctrl+C to stop); inside a project --id is optional | ||
| # Follow a Runtime's logs live by physical ID (Ctrl+C to stop) |
There was a problem hiding this comment.
nit: physical ID feels like strange language? Isn't this the resource id or more specifically the runtime's id?
| ); | ||
| }); | ||
|
|
||
| async function inProject( |
There was a problem hiding this comment.
should we just call create to scaffold the project? If create behavior changes, this will start failing and be unclear why.
b8b8877 to
bffbcc3
Compare
|
Claude Security Review: no high-confidence findings. (run) |
bffbcc3 to
0b873b6
Compare
|
Claude Security Review: no high-confidence findings. (run) |
Description
Adds project-scoped Runtime logging commands.
The command resolves a runtime from the current project into its deployed id and target region. The rest is delegated to the existing observability log search/tail implementation. Projects with only one configured runtime can omit
--name; projects with multiple runtimes are required to select one explicitly.This PR also makes the following updates to observability:
CoreObservabilityClientfrom runtime handler types into observabilityType of Change
Testing
How have you tested the change?
bun run test(3169 pass, 0 fail)npm run test:unitandnpm run test:integnpm run typechecknpm run lintsrc/assets/, I rannpm run test:update-snapshotsand committed the updated snapshotsChecklist
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the
terms of your choice.