fix: logger-not-working-in-plugins #36231

AnnaHariprasad5123 · 2024-09-10T12:42:30Z

Issue :

Missing Logging Implementation :

Without a logging implementation (such as SLF4J Simple or Logback) in the project's classpath, the logging statements in the plugins cannot be executed.
As a result, no log output is being printed to the terminal.

Solution :

The solution is to add a logging implementation to the plugins parent pom. In this case, you can add the slf4j-simple dependency to your pom.xml file. This will provide a simple logging implementation that will output log statements to the console.

<dependency>
<groupId>org.slf4j</groupId>
<artifactId>slf4j-api</artifactId>
<version>1.7.36</version>
</dependency>

<dependency>
    <groupId>org.slf4j</groupId>
    <artifactId>slf4j-simple</artifactId>
    <version>1.7.36</version> 
</dependency>

Explanation:

slf4j-api provides the SLF4J API, which is the interface for logging.
provides the SLF4J API, which is the interface for logging.
slf4j-simple provides a simple implementation of the SLF4J API, which is responsible for actually printing the log messages to the console.

Screenshots :

Amazon S3 Plugin and Postgres Plugin

Summary by CodeRabbit

New Features
- Enhanced logging capabilities with the integration of SLF4J API and SLF4J Simple implementations.
Improvements
- Improved log management and output formatting for better monitoring and debugging across various plugins, transitioning from standard output to structured logging.
- Refined logging practices in multiple plugins to support better maintainability and performance.
- Removed method for console logging from the Stopwatch class to streamline logging practices.

coderabbitai · 2024-09-10T12:42:39Z

Walkthrough

This pull request introduces two new dependencies to the pom.xml file for the Appsmith plugins server project. The additions include the SLF4J API and SLF4J Simple implementations, both at version 1.7.36. These changes aim to enhance the logging functionality within the application, addressing previously reported issues regarding the visibility of log statements in the console. Additionally, the logging mechanism across various plugins has been updated to utilize structured logging instead of standard output.

Changes

Files	Change Summary
`app/server/appsmith-plugins/pom.xml`	Added dependencies for `org.slf4j:slf4j-api:1.7.36` and `org.slf4j:slf4j-simple:1.7.36`
`app/server/appsmith-plugins/*Plugin.java`	Replaced `System.out.println` with `log.debug` for structured logging across multiple plugins.

Assessment against linked issues

Objective	Addressed	Explanation
Logger does not work from the /appsmith-plugins directory (#[36073])	❌	The issue persists as the visibility of log statements in the console is not confirmed.

🎉 In the realm of code, a change so bright,
New logging tools bring clarity and light.
With SLF4J now in play,
Logs will shine, come what may,
Debugging woes take flight,
As we code through day and night! 🌟

Recent review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 78483cc and 2bdf166.

Files selected for processing (1)

app/server/appsmith-plugins/pom.xml (1 hunks)

Files skipped from review as they are similar to previous changes (1)

app/server/appsmith-plugins/pom.xml

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?

Share

Tips

Chat

There are 3 ways to chat with CodeRabbit:

Review comments: Directly reply to a review comment made by CodeRabbit. Example:
-- I pushed a fix in commit <commit_id>, please review it.
-- Generate unit testing code for this file.
- Open a follow-up GitHub issue for this discussion.
Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
-- @coderabbitai generate unit testing code for this file.
-- @coderabbitai modularize this function.
PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
-- @coderabbitai generate interesting stats about this repository and render them as a table.
-- @coderabbitai read src/utils.ts and generate unit testing code.
-- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
-- @coderabbitai help me debug CodeRabbit configuration file.

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)

@coderabbitai pause to pause the reviews on a PR.
@coderabbitai resume to resume the paused reviews.
@coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
@coderabbitai full review to do a full review from scratch and review all the files again.
@coderabbitai summary to regenerate the summary of the PR.
@coderabbitai resolve resolve all the CodeRabbit review comments.
@coderabbitai configuration to show the current CodeRabbit configuration for the repository.
@coderabbitai help to get help.

Other keywords and placeholders

Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
Add @coderabbitai anywhere in the PR title to generate the title automatically.

Documentation and Community

Visit our Documentation for detailed information on how to use CodeRabbit.
Join our Discord Community to get help, request features, and share feedback.
Follow us on X/Twitter for updates and announcements.

coderabbitai

Actionable comments posted: 0

Outside diff range, codebase verification and nitpick comments (1)

app/server/appsmith-plugins/pom.xml (1)

69-73: Nice work adding the SLF4J Simple dependency! 🙌

The SLF4J Simple dependency provides a basic logging implementation that outputs logs to the console. This is a good starting point and will allow you to see the logs during development and testing.

However, for a production application, you might want to consider using a more robust logging implementation like Logback or Log4j2. These logging frameworks offer advanced features such as log rotation, log filtering, and the ability to write logs to different destinations (e.g., files, databases).

But for now, the SLF4J Simple dependency is a good choice to get started with logging. Well done! 😊

Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 294503b and d9a696a.

Files selected for processing (1)

app/server/appsmith-plugins/pom.xml (1 hunks)

Additional comments not posted (1)

app/server/appsmith-plugins/pom.xml (1)

64-68: Great job adding the SLF4J API dependency! 👍

Adding the SLF4J API is an excellent choice. It provides a standardized logging interface, allowing you to decouple your application from the specific logging implementation. This dependency will enable you to use the SLF4J API for logging throughout your application, making it easier to manage and maintain logging statements.

Keep up the good work! 🌟

NilanshBansal · 2024-09-10T14:08:22Z

@AnnaHariprasad5123 thanks for this fix!
Can you please also update the System.out.println() statements to log.debug() in the Plugins.
This was recently changed in this PR #36077

AnnaHariprasad5123 · 2024-09-10T16:39:20Z

Hi @NilanshBansal, I replaced all System.out.println to log.debug. Could you check once.

AnnaHariprasad5123 · 2024-09-11T04:29:34Z

Hi @NilanshBansal, Could you check once. Let me know if any changes required.

NilanshBansal · 2024-09-11T04:58:18Z

@AnnaHariprasad5123 can you also remove the method stopAndLogTimeInMillisWithSysOut() from Stopwatch.java and also replace all the usages of this method with stopAndLogTimeInMillis(), since it was added as a temporary fix

https://github.com/appsmithorg/appsmith/pull/36077/files#diff-8005b0d844e9004cc03abebe5ec522c3ff8be090da2c9288344a5c2a76e4717b

AnnaHariprasad5123 · 2024-09-11T05:48:12Z

Hi @NilanshBansal, I replaced all of them with with stopAndLogTimeInMillis(). Could you check now.

NilanshBansal · 2024-09-11T10:13:21Z

@AnnaHariprasad5123 I tried pulling and executing the changes locally, but I am not seeing some logs from PostgresPlugin.java.
For example this log is not getting printed.

appsmith/app/server/appsmith-plugins/postgresPlugin/src/main/java/com/external/plugins/PostgresPlugin.java

Line 247 in 55f428c

log.debug(printMessage);

Can you check the reasons for this.

...erver/appsmith-plugins/amazons3Plugin/src/main/java/com/external/plugins/AmazonS3Plugin.java

...r/appsmith-plugins/databricksPlugin/src/main/java/com/external/plugins/DatabricksPlugin.java

...ver/appsmith-plugins/firestorePlugin/src/main/java/com/external/plugins/FirestorePlugin.java

NilanshBansal · 2024-09-16T08:33:00Z

@AnnaHariprasad5123 were you able to work on the changes suggested? And also fix the logging issue?

AnnaHariprasad5123 · 2024-09-16T08:35:11Z

I changed all the logs. Now, I am checking the issue

NilanshBansal · 2024-09-16T08:38:06Z

I changed all the logs. Now, I am checking the issue

Thank You, can you please commit the changes done until now, this will help us to review the rest of the things while you work on resolving the logger issue 🙏

coderabbitai

Actionable comments posted: 0

Outside diff range and nitpick comments (1)

app/server/appsmith-plugins/postgresPlugin/src/main/java/com/external/plugins/PostgresPlugin.java (1)

Line range hint 12-24: The current discount and fee structure might not be the best approach.

While the intention to reward customer loyalty is great, the implementation of a flat $20 fee on discounted bills could actually negate the benefit of the discount, especially for smaller purchases or marginal loyalty tiers.

For example, a customer with 3 years of loyalty making a $100 purchase would receive a 10% discount, bringing their total to $90. But after adding the $20 fee, their final bill is $110, which is more than if they had no loyalty discount at all! This might lead to customer dissatisfaction, as the loyalty program could paradoxically increase their bill.

Consider revising either the discount percentages or the flat fee application to better align with customer incentives. Perhaps a percentage-based fee or a minimum purchase amount for the fee to apply could help ensure the discounts always benefit the customer.

Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 55f428c and 78483cc.

Files selected for processing (25)

app/server/appsmith-interfaces/src/main/java/com/appsmith/external/helpers/Stopwatch.java (0 hunks)
app/server/appsmith-plugins/amazons3Plugin/src/main/java/com/external/plugins/AmazonS3Plugin.java (19 hunks)
app/server/appsmith-plugins/anthropicPlugin/src/main/java/com/external/plugins/AnthropicPlugin.java (4 hunks)
app/server/appsmith-plugins/appsmithAiPlugin/src/main/java/com/external/plugins/AppsmithAiPlugin.java (5 hunks)
app/server/appsmith-plugins/arangoDBPlugin/src/main/java/com/external/plugins/ArangoDBPlugin.java (9 hunks)
app/server/appsmith-plugins/awsLambdaPlugin/src/main/java/com/external/plugins/AwsLambdaPlugin.java (5 hunks)
app/server/appsmith-plugins/databricksPlugin/src/main/java/com/external/plugins/DatabricksPlugin.java (7 hunks)
app/server/appsmith-plugins/dynamoPlugin/src/main/java/com/external/plugins/DynamoPlugin.java (7 hunks)
app/server/appsmith-plugins/elasticSearchPlugin/src/main/java/com/external/plugins/ElasticSearchPlugin.java (7 hunks)
app/server/appsmith-plugins/firestorePlugin/src/main/java/com/external/plugins/FirestorePlugin.java (10 hunks)
app/server/appsmith-plugins/googleAiPlugin/src/main/java/com/external/plugins/GoogleAiPlugin.java (3 hunks)
app/server/appsmith-plugins/googleSheetsPlugin/src/main/java/com/external/plugins/GoogleSheetsPlugin.java (6 hunks)
app/server/appsmith-plugins/graphqlPlugin/src/main/java/com/external/plugins/GraphQLPlugin.java (2 hunks)
app/server/appsmith-plugins/mongoPlugin/src/main/java/com/external/plugins/MongoPlugin.java (17 hunks)
app/server/appsmith-plugins/mssqlPlugin/src/main/java/com/external/plugins/MssqlPlugin.java (10 hunks)
app/server/appsmith-plugins/mysqlPlugin/src/main/java/com/external/plugins/MySqlPlugin.java (17 hunks)
app/server/appsmith-plugins/openAiPlugin/src/main/java/com/external/plugins/OpenAiPlugin.java (4 hunks)
app/server/appsmith-plugins/oraclePlugin/src/main/java/com/external/plugins/OraclePlugin.java (10 hunks)
app/server/appsmith-plugins/postgresPlugin/src/main/java/com/external/plugins/PostgresPlugin.java (22 hunks)
app/server/appsmith-plugins/redisPlugin/src/main/java/com/external/plugins/RedisPlugin.java (6 hunks)
app/server/appsmith-plugins/redshiftPlugin/src/main/java/com/external/plugins/RedshiftPlugin.java (13 hunks)
app/server/appsmith-plugins/restApiPlugin/src/main/java/com/external/plugins/RestApiPlugin.java (3 hunks)
app/server/appsmith-plugins/saasPlugin/src/main/java/com/external/plugins/SaasPlugin.java (5 hunks)
app/server/appsmith-plugins/smtpPlugin/src/main/java/com/external/plugins/SmtpPlugin.java (8 hunks)
app/server/appsmith-plugins/snowflakePlugin/src/main/java/com/external/plugins/SnowflakePlugin.java (13 hunks)

Files not reviewed due to no reviewable changes (1)

app/server/appsmith-interfaces/src/main/java/com/appsmith/external/helpers/Stopwatch.java

