fix: align adapter docs and tests with apiKey conventions#354
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Thank you for following the naming conventions! 🙏 |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughMigrates adapter credential examples to ChangesCredential Field Transition (Axiom & Better Stack)
Memory adapter & shared config
create*Drain() test expansions
HTTP Framework Test Consolidation
Tooling & Documentation Enhancements
Estimated code review effort 🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Benchmark reportBundle size
|
commit: |
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 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/evlog/src/adapters/memory.ts`:
- Around line 31-33: The parseMaxEvents function currently accepts positive
non-integer numbers and returns Math.floor(value) which for values like 0.5
becomes 0 and disables draining; update parseMaxEvents (the function named
parseMaxEvents) to ensure fractional numbers do not resolve to 0 by either
requiring integers or re-checking after flooring: if value is a number, compute
const n = Math.floor(value) and then return n > 0 ? n : undefined (or
throw/validate for non-integers per project policy); similarly for string input,
parse to number, floor, and ensure the floored result is > 0 before returning
it.
In `@packages/evlog/test/adapters/datadog.test.ts`:
- Around line 321-325: The test’s environment cleanup runs only after tests,
allowing pre-set DATADOG_API_KEY/DD_API_KEY/NUXT_DATADOG_API_KEY to leak into
the “missing API key” scenario; make it hermetic by saving originals in a
variable (e.g., origDatadogApiKey, origDdApiKey, origNuxtDatadogApiKey) in a
beforeEach, then delete those env vars in that beforeEach to ensure the test
starts with no keys, and finally restore the saved originals in afterEach
(replace the current delete-only afterEach with restore logic). Ensure you
update both places referenced in the file (the existing afterEach and the
similar block around the other tests).
In `@packages/evlog/test/adapters/fs.test.ts`:
- Around line 312-316: The test "uses default dir when no config is provided"
only checks that mockedAppendFile was called but doesn't assert the resolved
output path; update the test for createFsDrain / createDrainContext to capture
the first call args of mockedAppendFile (or its path parameter) and assert that
the file path includes the expected default directory/name (the computed output
path used by createFsDrain when no config is passed). Use
mockedAppendFile.mock.calls[0][0] (or equivalent) to read the path and assert it
equals or matches the expected default path string.
In `@packages/evlog/test/adapters/hyperdx.test.ts`:
- Around line 118-121: The missing-apiKey test can be influenced by the
environment because you only clear env vars in the afterEach; add a beforeEach
(adjacent to the existing afterEach) that deletes
process.env.NUXT_HYPERDX_API_KEY and process.env.HYPERDX_API_KEY to ensure the
test runs with a clean environment, and apply the same beforeEach/afterEach
pattern for the other related test block referenced around the existing
afterEach to guarantee isolation.
In `@packages/evlog/test/adapters/memory.test.ts`:
- Around line 106-120: The test mutates process.env (EVLOG_MEMORY_STORE,
EVLOG_MEMORY_MAX_EVENTS) but only deletes them on the success path; to prevent
leaking env into other tests, snapshot the original values before setting them,
run the test logic (using clearMemoryLogs, createMemoryDrain, readMemoryLogs as
currently used), and in a finally block restore the original env entries
(reassign originals or delete if originally undefined) and perform any necessary
cleanup like clearMemoryLogs('env-store'); ensure the try/finally wraps the
setup and assertions so env restoration always runs even on failure.
In `@packages/evlog/test/adapters/otlp.test.ts`:
- Around line 433-436: The missing-endpoint tests are non-deterministic because
environment variables are only cleared in afterEach; add a beforeEach that
deletes process.env.NUXT_OTLP_ENDPOINT and
process.env.OTEL_EXPORTER_OTLP_ENDPOINT so the "missing-endpoint" path is
isolated, and apply the same beforeEach change for the other test block covering
lines 444-451; reference the env names (process.env.NUXT_OTLP_ENDPOINT,
process.env.OTEL_EXPORTER_OTLP_ENDPOINT) and add the cleanup at the start of the
relevant describe/it blocks.
In `@packages/evlog/test/adapters/sentry.test.ts`:
- Around line 308-311: The tests are flaky because SENTRY_DSN/NUXT_SENTRY_DSN
are only cleared in afterEach (see afterEach block), which is too late if the
environment already defines them; add a beforeEach that deletes
process.env.SENTRY_DSN and process.env.NUXT_SENTRY_DSN (or clear them at the top
of the test file) so each test starts with a clean environment, and apply the
same change for the other test group that currently clears these vars only in
its afterEach.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 52d22602-42f9-45cd-a89d-bb4e043860a3
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (38)
.changeset/add-memory-drain-adapter.mdapps/docs/content/3.integrate/adapters/01.overview.mdapps/docs/content/3.integrate/adapters/cloud/01.axiom.mdapps/docs/content/3.integrate/adapters/cloud/03.posthog.mdapps/docs/content/3.integrate/adapters/cloud/05.better-stack.mdapps/docs/content/3.integrate/adapters/self-hosted/03.memory.mdapps/docs/content/3.integrate/frameworks/00.overview.mdapps/docs/content/3.integrate/frameworks/02.nextjs.mdapps/docs/content/3.integrate/frameworks/06.nestjs.mdapps/docs/content/4.use-cases/4.audit/04.pipeline.mdapps/docs/content/4.use-cases/4.audit/06.recipes.mdapps/docs/content/4.use-cases/5.enrichers.mdapps/docs/content/6.reference/5.vs-other-loggers.mdapps/docs/skills/review-logging-patterns/SKILL.mdpackages/evlog/README.mdpackages/evlog/package.jsonpackages/evlog/src/adapters/memory.tspackages/evlog/src/next/index.tspackages/evlog/src/nuxt/module.tspackages/evlog/src/shared/config.tspackages/evlog/test/README.mdpackages/evlog/test/adapters/axiom.test.tspackages/evlog/test/adapters/better-stack.test.tspackages/evlog/test/adapters/config.test.tspackages/evlog/test/adapters/datadog.test.tspackages/evlog/test/adapters/fs.test.tspackages/evlog/test/adapters/hyperdx.test.tspackages/evlog/test/adapters/memory.test.tspackages/evlog/test/adapters/otlp.test.tspackages/evlog/test/adapters/sentry.test.tspackages/evlog/test/frameworks/elysia.test.tspackages/evlog/test/frameworks/express.test.tspackages/evlog/test/frameworks/fastify.test.tspackages/evlog/test/frameworks/hono.test.tspackages/evlog/test/frameworks/nestjs.test.tspackages/evlog/test/frameworks/react-router.test.tspackages/evlog/test/frameworks/sveltekit.test.tspackages/evlog/test/toolkit/enrichers.test.ts
💤 Files with no reviewable changes (1)
- packages/evlog/test/frameworks/express.test.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/evlog/src/adapters/memory.ts`:
- Around line 32-38: The string branch of parseMaxEvents (in
packages/evlog/src/adapters/memory.ts) accepts values like "Infinity" because it
parses then floors without checking finiteness; update the string-handling path
in parseMaxEvents to parse the string to a number, verify
Number.isFinite(parsed) and that the floored value is > 0 before returning it
(otherwise return undefined) so writeToMemory's trimming logic works and
Infinity is rejected.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 455c4703-0cf1-404b-adef-0bba3c944fe7
📒 Files selected for processing (7)
packages/evlog/src/adapters/memory.tspackages/evlog/test/adapters/datadog.test.tspackages/evlog/test/adapters/fs.test.tspackages/evlog/test/adapters/hyperdx.test.tspackages/evlog/test/adapters/memory.test.tspackages/evlog/test/adapters/otlp.test.tspackages/evlog/test/adapters/sentry.test.ts
🔗 Linked issue
📚 Description
📝 Checklist
Summary by CodeRabbit
Documentation
New Features
Tests