Skip to content
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

feat: configure logging by slf4j + log4j 2 #79

Merged
merged 1 commit into from
Mar 26, 2024
Merged

feat: configure logging by slf4j + log4j 2 #79

merged 1 commit into from
Mar 26, 2024

Conversation

yuuahp
Copy link
Member

@yuuahp yuuahp commented Mar 26, 2024

Log4J を設定してログをちゃんと制御するようにしました。
INFO 以上のログがコンソールに、DEBUG 以上のログがファイル (./logs/vcspeaker-yyyy-mm-dd.log) に出力されます。

Kord (と Extensions) のデバッグログについては、ほぼ使ってないし、本当に出したい開発ログが流れてつらいのでコンソールには出さなくていいかなと思いました。

あと、まだ ./logs ディレクトリをほかの場所に設定できるようにはしていません。
必要なら調べて実装しますが、要りますか?

close #9

@github-actions github-actions bot requested a review from book000 March 26, 2024 06:56
@book000
Copy link
Member

book000 commented Mar 26, 2024

詳しいコードレビューはのちほどしますね。

Kord (と Extensions) のデバッグログについては、ほぼ使ってないし、本当に出したい開発ログが流れてつらいのでコンソールには出さなくていいかなと思いました。

気になってるのは、決して意図的に出力してから既存のログ(e.g., ログイン完了、コマンド実行 etc)が多くないので、そこがちょっとネックなところです。
その分、メンテナンスのために必要となる「本当に出したいログ」を追加していく必要がありますね…

まだ ./logs ディレクトリをほかの場所に設定できるようにはしていません。

Dockerで動作する時に、任意の場所にボリューム割り当てて書き出せますかね?
コンテナ再起動でログが飛ぶのはマズいです。その要件が満たせてるなら良いと思います。

@yuuahp
Copy link
Member Author

yuuahp commented Mar 26, 2024

Docker の方は logs 移動しなくてもボリューム割り当てられそうです。
タイムゾーンの設定ってこの部分であってますか?
https://github.com/jaoafa/watch-guilds/blob/e4d65b768fbe8cb3dadbdd7f575edc9905a059eb/Dockerfile#L22-L25
今たぶん GMT か何かになってる気がします。
image
(6時あたりのスクリーンショット)

あとログが少ないのはわかりました。今後のコミットで追加します。

@book000
Copy link
Member

book000 commented Mar 26, 2024

タイムゾーンはアプリケーションごとに何見てるかわからんくて、いつも困ってます。
とりあえずいつもそれを仕込んでますが、確定でそれ入れればうまくいく保証はないです。
log4j側で設定かけないとダメかも知れませぬ

@book000
Copy link
Member

book000 commented Mar 26, 2024

ログについては、ぱっと思いつく感じだと以下ですかねー

  • ログイン成功(with ログインユーザ名)
  • メッセージ受信・キュー追加(with ギルドID、メッセージID)
  • ダウンロード完了(with ダウンロード所要時間)
  • メッセージ読み上げ開始
  • コマンド実行(だれが、どこで、なにを)

@book000 book000 merged commit 292ca3c into main Mar 26, 2024
5 of 6 checks passed
@book000 book000 deleted the feat/logging branch March 26, 2024 11:49
@book000 book000 mentioned this pull request Mar 26, 2024
@book000
Copy link
Member

book000 commented Mar 26, 2024

今マージするべきじゃなかったかも知れない(タイムゾーン追加しようとしてた?)

@yuuahp
Copy link
Member Author

yuuahp commented Mar 26, 2024

してた...

@book000
Copy link
Member

book000 commented Mar 26, 2024

かなしみ

@yuuahp
Copy link
Member Author

yuuahp commented Mar 26, 2024

(◞‸◟)

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