New Components - splunk #15966
New Components - splunk #15966
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 3 Skipped Deployments
|
WalkthroughThis pull request introduces a series of new Splunk modules, including action modules (create-event, run-search, get-search-job-status) and source modules (new-alert-fired, new-search-event, new-search-result, along with a common base). The Splunk application module has been expanded with new property definitions and methods for enhanced interaction with the Splunk API. Additionally, the package configuration has been updated to increase the version and add new dependencies. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CreateEvent as Create Event Action
participant SplunkAPI as Splunk API
User->>CreateEvent: Trigger "Create Event" action
CreateEvent->>SplunkAPI: Call sendEvent(index, eventData, source, sourcetype)
SplunkAPI-->>CreateEvent: Return success response
CreateEvent-->>User: Emit summary message
sequenceDiagram
participant Scheduler
participant NewAlert as New Alert Source
participant SplunkSys as Splunk App
Scheduler->>NewAlert: Poll for fired alerts
NewAlert->>SplunkSys: Call listFiredAlerts()
SplunkSys-->>NewAlert: Return alert data
NewAlert->>NewAlert: Generate metadata for each alert
NewAlert-->>Scheduler: Emit alert events
Assessment against linked issues
Suggested labels
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 ESLint
components/splunk/sources/new-alert-fired/new-alert-fired.mjsOops! Something went wrong! :( ESLint: 8.57.1 Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'jsonc-eslint-parser' imported from /eslint.config.mjs components/splunk/splunk.app.mjsOops! Something went wrong! :( ESLint: 8.57.1 Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'jsonc-eslint-parser' imported from /eslint.config.mjs ✨ Finishing Touches
🪧 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: 5
🧹 Nitpick comments (13)
components/splunk/actions/create-event/create-event.mjs (2)
44-57: Add error handling to the run methodConsider adding try/catch block around the API call to handle potential errors gracefully and provide better error messages to users.
async run({ $ }) { + try { const response = await this.splunk.sendEvent({ $, selfSigned: this.selfSigned, params: { index: this.indexName, source: this.source, sourcetype: this.sourcetype, }, data: this.eventData, }); $.export("$summary", `Event sent to index ${this.indexName} successfully`); return response; + } catch (error) { + $.export("$summary", `Failed to send event: ${error.message}`); + throw error; + } },
55-55: Include more details in the success summaryConsider adding more context to the success message to help users understand what was sent.
- $.export("$summary", `Event sent to index ${this.indexName} successfully`); + $.export("$summary", `Event sent to index "${this.indexName}" successfully${this.source ? ` with source "${this.source}"` : ""}`);components/splunk/actions/get-search-job-status/get-search-job-status.mjs (1)
27-35: Add error handling to the run methodConsider adding try/catch block around the API call to handle potential errors gracefully and provide better error messages to users.
async run({ $ }) { + try { const response = await this.splunk.getSearchJobStatus({ $, selfSigned: this.selfSigned, jobId: this.jobId, }); $.export("$summary", `Successfully retrieved status for job ID ${this.jobId}`); return response; + } catch (error) { + $.export("$summary", `Failed to retrieve job status: ${error.message}`); + throw error; + } },components/splunk/actions/run-search/run-search.mjs (3)
32-32: Fix formatting in latestTime descriptionThere's extra leading whitespace in the latestTime description that should be removed for consistency with the earliestTime description.
- description: " Specify a time string. Sets the latest (exclusive), respectively, time bounds for the search. The time string can be either a UTC time (with fractional seconds), a relative time specifier (to now) or a formatted time string. Refer to [Time modifiers](https://docs.splunk.com/Documentation/Splunk/9.4.1/SearchReference/SearchTimeModifiers) for search for information and examples of specifying a time string.", + description: "Specify a time string. Sets the latest (exclusive), respectively, time bounds for the search. The time string can be either a UTC time (with fractional seconds), a relative time specifier (to now) or a formatted time string. Refer to [Time modifiers](https://docs.splunk.com/Documentation/Splunk/9.4.1/SearchReference/SearchTimeModifiers) for search for information and examples of specifying a time string.",
36-48: Add error handling to the run methodConsider adding try/catch block around the API call to handle potential errors gracefully and provide better error messages to users.
async run({ $ }) { + try { const response = await this.splunk.executeSearchQuery({ $, selfSigned: this.selfSigned, data: { search: this.query, earliest_time: this.earliestTime, latest_time: this.latestTime, }, }); $.export("$summary", `Executed Splunk search query: ${this.query}`); return response; + } catch (error) { + $.export("$summary", `Failed to execute search query: ${error.message}`); + throw error; + } },
46-46: Include more context in success summaryConsider adding more information to the success message, such as the time range if specified.
- $.export("$summary", `Executed Splunk search query: ${this.query}`); + $.export("$summary", `Executed Splunk search query: ${this.query}${this.earliestTime ? ` from ${this.earliestTime}` : ""}${this.latestTime ? ` to ${this.latestTime}` : ""}`);components/splunk/sources/new-search-result/new-search-result.mjs (1)
13-19: Consider enhancing the metadata generation with result timestamps.The
generateMetamethod currently usesDate.now()for the timestamp, which represents when the event was processed rather than when it occurred. If the Splunk result contains its own timestamp, consider using that for more accurate event sequencing.generateMeta(result) { return { id: result.sid, summary: `New Search Results with SID: ${result.sid}`, - ts: Date.now(), + ts: result._time ? new Date(result._time).getTime() : Date.now(), }; },components/splunk/sources/new-search-event/new-search-event.mjs (1)
14-20: Enhance event metadata for better information and deduplication.The current metadata generation has several areas for improvement:
- The summary is generic and could be more informative
- Using MD5 of the entire event might not be the most efficient approach if a unique field is available
- The timestamp should ideally come from the event itself when available
generateMeta(event) { + // Extract a more meaningful summary from the event if possible + const eventName = event.name || event._raw?.slice(0, 30) || "Unknown"; + + // Use a specific field if available, or fall back to MD5 hash + const eventId = event.id || event._cd || md5(JSON.stringify(event)); + + // Use event timestamp if available, fall back to current time + const eventTime = event._time ? new Date(event._time).getTime() : Date.now(); + return { - id: md5(JSON.stringify(event)), - summary: "New Search Event", - ts: Date.now(), + id: eventId, + summary: `New Search Event: ${eventName}`, + ts: eventTime, }; },components/splunk/sources/new-alert-fired/new-alert-fired.mjs (1)
13-19: Use alert timestamp for more accurate event sequencing.The current implementation uses the current time for the event timestamp rather than the actual time when the alert was triggered. If the alert object contains its own timestamp, it would be more accurate to use that.
generateMeta(alert) { + // Use the alert's trigger time if available + const alertTime = alert.trigger_time || alert.triggered_at || Date.now(); return { id: alert.id, summary: `New Alert Fired: ${alert.name}`, - ts: Date.now(), + ts: alertTime, }; },components/splunk/splunk.app.mjs (4)
8-74: Fix minor spelling issue in the documentation.In the description for
query(line 73), "sytax" should be "syntax". This helps keep the documentation polished and clear.- description: "The Splunk search query. Example: `search *`. [See the documentation](https://docs.splunk.com/Documentation/Splunk/9.4.1/SearchReference/Search) for more information about search command sytax." + description: "The Splunk search query. Example: `search *`. [See the documentation](https://docs.splunk.com/Documentation/Splunk/9.4.1/SearchReference/Search) for more information about search command syntax."
65-69: Document security implications.While allowing self-signed certificates (via
selfSigned) is sometimes necessary, it poses security risks in production environments. Consider providing a cautionary note in user-facing documentation to emphasize these risks and encourage secure certificates whenever possible.
105-128: Use consistent naming in resource methods.While the
listJobs,listIndexes,listSavedSearches, andlistFiredAlertsmethods have uniform naming, consider a shared pattern for request parameters (e.g., passingoptsconsistently). This keeps the API predictable.
160-166: Validate input before sending events.
sendEventposts data to Splunk. Consider adding minimal validation (e.g., verifying a non-empty payload) to prevent unintended or malformed events.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (9)
components/splunk/actions/create-event/create-event.mjs(1 hunks)components/splunk/actions/get-search-job-status/get-search-job-status.mjs(1 hunks)components/splunk/actions/run-search/run-search.mjs(1 hunks)components/splunk/package.json(2 hunks)components/splunk/sources/common/base.mjs(1 hunks)components/splunk/sources/new-alert-fired/new-alert-fired.mjs(1 hunks)components/splunk/sources/new-search-event/new-search-event.mjs(1 hunks)components/splunk/sources/new-search-result/new-search-result.mjs(1 hunks)components/splunk/splunk.app.mjs(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Verify TypeScript components
- GitHub Check: pnpm publish
- GitHub Check: Publish TypeScript components
🔇 Additional comments (9)
components/splunk/package.json (2)
3-3: Version update looks goodThe version bump from 0.0.1 to 0.1.0 follows semantic versioning practices, correctly indicating this is a minor feature update rather than just a patch.
15-18:✅ Verification successful
Verify necessity of the 'https' dependency
The 'https' module is a native Node.js module that should be available without explicit installation. Consider removing this dependency unless you're using a specific version or feature not available in the native module.
Additionally, note that MD5 is considered cryptographically weak for security-sensitive applications. If you're using it for non-security purposes like generating unique identifiers, that's acceptable.
🏁 Script executed:
#!/bin/bash # Check if the 'https' module is used in a non-standard way in the codebase echo "Checking for custom usage of 'https' module..." rg -A 3 -B 3 "import.*https" --type jsLength of output: 2741
Action: Remove explicit installation of the native 'https' module
After verifying the codebase, there’s no evidence of any non-standard or custom usage of the 'https' module that would justify keeping its explicit installation in the dependencies. Since Node.js provides 'https' natively, it's safe to remove it from
components/splunk/package.jsonunless a specific version or extended functionality is actually required. Additionally, please ensure that the use of MD5 (if any) is limited to non-security-related purposes, due to its known cryptographic weaknesses.
- File:
components/splunk/package.json(Lines 15–18)components/splunk/actions/create-event/create-event.mjs (1)
1-58: Well-structured new action moduleThe Create Event action is well-organized, following Pipedream's component patterns with appropriate documentation links, clear property definitions, and a clean implementation.
components/splunk/actions/get-search-job-status/get-search-job-status.mjs (1)
1-36: Well-structured new action moduleThe Get Search Job Status action is well-organized, following Pipedream's component patterns with appropriate documentation links, clear property definitions, and a clean implementation.
components/splunk/actions/run-search/run-search.mjs (1)
1-49: Well-structured new action moduleThe Run Search action is well-organized, following Pipedream's component patterns with appropriate documentation links, clear property definitions, and a clean implementation.
components/splunk/sources/new-alert-fired/new-alert-fired.mjs (1)
21-38:✅ Verification successful
Limit and filter alert processing for better performance.
The current implementation collects all alerts without any filtering or limits, which could lead to processing a large number of alerts and impact performance.
async run() { + // Use pagination parameters to limit and filter alerts + const paginationParams = { + count: 20, // Process a reasonable number of alerts + offset: 0, + // Only retrieve relatively recent alerts + timeRange: "-24h", + }; + const results = this.splunk.paginate({ resourceFn: this.splunk.listFiredAlerts, args: { selfSigned: this.selfSigned, + ...paginationParams, }, }); const alerts = []; + let count = 0; for await (const item of results) { alerts.push(item); + count++; + // Additional safeguard to prevent processing too many alerts + if (count >= 50) break; } alerts.forEach((alert) => { const meta = this.generateMeta(alert); this.$emit(alert, meta); }); },
🏁 Script executed:
#!/bin/bash # Check if the Splunk listFiredAlerts method supports pagination and time range parameters rg -A 10 "listFiredAlerts\s*\([^)]*\)\s*{" components/splunk/Length of output: 189
Below is a revised shell script using a corrected regex pattern that should properly match the definition of the
listFiredAlertsmethod. Please run the script to manually verify whether this method supports additional pagination parameters (likecount,offset, and atimeRange) as suggested in the review comment.Next Steps:
- Run the revised shell script to check the method’s signature.
- Based on the output, verify that the Splunk API indeed supports the additional parameters before proceeding with the refactor.
Once you confirm the method supports these parameters, you can safely adopt the refactoring changes suggested in the review.
🏁 Script executed:
#!/bin/bash # Corrected regex pattern to search for the definition of listFiredAlerts rg -A 10 'listFiredAlerts\s*\([^)]*\)\s*\{' components/splunk/Length of output: 827
Action Required: Confirm Pagination Parameters in Alert Processing
After verifying the implementation of
listFiredAlertsincomponents/splunk/splunk.app.mjs, we see that the method accepts anoptsobject and spreads its properties into the request. This confirms that passing additional options (including pagination parameters likecount,offset, andtimeRange) is supported.
Verified Location:
components/splunk/splunk.app.mjs– ThelistFiredAlerts(opts = {})method spreadsoptsinto the request.Key Considerations:
- The suggested approach to limit and filter alerts by spreading
paginationParamsinto the arguments oflistFiredAlertsis valid.- The additional safeguard in the loop (using a counter and breaking after 50 alerts) is an extra measure to protect against excessive processing.
Please ensure that the backend API correctly interprets these pagination parameters. Once confirmed, you can incorporate the diff snippet below to improve performance by limiting alert processing:
async run() { + // Use pagination parameters to limit and filter alerts + const paginationParams = { + count: 20, // Process a reasonable number of alerts at once + offset: 0, + // Only retrieve alerts from the past 24 hours + timeRange: "-24h", + }; const results = this.splunk.paginate({ resourceFn: this.splunk.listFiredAlerts, args: { selfSigned: this.selfSigned, + ...paginationParams, }, }); const alerts = []; + let count = 0; for await (const item of results) { alerts.push(item); + count++; + // Additional safeguard to prevent processing too many alerts + if (count >= 50) break; } alerts.forEach((alert) => { const meta = this.generateMeta(alert); this.$emit(alert, meta); }); },components/splunk/splunk.app.mjs (3)
80-104: Review certificate-rejection override for potential security impact.The
_makeRequest()method setsrejectUnauthorized: falseifselfSignedis true. This can allow MITM attacks. If this is only for local dev or test scenarios, confirm it’s properly documented or restricted from production usage.
129-159: Confirm request methods.
executeSearchQueryusesPOST, butgetSearchJobStatus,getSearchResults, andgetSearchEventsuseGETby default. Ensure both the method and endpoint path align with Splunk’s expectations (e.g., older Splunk versions might differ in endpoints).
77-79:❓ Verification inconclusive
Ensure base URL correctness.
The
_baseUrl()method constructs a URL that appends:${this.$auth.api_port}. Confirm that user-supplied values don’t inadvertently include a port, which could cause malformed URLs.
🏁 Script executed:
#!/bin/bash # Searching for references to `api_url` in the repo to confirm usage consistency. rg -A 3 '\$auth\.api_url'Length of output: 2818
Attention: Validate API URL Inputs in Splunk App
Please double-check that the value provided in
this.$auth.api_urldoes not already include a port. Unlike other components that simply append additional path segments, the Splunk app explicitly concatenates:${this.$auth.api_port}to the URL when constructing the base endpoint. If the API URL already embeds a port, it could yield a malformed URL (e.g., duplicating port information). Ensure that any user-supplied values are sanitized or validated accordingly.
- Location:
components/splunk/splunk.app.mjs(Lines 77-79)
components/splunk/sources/new-search-result/new-search-result.mjs
Outdated
Show resolved
Hide resolved
components/splunk/sources/new-search-event/new-search-event.mjs
Outdated
Show resolved
Hide resolved
|
Hi @michelle0927, could you remove the Self-signed prop on each action/source and use the variable |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
components/splunk/sources/common/base.mjs (1)
16-25: 🛠️ Refactor suggestionEnhance getRecentJobIds() to Filter Jobs
The current implementation fetches all jobs, which can degrade performance. To mitigate this, update the method to only fetch jobs within a recent time window and limit the number of results.
async getRecentJobIds() { + // Add parameters to filter jobs by time and limit results + const timeRange = { + // Default to jobs from the last hour + earliestTime: "-1h", + latestTime: "now", + }; const results = this.splunk.paginate({ resourceFn: this.splunk.listJobs, args: { + ...timeRange, + count: 20, // Limit the number of jobs to process + // Use the auth property instead of individual selfSigned parameters }, }); const jobIds = []; for await (const job of results) { jobIds.push(job.content.sid); } return jobIds; },
🧹 Nitpick comments (4)
components/splunk/sources/new-alert-fired/test-event.mjs (1)
1-30: Consider anonymizing potentially sensitive information in test data.The test event contains what appears to be a real IP address (
44.210.81.125) in thehostfield. While this may be intentional for testing, it's generally better practice to use reserved IP ranges for documentation and test data (like192.0.2.x,198.51.100.x, or203.0.113.xfrom TEST-NET ranges).Also note that several fields like
sid,search_name, andownerare empty. This is likely intentional for a test event, but ensure these will be populated correctly in production.components/splunk/actions/run-search/run-search.mjs (2)
18-25: Add error handling to run method.The current implementation doesn't include any error handling. If the Splunk API call fails, the action will fail without providing clear error information to the user.
Add try/catch block to handle potential errors:
async run({ $ }) { + try { const response = await this.splunk.executeSearchQuery({ $, name: this.name, }); $.export("$summary", `Executed Splunk search: ${this.name}`); return response; + } catch (error) { + $.export("$summary", `Failed to execute Splunk search: ${this.name}`); + throw error; + } },
11-16: Prop name doesn't match propDefinition name.There might be confusion between the prop name
nameand the propDefinition namesavedSearchName. Consider using consistent naming to improve code clarity.name: { propDefinition: [ splunk, "savedSearchName", ], + // Optional: Add a label or description to clarify this represents the saved search name },components/splunk/sources/new-search-result/new-search-result.mjs (1)
23-29: Improve generateMeta method with more meaningful identification.The current implementation uses
result.idwhich may not exist or be meaningful. Consider using more specific identifiers from the Splunk job content.generateMeta(result) { return { - id: result.id, + id: result.content?.sid || result.id, - summary: `New Search with ID: ${result.id}`, + summary: `New Search Results: ${result.content?.label || result.content?.sid || 'Unknown'}`, ts: Date.now(), }; },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
components/splunk/actions/create-event/create-event.mjs(1 hunks)components/splunk/actions/get-search-job-status/get-search-job-status.mjs(1 hunks)components/splunk/actions/run-search/run-search.mjs(1 hunks)components/splunk/sources/common/base.mjs(1 hunks)components/splunk/sources/new-alert-fired/new-alert-fired.mjs(1 hunks)components/splunk/sources/new-alert-fired/test-event.mjs(1 hunks)components/splunk/sources/new-search-event/new-search-event.mjs(1 hunks)components/splunk/sources/new-search-result/new-search-result.mjs(1 hunks)components/splunk/splunk.app.mjs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- components/splunk/actions/get-search-job-status/get-search-job-status.mjs
- components/splunk/actions/create-event/create-event.mjs
- components/splunk/sources/new-search-event/new-search-event.mjs
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: pnpm publish
- GitHub Check: Verify TypeScript components
- GitHub Check: Publish TypeScript components
🔇 Additional comments (8)
components/splunk/actions/run-search/run-search.mjs (1)
18-24:✅ Verification successful
Implement
selfSignedproperty as requested in PR comments.Per the PR comments, you should use
this.splunk.$auth.self_signedwithin the application instead of setting theselfSignedproperty on each action or source. This will streamline the process and allow end-users to specify the option only once.Update the
executeSearchQuerycall to use the auth property:const response = await this.splunk.executeSearchQuery({ $, name: this.name, + // Use the auth property instead of individual selfSigned parameters + // as requested in PR comments by vunguyenhung });Let's verify if the
executeSearchQuerymethod in the Splunk app already handles theself_signedproperty from auth:
🏁 Script executed:
#!/bin/bash # Check how executeSearchQuery is implemented and if it uses $auth.self_signed rg -A 10 "executeSearchQuery\s*\(" components/splunk/splunk.app.mjsLength of output: 310
Action: Use global auth for self-signed configuration in Splunk search actions
Based on our verification, the
executeSearchQueryimplementation incomponents/splunk/splunk.app.mjssimply spreads any options provided into its request. This confirms that the function will honor any self-signed configuration if passed via the global auth (this.splunk.$auth.self_signed). Therefore, instead of supplying aselfSignedparameter locally, you should rely on the auth property.
- In
components/splunk/actions/run-search/run-search.mjs(lines 18‑24), remove any localselfSignedhandling and ensure thatexecuteSearchQueryis called without an explicit selfSigned parameter.- Confirm that
this.splunk.$auth.self_signedis set up globally so that it applies to all requests without duplication.Please update the code accordingly and verify that the auth property is correctly configured in the Splunk module.
components/splunk/sources/new-search-result/new-search-result.mjs (1)
31-47: Improve error handling and consider result limits.The current implementation lacks error handling when fetching search results and doesn't limit the number of jobs processed, which could impact performance.
async run() { - const jobs = await this.getRecentJobs(); + try { + // Limit the number of jobs to process to avoid performance issues + const jobs = await this.getRecentJobs(); + const jobsToProcess = jobs.slice(0, 10); // Process only the 10 most recent jobs - for (const job of jobs) { + for (const job of jobsToProcess) { if (job.content?.resultCount > 0) { + try { const { results } = await this.splunk.getSearchResults({ jobId: job.content.sid, + // Use the auth property instead of individual selfSigned parameters }); if (results) { job.results = results; } + } catch (error) { + console.log(`Error retrieving results for job ${job.content.sid}:`, error.message); + } } } - jobs.forEach((result) => { + jobsToProcess.forEach((result) => { const meta = this.generateMeta(result); this.$emit(result, meta); }); + } catch (error) { + console.error("Error processing search results:", error); + } },components/splunk/sources/new-alert-fired/new-alert-fired.mjs (1)
1-36: LGTM! Good implementation of the Splunk alert source.The source module is well-structured with good separation of concerns between the metadata generation and event processing. It correctly follows the pattern of emitting events with metadata and has basic validation to prevent processing empty requests.
components/splunk/splunk.app.mjs (5)
1-3: Verify the https module usageThe
httpsmodule is imported and used for creating an agent to handle self-signed certificates. This corrects the previous comment flagging it as an unused import.
8-61: Well-structured prop definitions with dynamic options.The prop definitions are well organized and include descriptive labels and documentation links. I particularly like the implementation of the async options for dropdowns that fetch data from Splunk.
63-89: Good implementation of centralized request handling with self-signed cert support.The
_makeRequestmethod properly centralizes the API request logic and leveragesthis.$auth.self_signedfor handling self-signed certificates, which aligns with the suggestion from vunguyenhung to use this auth property instead of setting it on individual components.
90-153: Well-organized API methods with consistent patterns.The methods for interacting with the Splunk API follow a consistent pattern, making the code more maintainable. Each method properly leverages the centralized
_makeRequestfunction with appropriate path and parameter handling.
154-181: Enhance error handling in pagination logic.If
paging.totalorentryare missing from the API response, this code could throw errors. Consider adding validation or try/catch to handle unexpected responses from Splunk.async *paginate({ resourceFn, args, max, }) { args = { ...args, params: { ...args?.params, count: DEFAULT_LIMIT, }, }; let hasMore, count = 0; try { do { const { entry = [], paging = { total: 0 }, } = await resourceFn(args); for (const item of entry) { yield item; count++; if (max && count >= max) { return; } } hasMore = paging.total > count; + // Ensure offset is initialized if it doesn't exist + args.params.offset = (args.params.offset || 0) + args.params.count; - args.params.offset += args.params.count; } while (hasMore); + } catch (err) { + console.error("Error during pagination:", err); + throw err; + } }
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
components/splunk/splunk.app.mjs (2)
9-61: Consider extracting shared logic in prop definitions.
Each prop definition (e.g.jobId,indexName,savedSearchName) fetches lists from Splunk with nearly identical logic. You could reduce repetition by creating a helper function that accepts a resource-fetching method (likelistJobs,listIndexes, etc.) and returns the mapped results.
59-59: Correct the “sytax” typo.- description: "The Splunk search query. Example: `search *`. [See the documentation](... more information about search command sytax.", + description: "The Splunk search query. Example: `search *`. [See the documentation](... more information about search command syntax.",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
components/splunk/sources/new-alert-fired/new-alert-fired.mjs(1 hunks)components/splunk/splunk.app.mjs(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: pnpm publish
- GitHub Check: Publish TypeScript components
- GitHub Check: Verify TypeScript components
🔇 Additional comments (5)
components/splunk/splunk.app.mjs (3)
1-3: Imports look good.
No issues with using both@pipedream/platform’saxiosand the built-inhttpsmodule since the latter is leveraged for custom SSL handling.
84-88: Be cautious with ignoring SSL verification.
Disabling certificate checks can expose users to potential man-in-the-middle attacks. Confirm this is strictly for testing or trusted internal networks.
171-198: Enhance error handling in the pagination logic.
Ifpaging.totalorentryare missing, the code could break. Consider a try/catch or validation of Splunk responses to improve robustness.components/splunk/sources/new-alert-fired/new-alert-fired.mjs (2)
49-56: Validate_timeingenerateMeta().
Ifalert.result._timeis missing or not numeric, you could end up withNaN. Consider gracefully handling invalid or unexpected data to avoid partial or misleading results.
58-66: The “run” method looks good.
Emitting the alert data with metadata is straightforward. No additional concerns here.
|
/approve |
Resolves #15893
Note to QA (@vunguyenhung ): Set the
selfSignedprop totruefor testing.Summary by CodeRabbit
New Features
Chores