feat: wire observability client to harness and evals - #2182
Conversation
648c527 to
7c4f499
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## refactor #2182 +/- ##
============================================
- Coverage 97.05% 96.97% -0.09%
============================================
Files 555 559 +4
Lines 38386 38581 +195
============================================
+ Hits 37254 37412 +158
- Misses 1132 1169 +37 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
7c4f499 to
9564d5a
Compare
9564d5a to
cbc10fc
Compare
cbc10fc to
6c2d90f
Compare
6c2d90f to
d95292b
Compare
d95292b to
45e3cb9
Compare
8252f2f to
23e3d01
Compare
|
Claude Security Review: no high-confidence findings. (run) |
Hweinstock
left a comment
There was a problem hiding this comment.
few questions, mostly looks good.
| if (config.output) return resolve(cwd, config.output); | ||
| return resolve(cwd, `${config.traceId}.json`); | ||
| } | ||
| export { resolveTraceOutputPath } from "../../observability/traceOutputPath"; |
There was a problem hiding this comment.
whats the purpose of re-exporting here? Could we migrate consumers to the new path?
| flags: harnessFlags, | ||
| read: async (ctx, flags, query, signal) => { | ||
| const options = coreOptsFromCtx(ctx); | ||
| const runtime = await core.harness.resolveRuntime(flags.id, options, signal); |
| }); | ||
| }); | ||
|
|
||
| describe("HarnessClient.resolveRuntime", () => { |
There was a problem hiding this comment.
should we be using the fixtures here?
| private readonly now: () => number = () => Date.now(), | ||
| ) {} | ||
| ) { | ||
| this.cloudWatch = new CloudWatchClient(clients); |
There was a problem hiding this comment.
is there a reason we don't instantiate this here
agentcore-cli/src/core/index.tsx
Line 116 in 23e3d01
If a future client also wants to consume the CloudWatchClient, we would end up with two instances of the class, which I don't think we want.
23e3d01 to
41172a4
Compare
|
Claude Security Review: no high-confidence findings. (run) |
41172a4 to
cadd0ae
Compare
|
Claude Security Review: no high-confidence findings. (run) |
Description
Extend shared observability abstraction to Batch Evaluation and Harness. Plus, add endpoint qualifier support to Runtime observability commands.
GetLogEvents--qualifierflag to Runtime and Harness (falls back to default endpoint)Type of Change
Testing
How have you tested the change?
Stack created with GitHub Stacks CLI • Give Feedback 💬