Add CLI command to set log-forwarding config #225
Conversation
This file contains 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
brasewel
requested changes
Mar 19, 2025
| @@ -0,0 +1,250 @@ | |||
| /* | |||
There was a problem hiding this comment.
This class should be changed to set-log-forwarding-newrelic-.test.js as it is testing newrelic
| }); | ||
|
|
||
| // For MVP, only New Relic is supported | ||
| const destinations = ['newrelic']; |
There was a problem hiding this comment.
Please use an enum for the destination and make it so that we can extend this functionality for other destinations like splunk
| // For MVP, only New Relic is supported | ||
| const destinations = ['newrelic']; | ||
|
|
||
| let meshId = null; |
There was a problem hiding this comment.
I don't think we need to set meshid to null and as a variable as you are getting the meshid in line 52. Changing it to a const is better practice
| if (shouldContinue) { | ||
| try { | ||
| await setLogForwarding(imsOrgCode, projectId, workspaceId, { | ||
| destination: 'newrelic', |
|
|
||
| if (shouldContinue) { | ||
| try { | ||
| await setLogForwarding(imsOrgCode, projectId, workspaceId, { |
There was a problem hiding this comment.
I think we should make this code extensible so that we can support other destinations besides newrelic.
|
|
||
| return { | ||
| success: true, | ||
| destination: 'newrelic', |
| this.log('Log forwarding details set successfully.'); | ||
|
|
||
| return { | ||
| success: true, |
There was a problem hiding this comment.
Where is this success variable used?
| } catch (error) { | ||
| this.log(error.message); | ||
| this.error( | ||
| `Failed to set log forwarding details. Please try again. RequestId: ${global.requestId}`, |
There was a problem hiding this comment.
I think if this fails, we should provide a descriptive message of why it failed or else the user may find it confusing why it failed
jhadobe
approved these changes
Mar 19, 2025
|
|
||
| if (licenseKey.length !== 40) { | ||
| this.error( | ||
| `License key has wrong format. Expected: 40 characters (received: ${licenseKey.length})`, |
There was a problem hiding this comment.
Suggested change
| `License key has wrong format. Expected: 40 characters (received: ${licenseKey.length})`, | |
| `The license key is in the wrong format. Expected: 40 characters (received: ${licenseKey.length})`, |
| }, | ||
| }); | ||
|
|
||
| this.log('Log forwarding details set successfully.'); |
There was a problem hiding this comment.
Suggested change
| this.log('Log forwarding details set successfully.'); | |
| this.log('Log forwarding successful.'); |
| } catch (error) { | ||
| this.log(error.message); | ||
| this.error( | ||
| `Failed to set log forwarding details. Please try again. RequestId: ${global.requestId}`, |
There was a problem hiding this comment.
Suggested change
| `Failed to set log forwarding details. Please try again. RequestId: ${global.requestId}`, | |
| `Failed to set log forwarding details. Try again. RequestId: ${global.requestId}`, |
Comment on lines
137
to
140
| SetLogForwardingCommand.description = `Set log forwarding destination for API mesh. | ||
| - Select a log forwarding destination: Choose from available options ( example : newrelic). | ||
| - Enter the base URI: Provide the URI for the log forwarding service. Ensure it includes the protocol ( example : \`https://\`). | ||
| - Enter the license key: Provide the license key for authentication with the log forwarding service. The key must be 40 characters long.`; |
There was a problem hiding this comment.
Suggested change
| SetLogForwardingCommand.description = `Set log forwarding destination for API mesh. | |
| - Select a log forwarding destination: Choose from available options ( example : newrelic). | |
| - Enter the base URI: Provide the URI for the log forwarding service. Ensure it includes the protocol ( example : \`https://\`). | |
| - Enter the license key: Provide the license key for authentication with the log forwarding service. The key must be 40 characters long.`; | |
| SetLogForwardingCommand.description = `Sets the log forwarding destination for API mesh. | |
| - Select a log forwarding destination: Choose from available options ( example : newrelic). | |
| - Enter the base URI: Provide the URI for the log forwarding service. Ensure it includes the protocol ( example : \`https://www.adobe.com\`). | |
| - Enter the license key: Provide the license key for authentication with the log forwarding service. The key must be 40 characters long.`; |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This PR closes https://jira.corp.adobe.com/browse/CEXT-4231
Description
Related Issue
Motivation and Context
How Has This Been Tested?
Screenshots (if appropriate):
Types of changes
Checklist: