Skip to content

[Improvement-17361][TaskPlugin] Use logger marker to exclude the system log in task instance log content. #17378

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Aug 6, 2025

Conversation

ruanwenjun
Copy link
Member

Purpose of the pull request

close #17361

Brief change log

Remove some info in task instance log, the user doesn't need to know the engine detail.

  • Remove the thread name in task instance log
  • Remove system event in task instance log

Verify this pull request

This pull request is code cleanup without any test coverage.

(or)

This pull request is already covered by existing tests, such as (please describe tests).

(or)

This change added tests and can be verified as follows:

(or)

Pull Request Notice

Pull Request Notice

If your pull request contains incompatible change, you should also add it to docs/docs/en/guide/upgrade/incompatible.md

"{}" +
"\n****************************** Script Content *****************************************************************\n",
finalScript);
log.info("Final Script Content:\n====================\n{}\n====================", finalScript);

Check failure

Code scanning / CodeQL

Insertion of sensitive information into log files High

This
potentially sensitive information
is written to a log file.
This
potentially sensitive information
is written to a log file.
@ruanwenjun ruanwenjun added this to the 3.3.1 milestone Jul 29, 2025
taskExecutionContext.setStartTime(System.currentTimeMillis());
log.info("End initialize taskContext {}", JSONUtils.toPrettyJsonString(taskExecutionContext));
log.info(TaskLogMarkers.excludeInTaskLog(), "{}", JSONUtils.toJsonString(taskExecutionContext));
Copy link
Contributor

Choose a reason for hiding this comment

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

May log.info(TaskLogMarkers.excludeInTaskLog(), "Initialized taskContext {}", JSONUtils.toJsonString(taskExecutionContext)); is better?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, good catch

@ruanwenjun ruanwenjun force-pushed the dev_wenjun_fix17361 branch from ffab324 to 47b644e Compare July 30, 2025 01:17
@Sud0x67
Copy link
Contributor

Sud0x67 commented Jul 30, 2025

LGTM

SbloodyS
SbloodyS previously approved these changes Jul 30, 2025
Copy link
Member

@SbloodyS SbloodyS left a comment

Choose a reason for hiding this comment

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

LGTM

@SbloodyS SbloodyS added the improvement make more easy to user or prompt friendly label Jul 30, 2025
@ruanwenjun ruanwenjun force-pushed the dev_wenjun_fix17361 branch 2 times, most recently from 2df24ef to e50d2d8 Compare August 5, 2025 08:59
@ruanwenjun ruanwenjun force-pushed the dev_wenjun_fix17361 branch from e50d2d8 to 318df43 Compare August 5, 2025 12:02
@ruanwenjun ruanwenjun force-pushed the dev_wenjun_fix17361 branch from 318df43 to b01750e Compare August 6, 2025 01:32
Copy link

sonarqubecloud bot commented Aug 6, 2025

Quality Gate Failed Quality Gate failed

Failed conditions
2.8% Coverage on New Code (required ≥ 60%)

See analysis details on SonarQube Cloud

@SbloodyS SbloodyS merged commit 4d7f927 into apache:dev Aug 6, 2025
68 of 70 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend improvement make more easy to user or prompt friendly test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Improvement][TaskPlugin] Separate system log and task log using different loggers
4 participants