Add logger plugin #119
Add logger plugin #119
Conversation
📝 WalkthroughWalkthroughThis pull request introduces several changes across multiple files, primarily focusing on the addition of a new logger plugin for the Changes
Possibly related PRs
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (9)
examples/cli/indexers/1-evm.indexer.ts (1)
7-9: Document the significance of the starting block numberThe
10_000_000nvalue appears to be a magic number. Consider adding a comment explaining why this specific block number was chosen as the starting point.startingCursor: { + // Start indexing from block 10M (post-merge block) orderKey: 10_000_000n, },packages/indexer/src/plugins/logger.ts (2)
23-30: Improve error handling and add documentationThe
useLoggerhook could benefit from better documentation and type safety.Consider these improvements:
+/** + * Hook to access the logger instance from the indexer context. + * @throws {Error} If logger plugin is not initialized in the indexer + * @returns Consola logger instance + */ export function useLogger(): ConsolaInstance { const ctx = useIndexerContext(); - if (!ctx?.logger) - throw new Error("Logger plugin is not available in context"); + if (!ctx?.logger) { + throw new Error( + "Logger plugin is not available in context. Did you forget to add the logger plugin to your indexer?" + ); + } + + if (!('log' in ctx.logger)) { + throw new Error("Invalid logger instance in context"); + } return ctx.logger; }
1-30: Consider future configuration requirementsGiven the PR objective to support logger configuration through
apibara.config.ts, consider these architectural improvements:
- Add a configuration interface that can be extended later:
export interface LoggerConfig { reporter?: ConsolaReporter; level?: string; // Future config options customFormats?: unknown; destination?: unknown; }
- Modify the logger plugin to accept this configuration:
export function logger<T, B, P>(config: LoggerConfig = {})This will make it easier to add new configuration options without breaking changes.
packages/cli/src/runtime/internal/app.ts (3)
37-40: Consider adding type annotations for better type safetyThe logic for handling different indexer types is sound, but could benefit from explicit type annotations.
Consider adding type annotations:
- const definition = + const definition: IndexerDefinition = typeof indexerDefinition.indexer === "function" ? indexerDefinition.indexer(runtimeConfig) : indexerDefinition.indexer;
42-42: Consider enhancing plugin management robustnessWhile the current implementation works, consider these improvements:
- Validate for duplicate plugins
- Consider making plugin addition immutable
Here's a more robust approach:
- definition.plugins = [...(definition.plugins ?? []), logger()]; + const existingPlugins = definition.plugins ?? []; + const newDefinition = { + ...definition, + plugins: existingPlugins.some(p => p.name === 'logger') + ? existingPlugins + : [...existingPlugins, logger()] + };
37-44: Consider configuration flexibility for logger pluginThe current implementation adds the logger plugin globally to all indexers. As mentioned in the PR objectives, future configuration will be handled via
apibara.config.ts. Consider:
- Adding a mechanism to disable the logger for specific indexers
- Allowing logger configuration options to be passed through runtimeConfig
- Supporting different log levels per indexer
This will provide better flexibility when the configuration feature is implemented:
// Future apibara.config.ts example export default { logger: { enabled: true, level: 'info', // per-indexer settings indexers: { 'my-indexer': { level: 'debug' } } } }examples/cli/indexers/2-starknet.indexer.ts (2)
35-37: Enhance logging information for better debuggingWhile the basic logging is in place, consider including more context about the block being transformed, such as the block number and hash.
- logger.info("Transforming block ", endCursor); + logger.info("Transforming block", { + cursor: endCursor, + blockNumber: header?.blockNumber.toString(), + blockHash: header?.blockHash + });
Remove commented block data insertion code
The commented code is part of an example indexer implementation and appears to be unused. The code block is meant to demonstrate block data insertion but is currently inactive and should be removed since:
- The table 'test' is created with columns matching these fields (number, hash)
- No alternative block data insertion logic is implemented
- This is an example file (in examples/cli/indexers/) meant to demonstrate indexer functionality
- Keeping commented code reduces readability and may confuse users following the example
🔗 Analysis chain
Line range hint
39-42: Remove or implement commented codeThere's commented-out code that appears to be the actual block data insertion logic. This should either be implemented or removed if no longer needed.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if this insertion pattern is used elsewhere in the codebase ast-grep --pattern 'writer.insert([{ number: $$$, hash: $$$ }])'Length of output: 70
Script:
#!/bin/bash # Let's try a broader search for writer.insert patterns and commented code around it rg -U "writer\.insert.*?number.*?hash" -A 5 -B 5Length of output: 50
Script:
#!/bin/bash # Search for similar commented block data insertion patterns rg -U "//.*writer\.insert.*?number.*?hash" -A 5 -B 5Length of output: 54
Script:
#!/bin/bash # Let's also check the specific file for context cat "examples/cli/indexers/2-starknet.indexer.ts"Length of output: 1614
packages/indexer/package.json (1)
Line range hint
2-2: Consider version bump for new plugin addition.Adding a new plugin is a feature addition. Since the current version is
2.0.0-beta.19, consider bumping the version according to semver guidelines.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (8)
change/@apibara-indexer-95c6848e-72aa-429f-bbe9-556a867c123e.json(1 hunks)change/apibara-45ec1e62-bbea-48bc-ac29-b6ea1acc4024.json(1 hunks)examples/cli/indexers/1-evm.indexer.ts(1 hunks)examples/cli/indexers/2-starknet.indexer.ts(2 hunks)packages/cli/src/runtime/internal/app.ts(2 hunks)packages/indexer/build.config.ts(1 hunks)packages/indexer/package.json(1 hunks)packages/indexer/src/plugins/logger.ts(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- change/@apibara-indexer-95c6848e-72aa-429f-bbe9-556a867c123e.json
- change/apibara-45ec1e62-bbea-48bc-ac29-b6ea1acc4024.json
🔇 Additional comments (10)
examples/cli/indexers/1-evm.indexer.ts (2)
5-5: Consider using a more stable URL for examples
The preview environment URL might not be the best choice for example code as it could be subject to changes. Consider using a placeholder URL or adding a comment explaining this is for demonstration purposes only.
11-13: Review the transaction filter configuration
The empty transaction filter [{}] will match all transactions, which might not be optimal for performance. Consider:
- Adding comments to explain why all transactions are being captured
- Adding specific filters if only certain transactions are needed
✅ Verification successful
Let me gather more context about the usage and purpose of these filters by checking the surrounding code and documentation.
Let me check if there are any comments or documentation about the filter usage in the codebase.
Based on the context gathered, I can now provide the final response:
Empty transaction filter is intentional in example code
The empty transaction filter [{}] is a valid pattern used consistently across example code to capture all transactions when demonstrating basic indexer functionality. This is evidenced by:
- Similar usage in
examples/evm-client/src/main.tsfor demonstration purposes - Other example indexers showing how to add specific filters when needed (e.g.,
examples/indexer/src/indexer.tswithstrictandtopics)
The current implementation serves as a basic example and is appropriate in this context.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if this pattern is used consistently in other example indexers
rg -A 2 "transactions: \[\{\}\]" examples/
Length of output: 393
Script:
#!/bin/bash
# Check the full context of both files to understand the purpose
echo "=== examples/cli/indexers/1-evm.indexer.ts ==="
cat examples/cli/indexers/1-evm.indexer.ts
echo -e "\n=== examples/evm-client/src/main.ts ==="
cat examples/evm-client/src/main.ts
# Check if there are any comments or documentation about filters
echo -e "\n=== Searching for filter documentation ==="
rg -i "filter" examples/ -A 5 -B 5
Length of output: 139383
Script:
#!/bin/bash
# Search for any documentation about filters in markdown files or comments
echo "=== Searching for filter documentation in markdown files ==="
fd -e md | xargs rg -i "filter|transaction" -A 5 -B 5
echo -e "\n=== Searching for filter documentation in comments ==="
rg -g "!*.json" "filter|transaction" -A 5 -B 5 examples/
Length of output: 16439
packages/indexer/build.config.ts (2)
13-13: LGTM! The logger plugin entry is properly integrated.
The new entry for the logger plugin follows the established pattern and is correctly placed in the plugins section of the build configuration.
13-13: Verify the logger plugin implementation and exports.
Let's ensure the logger plugin file exists and is properly exported.
✅ Verification successful
Let me verify the logger plugin implementation with one more check to ensure it's properly implemented.
Logger plugin implementation and exports are properly configured
The verification shows that:
- The logger plugin file exists at the correct location
- It's properly exported in package.json with all necessary formats (CJS, MJS, and types)
- The implementation is complete with:
- A plugin factory function that configures the logger
- A
useLoggerhook for accessing the logger instance - Proper type exports from 'consola'
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the existence of the logger plugin file and its exports
# Check if the logger plugin file exists
echo "Checking for logger plugin file..."
fd "logger.ts" "packages/indexer/src/plugins"
# Check if the logger is properly exported in package.json
echo "Checking package exports..."
cat packages/indexer/package.json | jq '.exports["./plugins/logger"]'
# Look for logger plugin imports to ensure it's being used
echo "Checking logger plugin usage..."
rg -t ts "useLogger|from.*plugins/logger" "packages/indexer"
Length of output: 687
Script:
#!/bin/bash
# Check the implementation of the logger plugin
echo "Checking logger plugin implementation..."
cat packages/indexer/src/plugins/logger.ts
Length of output: 995
packages/indexer/src/plugins/logger.ts (1)
1-5: LGTM! Clean imports and type exports
The imports are well-organized and the type re-exports provide good TypeScript support for plugin consumers.
packages/cli/src/runtime/internal/app.ts (2)
2-2: LGTM: Logger import is properly structured
The import statement follows the project's module organization pattern, correctly importing from the plugins directory.
44-44: Verify error handling in _createIndexer
Ensure that _createIndexer properly handles potential errors from plugin initialization.
examples/cli/indexers/2-starknet.indexer.ts (2)
2-2: LGTM: Logger import is properly structured
The import statement follows the plugin architecture pattern and is correctly placed with other imports.
Line range hint 35-43: Verify error handling for endCursor
The transform function now accepts endCursor but there's no error handling if it's undefined. Consider adding validation or type checking.
packages/indexer/package.json (1)
55-60: LGTM! Verify dist files existence.
The export configuration for the logger plugin follows the established pattern and correctly defines all necessary module formats.
Let's verify the existence of the referenced dist files:
✅ Verification successful
Export configuration is correct and will be built properly
The logger plugin export configuration is valid and consistent with other plugins. The source file src/plugins/logger.ts exists and is properly included in the build configuration via unbuild. The build system will generate the necessary files in the dist directory with the correct formats (mjs, cjs, d.ts) matching the export paths.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that all referenced logger plugin files exist in the dist directory
fd --glob "dist/plugins/logger.{d.ts,mjs,cjs}" --type f
Length of output: 57
Script:
#!/bin/bash
# Let's check if the source files exist first
fd --glob "src/plugins/logger.{ts,js}" --type f
# Also check the build script to understand the dist structure
cat package.json | jq '.scripts.build'
# Check if there are any other plugins to compare the structure
fd --glob "src/plugins/*.{ts,js}" --type f
Length of output: 151
Script:
#!/bin/bash
# Let's check the entire src directory structure to understand the plugin organization
fd --type f --extension ts --extension js
# Let's also check if there are any build configuration files
fd --glob "{tsconfig.json,rollup.config.*,vite.config.*}" --type f
Length of output: 6263
Script:
#!/bin/bash
# Let's check the build configuration for the indexer package
cat packages/indexer/build.config.ts
# And check if there are any other plugin exports in package.json
cat packages/indexer/package.json | jq '.exports'
Length of output: 2484
| async transform({ endCursor }) { | ||
| console.log({ endCursor }); | ||
| }, |
There was a problem hiding this comment.
Use the new logger plugin instead of console.log
Since this PR introduces a logger plugin for standardizing logging, this example should demonstrate its usage instead of using console.log directly.
-async transform({ endCursor }) {
- console.log({ endCursor });
+async transform({ endCursor }, { logger }) {
+ logger.info('Processing block', { endCursor });
},📝 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.
| async transform({ endCursor }) { | |
| console.log({ endCursor }); | |
| }, | |
| async transform({ endCursor }, { logger }) { | |
| logger.info('Processing block', { endCursor }); | |
| }, |
| export function logger<TFilter, TBlock, TTxnParams>({ | ||
| logger, | ||
| }: { logger?: ConsolaReporter } = {}) { | ||
| return defineIndexerPlugin<TFilter, TBlock, TTxnParams>((indexer) => { | ||
| indexer.hooks.hook("run:before", () => { | ||
| const ctx = useIndexerContext(); | ||
|
|
||
| if (logger) { | ||
| ctx.logger = consola.create({ reporters: [logger] }); | ||
| } else { | ||
| ctx.logger = consola.create({}); | ||
| } | ||
| }); | ||
| }); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add cleanup, error handling, and type constraints
The logger plugin implementation could be improved in several areas:
- Add cleanup in "run:after" hook to prevent memory leaks
- Add error handling for logger creation
- Add type constraints for better type safety
- Add JSDoc documentation
Consider this implementation:
+/**
+ * Creates a logger plugin for the indexer.
+ * @param options - Plugin options
+ * @param options.logger - Optional custom consola reporter
+ * @returns Indexer plugin that provides logging capabilities
+ */
-export function logger<TFilter, TBlock, TTxnParams>({
+export function logger<
+ TFilter extends Record<string, unknown>,
+ TBlock extends Record<string, unknown>,
+ TTxnParams extends Record<string, unknown>
+>({
logger,
}: { logger?: ConsolaReporter } = {}) {
return defineIndexerPlugin<TFilter, TBlock, TTxnParams>((indexer) => {
indexer.hooks.hook("run:before", () => {
const ctx = useIndexerContext();
+ try {
if (logger) {
ctx.logger = consola.create({ reporters: [logger] });
} else {
ctx.logger = consola.create({});
}
+ } catch (error) {
+ throw new Error(`Failed to initialize logger: ${error.message}`);
+ }
});
+
+ indexer.hooks.hook("run:after", () => {
+ const ctx = useIndexerContext();
+ if (ctx.logger) {
+ // Clear any internal buffers or state
+ ctx.logger.clear();
+ }
+ });
});
}📝 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.
| export function logger<TFilter, TBlock, TTxnParams>({ | |
| logger, | |
| }: { logger?: ConsolaReporter } = {}) { | |
| return defineIndexerPlugin<TFilter, TBlock, TTxnParams>((indexer) => { | |
| indexer.hooks.hook("run:before", () => { | |
| const ctx = useIndexerContext(); | |
| if (logger) { | |
| ctx.logger = consola.create({ reporters: [logger] }); | |
| } else { | |
| ctx.logger = consola.create({}); | |
| } | |
| }); | |
| }); | |
| } | |
| /** | |
| * Creates a logger plugin for the indexer. | |
| * @param options - Plugin options | |
| * @param options.logger - Optional custom consola reporter | |
| * @returns Indexer plugin that provides logging capabilities | |
| */ | |
| export function logger< | |
| TFilter extends Record<string, unknown>, | |
| TBlock extends Record<string, unknown>, | |
| TTxnParams extends Record<string, unknown> | |
| >({ | |
| logger, | |
| }: { logger?: ConsolaReporter } = {}) { | |
| return defineIndexerPlugin<TFilter, TBlock, TTxnParams>((indexer) => { | |
| indexer.hooks.hook("run:before", () => { | |
| const ctx = useIndexerContext(); | |
| try { | |
| if (logger) { | |
| ctx.logger = consola.create({ reporters: [logger] }); | |
| } else { | |
| ctx.logger = consola.create({}); | |
| } | |
| } catch (error) { | |
| throw new Error(`Failed to initialize logger: ${error.message}`); | |
| } | |
| }); | |
| indexer.hooks.hook("run:after", () => { | |
| const ctx = useIndexerContext(); | |
| if (ctx.logger) { | |
| // Clear any internal buffers or state | |
| ctx.logger.clear(); | |
| } | |
| }); | |
| }); | |
| } |
2 participants
Add a new logger plugin to standardize how users can log information.
This is a simple wrapper around consola for now and will be used later
to make the logger configurable by the user in
apibara.config.ts.