Skip to content

manager, config: watch and apply the latest log configuration online#95

Merged
djshow832 merged 8 commits into
pingcap:mainfrom
djshow832:push_log
Sep 26, 2022
Merged

manager, config: watch and apply the latest log configuration online#95
djshow832 merged 8 commits into
pingcap:mainfrom
djshow832:push_log

Conversation

@djshow832

@djshow832 djshow832 commented Sep 22, 2022

Copy link
Copy Markdown
Collaborator

What problem does this PR solve?

Issue Number: ref #94

Problem Summary:
Log configurations need to take effect dynamically.

What is changed and how it works:

  • Watch the update of the log configuration and apply them to the LoggerManager.
  • Manage the LoggerManager by Server and remove the logs in main().

TODO:
The LoggerManager relies on the ConfigManager to get the log path, but ConfigManager relies on LoggerManager to get the logger. Currently, LoggerManager logs to the path specified in the static config first and then switches to the path specified in the dynamic config. There are some ways to solve it:

  • Forbid overwriting the log path
  • Do not log in the ConfigManager before the logger is initialized
  • There should be a global log path and a log path for each tenant. The global log path cannot be updated, but tenants can update theirs. Logging into different paths helps privilege control among tenants.

Check List

Tests

  • Unit test
  • Integration test
  • Manual test (add detailed scripts or steps below)
  • No code
$ echo '{"log-file": {"filename":"proxy.log"}}' | ./bin/tiproxyctl config log set
""
$ ./bin/tiproxyctl config log get
{"log-file":{"filename":"proxy.log"}}
$ cat proxy.log
[2022/09/23 10:27:05.069 +08:00] [INFO] [main.gin] [/api/admin/config/log] [status=200] [method=GET] [path=/api/admin/config/log] [query=] [ip=::1] [user-agent=Go-http-client/1.1] [latency=93.715µs]

Notable changes

  • Has configuration change
  • Has HTTP API interfaces change (Don't forget to add the declarative for API)
  • Has weirctl change
  • Other user behavior changes

Release note

Please refer to Release Notes Language Style Guide to write a quality release note.

None

@djshow832
djshow832 requested a review from xhebox September 22, 2022 07:06

@xhebox xhebox 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.

Rest LGTM

Comment thread pkg/manager/logger/manager.go Outdated
Comment thread lib/config/proxy.go Outdated
Comment thread lib/cli/config.go Outdated
Comment thread pkg/manager/config/proxy.go Outdated
Comment thread pkg/server/api/config.go
Comment thread pkg/manager/config/proxy.go
Comment thread pkg/manager/config/proxy.go
Comment thread pkg/manager/config/proxy.go Outdated

@xhebox xhebox 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.

Rest LGTM

Comment thread lib/config/proxy.go Outdated
@djshow832
djshow832 merged commit 3bf1000 into pingcap:main Sep 26, 2022
@djshow832
djshow832 deleted the push_log branch September 26, 2022 07:52
@aytrack aytrack mentioned this pull request Sep 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants