Conversation
🦋 Changeset detectedLatest commit: 6171c6c The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
📝 WalkthroughWalkthroughAdds the ChangesEve observability integration
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant EveExtension
participant WideEventsHook
participant ResolveDrain
participant EventAdapters
EveExtension->>WideEventsHook: pass validated configuration
WideEventsHook->>ResolveDrain: resolve adapters and batching
ResolveDrain->>EventAdapters: fan out wide events
EventAdapters-->>ResolveDrain: drain results or failures
sequenceDiagram
participant EveTurn
participant DefineEvlogInstrumentation
participant AISDKStep
EveTurn->>DefineEvlogInstrumentation: expose active turn and session
DefineEvlogInstrumentation->>AISDKStep: attach request and session IDs
AISDKStep-->>DefineEvlogInstrumentation: omit context outside tracked turns
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 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 `@packages/eve-extension/extension/extension.ts`:
- Line 39: Update the adapter schema definition to reject empty adapter arrays
by applying a minimum-length constraint to the array branch of the z.union.
Preserve support for a single adapter value and non-empty adapter lists so
resolveDrain() always has at least one adapter.
In `@packages/eve-extension/extension/hooks/wide-events.ts`:
- Around line 5-13: Correct the mismatch between the JSDoc and implementation in
the default evlog hook: update the comment to state that an omitted mount
service is fixed to “eve-agent” via toHookOptions and init.env.service, or
change the fallback behavior so the service is resolved from turn context as
documented.
In `@packages/eve-extension/extension/lib/adapters.ts`:
- Around line 41-48: Add JSDoc comments to the exported AdapterConfig type and
ResolveDrainOptions interface, documenting AdapterConfig’s accepted adapter-name
or typed-options forms and ResolveDrainOptions’ adapter input plus optional
batch size and interval behavior. Keep the existing type definitions unchanged.
In `@packages/eve-extension/extension/lib/options.ts`:
- Around line 5-15: Add JSDoc documentation to the public ExtensionConfig
interface, describing its declarative configuration contract and the purpose of
each configuration property. Keep the existing property types and interface
shape unchanged.
- Line 41: Update the sampleRate calculation in the options configuration flow
to multiply config.sample by 100 without rounding, preserving fractional
percentage values such as 0.1%; retain undefined handling and add a regression
test covering sub-percent sample values.
In `@packages/eve-extension/extension/tools/annotate.ts`:
- Around line 13-31: Add tests under packages/eve-extension/test for the public
tool defined by defineTool, covering valid string/number/boolean values,
rejection of invalid keys and nested values, logger writes using ctx,
reserved-key handling, and replacement of an existing annotation. Ensure the
tests exercise both schema validation and execute behavior, including a
regression case for the reported bug before the fix.
- Around line 19-29: Prevent the annotation key accepted by the tool schema from
overwriting evlog-owned logger fields when execute calls useLogger(ctx).set.
Update the key validation around the visible key schema and execute method to
reject reserved top-level names such as service, error, approval, audit, and
request_id, or route annotations through a dedicated namespace while preserving
valid annotation behavior.
- Around line 23-29: Update the annotation tool’s value validation and execute
path around the visible schema and execute method so only approved business
identifiers and amounts can be stored; reject unsupported primitive values
before useLogger(ctx).set persists them, including cases where redaction is
disabled or paths omit the dynamic key. Add coverage for the tool schema and the
relevant policy behavior.
In `@packages/eve-extension/test/adapters.test.ts`:
- Around line 31-40: Expand the test named “keeps draining the other adapters
when one fails” to use test/helpers utilities, controlled drain spies, and fake
timers; assert the event is delivered to the memory adapter despite OTLP
failure, and cover the batching path by verifying the expected batch payload.
Preserve the existing failure-isolation scenario while asserting fan-out and
delivery rather than only drain() resolution.
🪄 Autofix
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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 5d48d784-8adc-4b01-9936-53a2e89a1d1c
⛔ Files ignored due to path filters (2)
packages/evlog/test/toolkit/__snapshots__/api-surface.test.ts.snapis excluded by!**/*.snappnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (21)
.changeset/eve-extension-package.md.changeset/eve-instrumentation-bridge.md.github/pull_request_template.md.github/workflows/semantic-pull-request.ymlpackages/eve-extension/.gitignorepackages/eve-extension/AGENTS.mdpackages/eve-extension/README.mdpackages/eve-extension/extension/extension.tspackages/eve-extension/extension/hooks/wide-events.tspackages/eve-extension/extension/instructions.mdpackages/eve-extension/extension/lib/adapters.tspackages/eve-extension/extension/lib/options.tspackages/eve-extension/extension/skills/observability/SKILL.mdpackages/eve-extension/extension/tools/annotate.tspackages/eve-extension/package.jsonpackages/eve-extension/test/adapters.test.tspackages/eve-extension/test/options.test.tspackages/eve-extension/tsconfig.jsonpackages/eve-extension/vitest.config.tspackages/evlog/src/eve/index.tspackages/evlog/test/eve.test.ts
| /** Service name on every event. Defaults to the agent name. */ | ||
| service: z.string().optional(), | ||
| /** Where events go. Several adapters fan out. */ | ||
| adapter: z.union([adapter, z.array(adapter)]).default('fs'), |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Reject an empty adapter list.
adapter: [] passes this schema. resolveDrain() then creates no drains, and events complete without delivery or an error. Require at least one adapter.
Proposed fix
- adapter: z.union([adapter, z.array(adapter)]).default('fs'),
+ adapter: z.union([adapter, z.array(adapter).min(1)]).default('fs'),📝 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.
Suggested change
| adapter: z.union([adapter, z.array(adapter)]).default('fs'), | |
| adapter: z.union([adapter, z.array(adapter).min(1)]).default('fs'), |
🤖 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 `@packages/eve-extension/extension/extension.ts` at line 39, Update the adapter
schema definition to reject empty adapter arrays by applying a minimum-length
constraint to the array branch of the z.union. Preserve support for a single
adapter value and non-empty adapter lists so resolveDrain() always has at least
one adapter.
Comment on lines
+5
to
+13
| /** | ||
| * One evlog wide event per agent turn, wired from the mount config. | ||
| * | ||
| * The agent name is not available to a hook file at module scope, so the | ||
| * service falls back to the mount's `service` and is otherwise resolved from | ||
| * the turn context by evlog itself. | ||
| */ | ||
| export default defineEvlogHook( | ||
| toHookOptions(extension.config as ExtensionConfig, extension.config.service ?? 'eve-agent'), |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Correct the service fallback documentation.
Line 10 says evlog resolves an omitted service from turn context. Line 13 passes 'eve-agent' to toHookOptions, and the option translator writes that value to init.env.service. The service is therefore fixed to 'eve-agent' when mount configuration omits service.
Update the JSDoc to describe the fixed fallback, or change the implementation to resolve the agent name as documented.
🤖 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 `@packages/eve-extension/extension/hooks/wide-events.ts` around lines 5 - 13,
Correct the mismatch between the JSDoc and implementation in the default evlog
hook: update the comment to state that an omitted mount service is fixed to
“eve-agent” via toHookOptions and init.env.service, or change the fallback
behavior so the service is resolved from turn context as documented.
Comment on lines
+41
to
+48
| export type AdapterConfig = | ||
| | AdapterName | ||
| | { type: AdapterName, options?: Record<string, unknown> } | ||
|
|
||
| export interface ResolveDrainOptions { | ||
| adapter: AdapterConfig | AdapterConfig[] | ||
| batch?: { size: number, intervalMs: number } | ||
| } |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Add JSDoc to exported configuration types.
AdapterConfig and ResolveDrainOptions are public TypeScript APIs. Document their accepted forms and batching behavior.
As per coding guidelines, “Write JSDoc for all public TypeScript APIs.”
🤖 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 `@packages/eve-extension/extension/lib/adapters.ts` around lines 41 - 48, Add
JSDoc comments to the exported AdapterConfig type and ResolveDrainOptions
interface, documenting AdapterConfig’s accepted adapter-name or typed-options
forms and ResolveDrainOptions’ adapter input plus optional batch size and
interval behavior. Keep the existing type definitions unchanged.
Source: Coding guidelines
Comment on lines
+5
to
+15
| export interface ExtensionConfig { | ||
| service?: string | ||
| adapter: AdapterConfig | AdapterConfig[] | ||
| message: 'omit' | 'preview' | 'full' | ||
| messagePreviewLength?: number | ||
| redact: boolean | { paths: string[] } | ||
| sample?: number | ||
| keepOnFailure: boolean | ||
| sessionEvent: boolean | ||
| batch?: { size: number, intervalMs: number } | ||
| } |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Add JSDoc to ExtensionConfig.
ExtensionConfig is a public TypeScript API. Document its declarative configuration contract.
As per coding guidelines, “Write JSDoc for all public TypeScript APIs.”
🤖 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 `@packages/eve-extension/extension/lib/options.ts` around lines 5 - 15, Add
JSDoc documentation to the public ExtensionConfig interface, describing its
declarative configuration contract and the purpose of each configuration
property. Keep the existing property types and interface shape unchanged.
Source: Coding guidelines
| * in but never drop one. | ||
| */ | ||
| export function toHookOptions(config: ExtensionConfig, agentName: string): EvlogEveOptions { | ||
| const sampleRate = config.sample === undefined ? undefined : Math.round(config.sample * 100) |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Preserve sub-percent sample rates.
sample: 0.001 is valid configuration, but Math.round() converts its intended 0.1% rate to 0%. This silently drops all routine turns for valid sub-percent settings. Pass the percentage without rounding and add a regression test.
Proposed fix
- const sampleRate = config.sample === undefined ? undefined : Math.round(config.sample * 100)
+ const sampleRate = config.sample === undefined ? undefined : config.sample * 100📝 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.
Suggested change
| const sampleRate = config.sample === undefined ? undefined : Math.round(config.sample * 100) | |
| const sampleRate = config.sample === undefined ? undefined : config.sample * 100 |
🤖 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 `@packages/eve-extension/extension/lib/options.ts` at line 41, Update the
sampleRate calculation in the options configuration flow to multiply
config.sample by 100 without rounding, preserving fractional percentage values
such as 0.1%; retain undefined handling and add a regression test covering
sub-percent sample values.
Comment on lines
+13
to
+31
| export default defineTool({ | ||
| description: | ||
| 'Record a business fact about the current turn on its observability event. ' | ||
| + 'Use it for identifiers, decisions and amounts that would make this turn ' | ||
| + 'findable later. Never record user message content or personal data.', | ||
| inputSchema: z.object({ | ||
| key: z | ||
| .string() | ||
| .regex(/^[a-z][a-z0-9_]*$/, 'lowercase identifier, e.g. order or customer') | ||
| .describe('Field name to set on the event, e.g. "order".'), | ||
| value: z | ||
| .record(z.string(), z.union([z.string(), z.number(), z.boolean()])) | ||
| .describe('Flat object of facts, e.g. { "id": "4821", "amount": 89 }.'), | ||
| }), | ||
| approval: never(), | ||
| execute({ key, value }, ctx) { | ||
| useLogger(ctx).set({ [key]: value }) | ||
| return { recorded: key } | ||
| }, |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add a matching test for the new public tool.
The provided changes add schema and execution behavior but no matching test under packages/eve-extension/test. Cover valid primitive values, invalid keys, nested-value rejection, logger writes with ctx, reserved-key behavior, and annotation replacement.
As per coding guidelines, every code change must have a matching test, and bug fixes require a failing regression test before the fix.
🤖 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 `@packages/eve-extension/extension/tools/annotate.ts` around lines 13 - 31, Add
tests under packages/eve-extension/test for the public tool defined by
defineTool, covering valid string/number/boolean values, rejection of invalid
keys and nested values, logger writes using ctx, reserved-key handling, and
replacement of an existing annotation. Ensure the tests exercise both schema
validation and execute behavior, including a regression case for the reported
bug before the fix.
Source: Coding guidelines
Comment on lines
+19
to
+29
| key: z | ||
| .string() | ||
| .regex(/^[a-z][a-z0-9_]*$/, 'lowercase identifier, e.g. order or customer') | ||
| .describe('Field name to set on the event, e.g. "order".'), | ||
| value: z | ||
| .record(z.string(), z.union([z.string(), z.number(), z.boolean()])) | ||
| .describe('Flat object of facts, e.g. { "id": "4821", "amount": 89 }.'), | ||
| }), | ||
| approval: never(), | ||
| execute({ key, value }, ctx) { | ||
| useLogger(ctx).set({ [key]: value }) |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
rg -n -C 8 'interface AuditableLogger|type AuditableLogger|class AuditableLogger|\bset\s*\(' \
packages/evlog/src packages/eve-extension
rg -n -C 4 'request_id|service|duration_ms|error|annotation' \
packages/evlog/src packages/eve-extensionRepository: HugoRCD/evlog
Length of output: 50372
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "== locate audit/types/logger files =="
git ls-files packages/evlog/src | rg '(^packages/evlog/src/(audit|types)\.ts$|logger|request|types|eve/index\.ts$|eve/index\.ts$)'
echo
echo "== audit.ts outline =="
ast-grep outline packages/evlog/src/audit.ts --view compact 2>/dev/null || true
echo
echo "== audit.ts relevant lines =="
sed -n '1,260p' packages/evlog/src/audit.ts
echo
echo "== inspect requested dynamic annotation tool file =="
sed -n '1,140p' packages/eve-extension/extension/tools/annotate.tsRepository: HugoRCD/evlog
Length of output: 11711
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "== types.ts outline =="
ast-grep outline packages/evlog/src/types.ts --view compact 2>/dev/null || true
echo
echo "== types.ts relevant sections =="
wc -l packages/evlog/src/types.ts
sed -n '1,260p' packages/evlog/src/types.ts
echo
echo "== logger.ts outline =="
ast-grep outline packages/evlog/src/logger.ts --view compact 2>/dev/null || true
echo
echo "== logger.ts relevant sections =="
wc -l packages/evlog/src/logger.ts
sed -n '1,260p' packages/evlog/src/logger.ts
echo
echo "== focused search for set implementation =="
rg -n -C 6 'function createLogger|set\(|set\s*:|getContext|WideEvent|FieldContext|context\+=' packages/evlog/src/logger.ts packages/evlog/src/types.ts packages/evlog/src/eve/index.tsRepository: HugoRCD/evlog
Length of output: 50371
Prevent annotation keys from overwriting evlog-owned fields.
set() merges into the logger context with Reflect.set, and emit() exposes that context to tail sampling and drains. Values accepted for this tool can overwrite evlog-owned fields such as service, error, approval, audit, and request_id; use a dedicated namespace/prefix for annotations or reject reserved top-level keys.
🤖 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 `@packages/eve-extension/extension/tools/annotate.ts` around lines 19 - 29,
Prevent the annotation key accepted by the tool schema from overwriting
evlog-owned logger fields when execute calls useLogger(ctx).set. Update the key
validation around the visible key schema and execute method to reject reserved
top-level names such as service, error, approval, audit, and request_id, or
route annotations through a dedicated namespace while preserving valid
annotation behavior.
Comment on lines
+23
to
+29
| value: z | ||
| .record(z.string(), z.union([z.string(), z.number(), z.boolean()])) | ||
| .describe('Flat object of facts, e.g. { "id": "4821", "amount": 89 }.'), | ||
| }), | ||
| approval: never(), | ||
| execute({ key, value }, ctx) { | ||
| useLogger(ctx).set({ [key]: value }) |
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
rg -n -C 5 '\bset\s*\(|redact|redaction|PII|email|phone|token' \
packages/evlog/src packages/eve-extension
rg -n -C 5 'annotate|evlog__annotate|email|phone|token' \
packages/eve-extension || trueRepository: HugoRCD/evlog
Length of output: 50372
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '--- annotate.ts ---\n'
cat -n packages/eve-extension/extension/tools/annotate.ts
printf '\n--- eve/index.ts around tools registration ---\n'
fd -a 'index\.ts$' packages/eve-extension extends packages evlog src eve 2>/dev/null | sed 's#^\./##' | grep 'packages/eve-extension' | head -20 || true
rg -n -C 8 'tools|annotate|default|execute|approve' packages/eve-extension src/shared packages/eve-extension/extension packages/evlog/src packages/eve-extension/src 2>/dev/null | head -320
printf '\n--- relevant logger slices ---\n'
sed -n '800,875p' packages/evlog/src/logger.ts | cat -n
sed -n '248,290p' packages/evlog/src/logger.ts | cat -n
sed -n '750,850p' packages/evlog/src/redact.ts | cat -n
printf '\n--- redact path matcher / value behavior snippets ---\n'
rg -n -C 12 'function compileRedactPathMatchers|function redactValueByPaths|markGloballyRedacted|redactEvent' packages/evlog/src/redact.ts
sed -n '1,230p' packages/evlog/src/redact.ts | cat -n
printf '\n--- tests for annotate (if any) ---\n'
git ls-files 'packages/eve-extension/**' 'packages/evlog/**' | grep -i 'annotate\|eve' | grep -E '(test|spec|__tests__|tests)' || trueRepository: HugoRCD/evlog
Length of output: 242
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo '--- annotate.ts ---'
cat -n packages/eve-extension/extension/tools/annotate.ts
echo
echo '--- eve/index.ts/tool references ---'
rg -n -C 8 'tools|annotate|default|execute|approve|EvlogTool|defineTool|tool' \
packages/eve-extension/extension packages/eve-extension/packages packages/eve-extension/src packages/evlog/src 2>/dev/null | head -360
echo
echo '--- relevant logger slices ---'
sed -n '800,875p' packages/evlog/src/logger.ts | cat -n
sed -n '248,290p' packages/evlog/src/logger.ts | cat -n
sed -n '750,850p' packages/evlog/src/redact.ts | cat -n
echo
echo '--- redact path/value behavior ---'
rg -n -C 12 'function compileRedactPathMatchers|function redactValueByPaths|markGloballyRedacted|redactEvent' packages/evlog/src/redact.ts
sed -n '1,230p' packages/evlog/src/redact.ts | cat -n
echo
echo '--- annotate tests ---'
git ls-files 'packages/eve-extension/**' 'packages/evlog/**' | grep -i 'annotate\|eve' | grep -E '(test|spec|__tests__|tests)' || trueRepository: HugoRCD/evlog
Length of output: 28162
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo '--- eve extension tool discovery/includes ---'
rg -n -C 10 "annotation|annotate|tools" packages/eve-extension packages/evlog 2>/dev/null | head -240
echo
echo '--- packages/eve-extension extension.ts tail/redact mapping ---'
sed -n '34,65p' packages/eve-extension/extension/extension.ts | cat -n
sed -n '65,95p' packages/eve-extension/extension/extension.ts | cat -n
sed -n '1,90p' packages/eve-extension/extension/lib/options.ts | cat -nRepository: HugoRCD/evlog
Length of output: 22178
Enforce annotation value eligibility before storage.
The annotation schema permits any primitive value, but the extension only documents what agents must not write. With redact: false or { paths } that does not include the dynamic annotation key, raw values can drift into the drain pipeline at least via built-in pattern matches. Add deterministic validation at this boundary, such as allowing only business identifiers and amounts, and add tests for the tool schema and policy.
🤖 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 `@packages/eve-extension/extension/tools/annotate.ts` around lines 23 - 29,
Update the annotation tool’s value validation and execute path around the
visible schema and execute method so only approved business identifiers and
amounts can be stored; reject unsupported primitive values before
useLogger(ctx).set persists them, including cases where redaction is disabled or
paths omit the dynamic key. Add coverage for the tool schema and the relevant
policy behavior.
Comment on lines
+31
to
+40
| it('keeps draining the other adapters when one fails', async () => { | ||
| const error = vi.spyOn(console, 'error').mockImplementation(() => {}) | ||
| const drain = resolveDrain({ | ||
| adapter: [{ type: 'otlp', options: { endpoint: 'http://127.0.0.1:1' } }, 'memory'], | ||
| }) | ||
|
|
||
| await drain(drainContext()) | ||
|
|
||
| error.mockRestore() | ||
| }) |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Assert adapter delivery and batching behavior.
This test only checks that drain() resolves. It does not prove that the memory adapter received the event after OTLP fails. It also leaves the batching branch untested. Use controlled drain spies and fake timers to assert fan-out, failure isolation, and batch payload delivery.
As per coding guidelines, “Every code change must have a matching test” and tests must use helpers from test/helpers/.
🤖 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 `@packages/eve-extension/test/adapters.test.ts` around lines 31 - 40, Expand
the test named “keeps draining the other adapters when one fails” to use
test/helpers utilities, controlled drain spies, and fake timers; assert the
event is delivered to the memory adapter despite OTLP failure, and cover the
batching path by verifying the expected batch payload. Preserve the existing
failure-isolation scenario while asserting fan-out and delivery rather than only
drain() resolution.
Source: Coding guidelines
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 free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
🔗 Linked issue
📚 Description
📝 Checklist
Summary by CodeRabbit
New Features
@evlog/eveextension for configurable observability, including adapters, sampling, redaction, batching, and failure retention.annotatetool and observability guidance for recording structured business context.Documentation
Bug Fixes