Uh oh!
There was an error while loading. Please reload this page.
feat: add observability abstractions and wire to runtime - #2147
Draft
nborges-aws wants to merge 1 commit into
Draft
feat: add observability abstractions and wire to runtime#2147nborges-aws wants to merge 1 commit into
nborges-aws wants to merge 1 commit into
Conversation
Contributor
Claude Security Review: no high-confidence findings. (run) |
codecov-commenter
commented
Aug 31, 2026
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@## refactor #2147 +/- ##
============================================
+ Coverage 97.29% 97.33% +0.03%
============================================
Files 479 486 +7 Lines 29673 30066 +393 ============================================
+ Hits 28871 29264 +393
Misses 802 802 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
AgentCore Harness Review
Verdict: Looks good
Nice, well-factored change. The ObservabilityClient / SourceReader split cleanly separates identity resolution from CloudWatch I/O and leaves an obvious extension point for future resource kinds. A few things I looked at carefully:
- AWS SDK mocking:
sourceReader.test.tsandclient.test.tsmock at the SDK client boundary (send) rather than the pieces of internal state, which matches the guidance in the review criteria. No excessive mocking. - Pagination + limit:
CloudWatchSourceReader.searchLogscorrectly caps the per-pagelimitatquery.limit - yielded, terminates whennextToken === requestToken, and short-circuits onlimit <= 0. Edge cases (limit=1, single page, empty page with token) all look right. - Live Tail:
tailLogshandles both the in-bandSessionTimeoutExceptionevent and the thrown variant, reconnects only when timed out, and exits cleanly on abort. Legacyarn:...:*suffix stripping is guarded and covered. - Missing log group: nice consistent
ResourceNotFoundErrortranslation with actionable message in both search and tail paths (including the pre-flightDescribeLogGroupscase for tail). - Handler wiring: input validation (
--tailvs--since/--until,--limitoutside search mode,--since > --until) all raise typedInputValidationErrors and are covered in tests.withUserCancellationpropagates the abort into the SDK calls. - Telemetry: this codebase currently instruments telemetry at the top-level command run in
src/index.tsrather than per-handler, so no per-feature instrumentation is missing here.
Non-blocking observations if you want to iterate later:
--tailis effectively a no-op flag when neither--sincenor--untilis passed (tailing is already the default in that case). Consider either making search the default with--tailrequired for streaming, or documenting the current behavior in the flag help. Either is fine, just be intentional.ResourceFlagValuesinhandlers/observability/types.tsduplicates the existingFlagsOfinrouter/handler.tsx. Could reuse or export the router one to keep a single source of truth.- The log group naming convention
/aws/bedrock-agentcore/runtimes/<id>-<qualifier>is hard-coded; if the service ever exposes this via an API, worth switching to that to avoid a lurking coupling.
Nothing here blocks merge. Ship it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR adds reusable observability infrastructure. The setup is built to apply generally across our primitives, while allowing for resource-specific customization where necessary. This PR wires runtime to the infrastructure. Remaining primitives wiring will be released as a follow pending alignment on the abstractions added here.
ObservabilityClientas the shared API entry pointlogsunder RuntimeLogRecordArchitecture
Runtime handler → ObservabilityClient → RuntimeSourceResolver → CloudWatchSourceReader → LogRecord
The resolver owns resource-to-log-group translation. The source reader owns CloudWatch mechanics without ever needing knowledge of our resource types. The client is responsible for the orchestration of these layers.
Commands
Tail logs:
Search logs:
--qualifier selects a non-default Runtime endpoint.
Type of Change
Testing
How have you tested the change?
bun run test(2322 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.