Files skipped from review due to trivial changes (7)

app/server/appsmith-plugins/amazons3Plugin/src/main/java/com/external/plugins/AmazonS3Plugin.java
app/server/appsmith-plugins/awsLambdaPlugin/src/main/java/com/external/plugins/AwsLambdaPlugin.java
app/server/appsmith-plugins/elasticSearchPlugin/src/main/java/com/external/plugins/ElasticSearchPlugin.java
app/server/appsmith-plugins/firestorePlugin/src/main/java/com/external/plugins/FirestorePlugin.java
app/server/appsmith-plugins/openAiPlugin/src/main/java/com/external/plugins/OpenAiPlugin.java
app/server/appsmith-plugins/oraclePlugin/src/main/java/com/external/plugins/OraclePlugin.java
app/server/appsmith-plugins/redisPlugin/src/main/java/com/external/plugins/RedisPlugin.java

Files skipped from review as they are similar to previous changes (16)

app/server/appsmith-plugins/anthropicPlugin/src/main/java/com/external/plugins/AnthropicPlugin.java
app/server/appsmith-plugins/appsmithAiPlugin/src/main/java/com/external/plugins/AppsmithAiPlugin.java
app/server/appsmith-plugins/arangoDBPlugin/src/main/java/com/external/plugins/ArangoDBPlugin.java
app/server/appsmith-plugins/databricksPlugin/src/main/java/com/external/plugins/DatabricksPlugin.java
app/server/appsmith-plugins/dynamoPlugin/src/main/java/com/external/plugins/DynamoPlugin.java
app/server/appsmith-plugins/googleAiPlugin/src/main/java/com/external/plugins/GoogleAiPlugin.java
app/server/appsmith-plugins/googleSheetsPlugin/src/main/java/com/external/plugins/GoogleSheetsPlugin.java
app/server/appsmith-plugins/graphqlPlugin/src/main/java/com/external/plugins/GraphQLPlugin.java
app/server/appsmith-plugins/mongoPlugin/src/main/java/com/external/plugins/MongoPlugin.java
app/server/appsmith-plugins/mssqlPlugin/src/main/java/com/external/plugins/MssqlPlugin.java
app/server/appsmith-plugins/mysqlPlugin/src/main/java/com/external/plugins/MySqlPlugin.java
app/server/appsmith-plugins/redshiftPlugin/src/main/java/com/external/plugins/RedshiftPlugin.java
app/server/appsmith-plugins/restApiPlugin/src/main/java/com/external/plugins/RestApiPlugin.java
app/server/appsmith-plugins/saasPlugin/src/main/java/com/external/plugins/SaasPlugin.java
app/server/appsmith-plugins/smtpPlugin/src/main/java/com/external/plugins/SmtpPlugin.java
app/server/appsmith-plugins/snowflakePlugin/src/main/java/com/external/plugins/SnowflakePlugin.java

AnnaHariprasad5123 · 2024-09-17T04:40:46Z

Hi @NilanshBansal, log statements are visible in console now. I have added scope as provided for slf4j dependencies in parent pom of appsmith-plugins and removed slf4j exclusion from pf4j dependency. I have checked some of the plugins with test- configurations action only. Could you check all logs in all plugins from your end.

MySQL Plugin

Postgres Plugin

Amazon S3 Plugin

Dynamo Plugin

NilanshBansal · 2024-09-17T04:57:10Z

Thank You @AnnaHariprasad5123. I am checking this

NilanshBansal · 2024-09-17T10:08:43Z

Thanks @AnnaHariprasad5123 for your valuable contribution! We highly appreciate your efforts in making Appsmith better ❤️
You can find more issues to contribute from this list!
If you want to specifically work on Datasources and Query Related Issues, you can pick up ones from this list.

fix:logger-not-working-in-plugins

d9a696a

AnnaHariprasad5123 requested a review from sharat87 as a code owner September 10, 2024 12:42

coderabbitai bot reviewed Sep 10, 2024

View reviewed changes

AnnaHariprasad5123 changed the title ~~fix:logger-not-working-in-plugins~~ fix: logger-not-working-in-plugins Sep 10, 2024

fix: changed-sout-to-logs

