close
Skip to content

[Improve][Connector] Replace printStackTrace with proper logging#10176

Merged
zhangshenghang merged 5 commits into
apache:devfrom
mengxpgogogo-eng:fix/replace-printstacktrace-with-logging
Dec 17, 2025
Merged

[Improve][Connector] Replace printStackTrace with proper logging#10176
zhangshenghang merged 5 commits into
apache:devfrom
mengxpgogogo-eng:fix/replace-printstacktrace-with-logging

Conversation

@mengxpgogogo-eng

Copy link
Copy Markdown
Contributor

This commit improves code quality by replacing printStackTrace() calls with proper SLF4J logging in connector modules.

Changes:

  • GraphQLWebSocket: Replaced 3 printStackTrace() calls with log.error()
  • ActivemqClient: Replaced 1 printStackTrace() call with log.error()
  • SlsSinkWriter: Replaced 1 printStackTrace() call with log.error()

Additional improvements:

  • Added proper interrupt handling for InterruptedException
  • Ensured exception stack traces are properly logged via SLF4J

Benefits:

  1. Better production logging - stack traces go to log files instead of stderr
  2. Consistent with project logging standards
  3. Easier to debug issues in production environments
  4. Follows Java best practices for exception handling

Purpose of this pull request

Does this PR introduce any user-facing change?

How was this patch tested?

Check list

This commit improves code quality by replacing printStackTrace() calls
with proper SLF4J logging in connector modules.

Changes:
- GraphQLWebSocket: Replaced 3 printStackTrace() calls with log.error()
- ActivemqClient: Replaced 1 printStackTrace() call with log.error()
- SlsSinkWriter: Replaced 1 printStackTrace() call with log.error()

Additional improvements:
- Added proper interrupt handling for InterruptedException
- Ensured exception stack traces are properly logged via SLF4J

Benefits:
1. Better production logging - stack traces go to log files instead of stderr
2. Consistent with project logging standards
3. Easier to debug issues in production environments
4. Follows Java best practices for exception handling

@chl-wxp chl-wxp left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

davidzollo
davidzollo previously approved these changes Dec 16, 2025

@davidzollo davidzollo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1
Good job

…ache/seatunnel/connectors/seatunnel/graphql/source/reader/GraphQLWebSocket.java

Co-authored-by: dy102 <132787602+dybyte@users.noreply.github.com>
davidzollo
davidzollo previously approved these changes Dec 16, 2025

@davidzollo davidzollo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1
Good job

@zhangshenghang zhangshenghang left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't delete .idea/vcs.xml

@mengxpgogogo-eng

Copy link
Copy Markdown
Contributor Author

don't delete .idea/vcs.xml

Thanks I have restored .idea/vcs.xml file

@davidzollo davidzollo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

@zhangshenghang zhangshenghang merged commit 86c8450 into apache:dev Dec 17, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants