use unified root logger for fx dependency-injection in chaos-controller-manager #2808

STRRL · 2022-01-20T08:27:47Z

What problem does this PR solve?

It's a part of #2752.

What's changed and how it works?

use a certain logr.Logger for both controller-runtime and fx

Related changes

Need to update chaos-mesh/website
Need to update Dashboard UI
Need to cheery-pick to release branches
- release-2.1
- release-2.0

Checklist

Tests

Unit test
E2E test
No code
Manual test (add steps below)

Side effects

Breaking backward compatibility

Release note

Please add a release note.

You can safely ignore this section if you don't think this PR needs a release note.

DCO

If you find the DCO check fails, please run commands like below (Depends on the actual situations. For example, if the failed commit isn't the most recent) to fix it:

git commit --amend --signoff
git push --force

Signed-off-by: STRRL <str_ruiling@outlook.com>

ti-chi-bot · 2022-01-20T08:27:48Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

Andrewmatilde
iguoyr

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

The full list of commands accepted by this bot can be found here.

Reviewer can indicate their review by submitting an approval review.
Reviewer can cancel approval by submitting a request changes review.

Signed-off-by: STRRL <str_ruiling@outlook.com>

codecov · 2022-01-21T08:28:42Z

Codecov Report

Merging #2808 (a5c36ae) into master (0a96a2a) will decrease coverage by 0.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #2808      +/-   ##
==========================================
- Coverage   40.96%   40.94%   -0.02%     
==========================================
  Files         147      147              
  Lines       11985    11983       -2     
==========================================
- Hits         4910     4907       -3     
- Misses       6705     6708       +3     
+ Partials      370      368       -2

Impacted Files	Coverage Δ
...md/chaos-controller-manager/provider/controller.go	`38.79% <ø> (-1.04%)`	⬇️
.../workflow/controllers/workflow_entry_reconciler.go	`46.03% <0.00%> (-6.88%)`	⬇️
pkg/selector/generic/mode.go	`25.64% <0.00%> (-2.57%)`	⬇️
api/v1alpha1/zz_generated.chaosmesh.go	`57.64% <0.00%> (ø)`
pkg/workflow/controllers/serial_node_reconciler.go	`60.73% <0.00%> (+1.57%)`	⬆️
pkg/workflow/controllers/chaos_node_reconciler.go	`61.31% <0.00%> (+3.15%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 712f58c...a5c36ae. Read the comment docs.

Signed-off-by: STRRL <str_ruiling@outlook.com>

cmd/chaos-controller-manager/main.go

Signed-off-by: STRRL <str_ruiling@outlook.com>

STRRL · 2022-02-07T05:58:53Z

PTAL

/cc @iguoyr @WangXiangUSTC @YangKeao

iguoyr · 2022-02-07T11:30:30Z

pkg/log/global_logger.go

+	return result
+}
+
+// ReplaceGlobals would replace the global logger with the given logger.


Add some comments about this function should only be called when application starting?

How about "should be called when application starting" ?🤔 I think this restriction ("should only be") is not necessary.

Agree with you :)

Updated, PTAL!

Signed-off-by: STRRL <str_ruiling@outlook.com>

iguoyr

LGTM

Andrewmatilde

LGTM. Need example in future.

STRRL · 2022-02-08T08:49:05Z

/merge

ti-chi-bot · 2022-02-08T08:49:07Z

This pull request has been accepted and is ready to merge.

Commit hash: f87e4d7

STRRL · 2022-02-09T07:48:00Z

/merge

Signed-off-by: STRRL <str_ruiling@outlook.com>

STRRL · 2022-02-09T08:05:37Z

/merge

ti-chi-bot · 2022-02-09T08:05:39Z

This pull request has been accepted and is ready to merge.

Commit hash: a5c36ae

ti-srebot · 2022-02-09T08:26:01Z

cherry pick to release-2.1 failed

STRRL · 2022-02-09T08:31:51Z

cherry pick to release-2.1 failed

I would manually cherry pick it.

…er-manager (chaos-mesh#2808) * chore: package for logger Signed-off-by: STRRL <str_ruiling@outlook.com> * chore: fix dependency injection Signed-off-by: STRRL <str_ruiling@outlook.com> * chore: update comments for global logger L Signed-off-by: STRRL <str_ruiling@outlook.com> * chore: use standard log instead of fmt.Print Signed-off-by: STRRL <str_ruiling@outlook.com> * chore: configure logger for fx Signed-off-by: STRRL <str_ruiling@outlook.com> * chore: make check Signed-off-by: STRRL <str_ruiling@outlook.com> * chore: update comments for ReplaceGlobals Signed-off-by: STRRL <str_ruiling@outlook.com> * chore: make check Signed-off-by: STRRL <str_ruiling@outlook.com> Co-authored-by: Ti Chi Robot <ti-community-prow-bot@tidb.io>

* use unified root logger for fx dependency-injection in chaos-controller-manager (#2808) * chore: package for logger Signed-off-by: STRRL <str_ruiling@outlook.com> * chore: fix dependency injection Signed-off-by: STRRL <str_ruiling@outlook.com> * chore: update comments for global logger L Signed-off-by: STRRL <str_ruiling@outlook.com> * chore: use standard log instead of fmt.Print Signed-off-by: STRRL <str_ruiling@outlook.com> * chore: configure logger for fx Signed-off-by: STRRL <str_ruiling@outlook.com> * chore: make check Signed-off-by: STRRL <str_ruiling@outlook.com> * chore: update comments for ReplaceGlobals Signed-off-by: STRRL <str_ruiling@outlook.com> * chore: make check Signed-off-by: STRRL <str_ruiling@outlook.com> Co-authored-by: Ti Chi Robot <ti-community-prow-bot@tidb.io> * chore: make check Signed-off-by: STRRL <str_ruiling@outlook.com> Co-authored-by: Ti Chi Robot <ti-community-prow-bot@tidb.io>

* use unified root logger for fx dependency-injection in chaos-controller-manager (chaos-mesh#2808) * chore: package for logger Signed-off-by: STRRL <str_ruiling@outlook.com> * chore: fix dependency injection Signed-off-by: STRRL <str_ruiling@outlook.com> * chore: update comments for global logger L Signed-off-by: STRRL <str_ruiling@outlook.com> * chore: use standard log instead of fmt.Print Signed-off-by: STRRL <str_ruiling@outlook.com> * chore: configure logger for fx Signed-off-by: STRRL <str_ruiling@outlook.com> * chore: make check Signed-off-by: STRRL <str_ruiling@outlook.com> * chore: update comments for ReplaceGlobals Signed-off-by: STRRL <str_ruiling@outlook.com> * chore: make check Signed-off-by: STRRL <str_ruiling@outlook.com> Co-authored-by: Ti Chi Robot <ti-community-prow-bot@tidb.io> * chore: make check Signed-off-by: STRRL <str_ruiling@outlook.com> Co-authored-by: Ti Chi Robot <ti-community-prow-bot@tidb.io> Signed-off-by: STRRL <str_ruiling@outlook.com>

* cherry-pick logging updates into release-2.1 (#2970) * use unified root logger for fx dependency-injection in chaos-controller-manager (#2808) * chore: package for logger Signed-off-by: STRRL <str_ruiling@outlook.com> * chore: fix dependency injection Signed-off-by: STRRL <str_ruiling@outlook.com> * chore: update comments for global logger L Signed-off-by: STRRL <str_ruiling@outlook.com> * chore: use standard log instead of fmt.Print Signed-off-by: STRRL <str_ruiling@outlook.com> * chore: configure logger for fx Signed-off-by: STRRL <str_ruiling@outlook.com> * chore: make check Signed-off-by: STRRL <str_ruiling@outlook.com> * chore: update comments for ReplaceGlobals Signed-off-by: STRRL <str_ruiling@outlook.com> * chore: make check Signed-off-by: STRRL <str_ruiling@outlook.com> Co-authored-by: Ti Chi Robot <ti-community-prow-bot@tidb.io> * chore: make check Signed-off-by: STRRL <str_ruiling@outlook.com> Co-authored-by: Ti Chi Robot <ti-community-prow-bot@tidb.io> Signed-off-by: STRRL <str_ruiling@outlook.com> * log: add customized logger for daemon and bpm (#2902) * add customized logger for daemon and bpm Signed-off-by: YangKeao <yangkeao@chunibyo.icu> * extract the grpc metadata into the context Signed-off-by: YangKeao <yangkeao@chunibyo.icu> * make check Signed-off-by: YangKeao <yangkeao@chunibyo.icu> * make check Signed-off-by: YangKeao <yangkeao@chunibyo.icu> * remove log parameter in killIOChaos Signed-off-by: YangKeao <yangkeao@chunibyo.icu> * add logs Signed-off-by: YangKeao <yangkeao@chunibyo.icu> * remove log parameter in httpchaos server Signed-off-by: YangKeao <yangkeao@chunibyo.icu> * add the context comments for process builder Signed-off-by: YangKeao <yangkeao@chunibyo.icu> * make check Signed-off-by: YangKeao <yangkeao@chunibyo.icu> * use the logger in arguments Signed-off-by: YangKeao <yangkeao@chunibyo.icu> * add back boilerplate Signed-off-by: YangKeao <yangkeao@chunibyo.icu> * fix make check Signed-off-by: YangKeao <yangkeao@chunibyo.icu> Signed-off-by: STRRL <str_ruiling@outlook.com> * remove unused logger in ProcessBuilder Signed-off-by: STRRL <str_ruiling@outlook.com> * chore: make check Signed-off-by: STRRL <str_ruiling@outlook.com> * chore: update changelog Signed-off-by: STRRL <str_ruiling@outlook.com> * retrigger lift Signed-off-by: STRRL <str_ruiling@outlook.com> Co-authored-by: Ti Chi Robot <ti-community-prow-bot@tidb.io> Co-authored-by: YangKeao <yangkeao@chunibyo.icu>

…er-manager (chaos-mesh#2808) * chore: package for logger Signed-off-by: STRRL <str_ruiling@outlook.com> * chore: fix dependency injection Signed-off-by: STRRL <str_ruiling@outlook.com> * chore: update comments for global logger L Signed-off-by: STRRL <str_ruiling@outlook.com> * chore: use standard log instead of fmt.Print Signed-off-by: STRRL <str_ruiling@outlook.com> * chore: configure logger for fx Signed-off-by: STRRL <str_ruiling@outlook.com> * chore: make check Signed-off-by: STRRL <str_ruiling@outlook.com> * chore: update comments for ReplaceGlobals Signed-off-by: STRRL <str_ruiling@outlook.com> * chore: make check Signed-off-by: STRRL <str_ruiling@outlook.com> Co-authored-by: Ti Chi Robot <ti-community-prow-bot@tidb.io> Signed-off-by: zhqqqy <zhqqqy@163.com>

STRRL added 2 commits January 20, 2022 15:30

chore: package for logger

c152ed2

Signed-off-by: STRRL <str_ruiling@outlook.com>

chore: fix dependency injection

d8245ea

Signed-off-by: STRRL <str_ruiling@outlook.com>

ti-chi-bot added the do-not-merge/work-in-progress label Jan 20, 2022

ti-chi-bot requested review from Hexilee and YangKeao January 20, 2022 08:27

ti-chi-bot added the size/L label Jan 20, 2022

STRRL added needs-cherry-pick-release-2.1 and removed size/L labels Jan 20, 2022

ti-chi-bot added the size/L label Jan 20, 2022

STRRL changed the title ~~[WIP]: use unified root logger for chaos-controller-manager~~ [WIP]: use unified root logger for fx dependency-injection in chaos-controller-manager Jan 20, 2022

chore: update comments for global logger L

47ff8c1

Signed-off-by: STRRL <str_ruiling@outlook.com>

STRRL mentioned this pull request Jan 21, 2022

Tracking Issue: Refine Logging #2752

Open

30 tasks

chore: use standard log instead of fmt.Print

030e6d9

Signed-off-by: STRRL <str_ruiling@outlook.com>

sonatype-lift bot reviewed Feb 7, 2022

View reviewed changes

cmd/chaos-controller-manager/main.go Show resolved Hide resolved

STRRL added 2 commits February 7, 2022 13:27

chore: configure logger for fx

37914f9

Signed-off-by: STRRL <str_ruiling@outlook.com>

chore: make check

db03e3e

Signed-off-by: STRRL <str_ruiling@outlook.com>

STRRL changed the title ~~[WIP]: use unified root logger for fx dependency-injection in chaos-controller-manager~~ use unified root logger for fx dependency-injection in chaos-controller-manager Feb 7, 2022

STRRL marked this pull request as ready for review February 7, 2022 05:58

ti-chi-bot removed the do-not-merge/work-in-progress label Feb 7, 2022

ti-chi-bot requested review from iguoyr and WangXiangUSTC February 7, 2022 05:58

iguoyr reviewed Feb 7, 2022

View reviewed changes

chore: update comments for ReplaceGlobals

f87e4d7

Signed-off-by: STRRL <str_ruiling@outlook.com>

iguoyr approved these changes Feb 8, 2022

View reviewed changes

ti-chi-bot added the status/LGT1 label Feb 8, 2022

Andrewmatilde approved these changes Feb 8, 2022

View reviewed changes

ti-chi-bot removed the status/LGT1 label Feb 8, 2022

ti-chi-bot added the status/LGT2 label Feb 8, 2022

ti-chi-bot added the status/can-merge label Feb 8, 2022

ti-chi-bot added 2 commits February 8, 2022 16:49

Merge branch 'master' into root-logger-for-chaos-controller-manager

19654de

Merge branch 'master' into root-logger-for-chaos-controller-manager

d3b7b63

chore: make check

a5c36ae

Signed-off-by: STRRL <str_ruiling@outlook.com>

ti-chi-bot removed the status/can-merge label Feb 9, 2022

ti-chi-bot added the status/can-merge label Feb 9, 2022

ti-chi-bot merged commit 149e540 into chaos-mesh:master Feb 9, 2022

STRRL mentioned this pull request Mar 3, 2022

unreadable logging timestamp format in chaos-controller-manager #2969

Closed

3 tasks

STRRL mentioned this pull request Mar 3, 2022

cherry-pick logging updates into release-2.1 #2970

Merged

9 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

use unified root logger for fx dependency-injection in chaos-controller-manager #2808

use unified root logger for fx dependency-injection in chaos-controller-manager #2808

STRRL commented Jan 20, 2022 •

edited

ti-chi-bot commented Jan 20, 2022 •

edited

codecov bot commented Jan 21, 2022 •

edited

STRRL commented Feb 7, 2022

iguoyr Feb 7, 2022

STRRL Feb 8, 2022

iguoyr Feb 8, 2022

STRRL Feb 8, 2022

iguoyr left a comment

Andrewmatilde left a comment

STRRL commented Feb 8, 2022

ti-chi-bot commented Feb 8, 2022

STRRL commented Feb 9, 2022

STRRL commented Feb 9, 2022

ti-chi-bot commented Feb 9, 2022

ti-srebot commented Feb 9, 2022

STRRL commented Feb 9, 2022

use unified root logger for fx dependency-injection in chaos-controller-manager #2808

use unified root logger for fx dependency-injection in chaos-controller-manager #2808

Conversation

STRRL commented Jan 20, 2022 • edited

What problem does this PR solve?

What's changed and how it works?

Related changes

Checklist

Release note

DCO

ti-chi-bot commented Jan 20, 2022 • edited

codecov bot commented Jan 21, 2022 • edited

Codecov Report

STRRL commented Feb 7, 2022

iguoyr Feb 7, 2022

Choose a reason for hiding this comment

STRRL Feb 8, 2022

Choose a reason for hiding this comment

iguoyr Feb 8, 2022

Choose a reason for hiding this comment

STRRL Feb 8, 2022

Choose a reason for hiding this comment

iguoyr left a comment

Choose a reason for hiding this comment

Andrewmatilde left a comment

Choose a reason for hiding this comment

STRRL commented Feb 8, 2022

ti-chi-bot commented Feb 8, 2022

STRRL commented Feb 9, 2022

STRRL commented Feb 9, 2022

ti-chi-bot commented Feb 9, 2022

ti-srebot commented Feb 9, 2022

STRRL commented Feb 9, 2022

STRRL commented Jan 20, 2022 •

edited

ti-chi-bot commented Jan 20, 2022 •

edited

codecov bot commented Jan 21, 2022 •

edited