262762a

AnnaHariprasad5123 requested review from nidhi-nair and sondermanish as code owners September 10, 2024 16:38

AnnaHariprasad5123 added 2 commits September 10, 2024 22:32

fix: logged-exceptions

8fa09e3

replace-plus-to-comma

e2a8dea

NilanshBansal requested review from NilanshBansal and removed request for sharat87, nidhi-nair and sondermanish September 11, 2024 04:40

replaced-sysOutFn-in-all-plugins

55f428c

NilanshBansal added Query & JS Pod Issues related to the query & JS Pod Integrations Pod General Issues related to the Integrations Pod that don't fit into other tags. Community Contributor Meant to track issues that are assigned to external contributors Integrations Pod labels Sep 11, 2024

AnaghHegde requested changes Sep 12, 2024

View reviewed changes

changed-all-logs

78483cc

coderabbitai bot reviewed Sep 16, 2024

View reviewed changes

NilanshBansal requested a review from AnaghHegde September 16, 2024 10:07

fixed-logging-issue

2bdf166

NilanshBansal mentioned this pull request Sep 17, 2024

[Do Not Merge] chore: Shadow PR to external contribution #36231 #36361

Closed

2 tasks

NilanshBansal approved these changes Sep 17, 2024

View reviewed changes

nerbos merged commit 7aae152 into appsmithorg:release Sep 17, 2024
16 of 17 checks passed

This was referenced Sep 17, 2024

chore: Upgrading spring to 3.3.3 to resolve vulnerable dependencies #36266

Open

chore: making autocommit GA #36347

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: logger-not-working-in-plugins #36231

fix: logger-not-working-in-plugins #36231

AnnaHariprasad5123 commented Sep 10, 2024 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Sep 10, 2024 •

edited

Loading

Chat

CodeRabbit Commands (Invoked using PR comments)

Other keywords and placeholders

Documentation and Community

coderabbitai bot left a comment

NilanshBansal commented Sep 10, 2024

AnnaHariprasad5123 commented Sep 10, 2024

AnnaHariprasad5123 commented Sep 11, 2024

NilanshBansal commented Sep 11, 2024 •

edited

Loading

AnnaHariprasad5123 commented Sep 11, 2024

NilanshBansal commented Sep 11, 2024

NilanshBansal commented Sep 16, 2024

AnnaHariprasad5123 commented Sep 16, 2024

NilanshBansal commented Sep 16, 2024

coderabbitai bot left a comment

AnnaHariprasad5123 commented Sep 17, 2024

NilanshBansal commented Sep 17, 2024

NilanshBansal commented Sep 17, 2024 •

edited

Loading

fix: logger-not-working-in-plugins #36231

fix: logger-not-working-in-plugins #36231

Conversation

AnnaHariprasad5123 commented Sep 10, 2024 • edited by coderabbitai bot Loading

Summary by CodeRabbit

Summary by CodeRabbit

coderabbitai bot commented Sep 10, 2024 • edited Loading

Walkthrough

Changes

Assessment against linked issues

Chat

CodeRabbit Commands (Invoked using PR comments)

Other keywords and placeholders

Documentation and Community

coderabbitai bot left a comment

Choose a reason for hiding this comment

NilanshBansal commented Sep 10, 2024

AnnaHariprasad5123 commented Sep 10, 2024

AnnaHariprasad5123 commented Sep 11, 2024

NilanshBansal commented Sep 11, 2024 • edited Loading

AnnaHariprasad5123 commented Sep 11, 2024

NilanshBansal commented Sep 11, 2024

NilanshBansal commented Sep 16, 2024

AnnaHariprasad5123 commented Sep 16, 2024

NilanshBansal commented Sep 16, 2024

coderabbitai bot left a comment

Choose a reason for hiding this comment

AnnaHariprasad5123 commented Sep 17, 2024

NilanshBansal commented Sep 17, 2024

NilanshBansal commented Sep 17, 2024 • edited Loading

AnnaHariprasad5123 commented Sep 10, 2024 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Sep 10, 2024 •

edited

Loading

NilanshBansal commented Sep 11, 2024 •

edited

Loading

NilanshBansal commented Sep 17, 2024 •

edited

Loading