ServerConnectionReport: ボットが出力するログの形式と、IRC からの応答のログの形式を分離した

[koi-chan]

fix #74

PluginBase::Adapter のコンストラクタに、アダプターでもプラグイン設定を使えるような仕組みを導入した。

そもそも当該のログ出力メッセージはわかりづらいでしょうか。
この時にはメールを送信しておらず、メールの送信準備ができた段階です。

[ochaochaocha3]

メッセージは問題ないと思います。ただ、ロガーとして別のものを使う場面がまだ思い浮かんでおりません。

アダプタからはいつでもconfigメソッドでAdapterOptionsオブジェクトを呼び出せるので、個人的にはそれほど乗り気ではないです。

[koi-chan]

RGRB本体・プラグイン・IRCやDiscordとの通信、これら3種類のログ出力が、見て分かるようになっていたほうが、開発も運用管理もしやすいのではないか、という意図があります。
チャットボットは基本的に動かしているときは放置なので、ログを見ても原因が分かりづらいと、利用者から不具合報告を受けて、しかしすぐに修正改良できないとき、利用者側で操作法を変えてもらう対応を取ってもらうためのお願いもしづらくなってしまいます。

ただ、今回の変更ではこの意図に基づく変更も中途半端になってしまっているのも事実です。
3つをきっちり分けられるように、引き続き IRC/Discord ともに作り込みたいと思っています。

[ochaochaocha3]

RGRB本体・プラグイン・IRCやDiscordとの通信、これら3種類のログ出力が、見て分かるようになっていたほうが、開発も運用管理もしやすいのではないか、という意図があります。

意図を理解できました。これに対しては私も賛成いたします。そのようにする方が開発や運用管理を行いやすいと考えます。

ただし、関連するissueである#133「プラグイン構成要素別の、基底クラス的に使うモジュールを作成する」とは別件として作りたいです（つまり、マージ先を変えたいです）。#133 の目的は、新しい機能を追加することではなく、現在の機能をより簡単に実現できるように仕組みを作り直すことだと、私は捉えておりました。

[koi-chan]

13:37:19 (ocha) > exec/xxx_bot.rb から各プラグインにプラグイン設定として渡しているロガー(AdapterOptions.logger)はLumberjack::Logger のはずです
13:38:06 以前の状態を確認したところ、これも挙動が変わっていて「あれ？」と思いました
13:39:39 (ocha) > <�13koi-chan�> その上でrgrb本体・プラグインで1つ+通信の2つ、可能ならrgrb本体・プラグイン・通信の3つが、見てどのログか分かるようになっているのが理想型なので、なかなか難しいです。
13:40:13 これをやろうとしていたのでしょうか…？
13:41:13 以前は、ジェネレータに渡すロガーは、（ロガーの機能も併せ持つ）IRCアダプタだったはず
13:42:09 この部分

rgrb/lib/rgrb/plugin/configurable_adapter.rb

Lines 18 to 20 in 354f52a

	# プラグインをロガーとして使えるよう、設定に含める
	config_data = config[:plugin].merge({ logger: self })
	@generator.configure(config_data)

13:42:10 (Toybox) FetchTitle: rgrb/configurable_adapter.rb at 354f52a · cre-ne-jp/rgrb · GitHub
13:42:56 だから、RGRB本体のロガーをジェネレータに渡したのではなかった
13:43:40 なぜIRCアダプタをロガーとして渡していたかというと、IRCアダプタのロガー機能を使うと、ログの頭にプラグイン名が追加されたからだったと思います
13:44:08 ところが、今はRGRB本体のロガーを使っているから、ログにプラグイン名が含まれなくなった
13:48:07 直すとしたら、ここか

rgrb/lib/rgrb/plugin_base/adapter.rb

Lines 17 to 20 in 7d4f640

	@generator.logger = config.logger
	# プラグインをロガーとして使えるよう、設定に含める
	config_data = config.plugin.merge({ logger: self })

13:48:08 (Toybox) FetchTitle: rgrb/adapter.rb at 7d4f640 · cre-ne-jp/rgrb · GitHub
13:49:56 @generator.logger = self にして、config_data = config.plugin.merge({ logger: self }) は削除する（重複しているから）
13:52:38 あとは server_connection_report のここを上の変更と合うように変える

rgrb/lib/rgrb/plugin/server_connection_report/irc_adapter_methods.rb

Line 32 in 7d4f640

@mail_generator.logger = config[:logger]

13:52:39 (Toybox) FetchTitle: rgrb/irc_adapter_methods.rb at 7d4f640 · cre-ne-jp/rgrb · GitHub
13:53:23 @mail_generator.logger = self と変える
13:54:18 これでいいのかな
13:55:49 あと、テスト時のようなアダプタ未使用時のロガーについて、プラグイン名を出力しなくてもよいのならば、PluginBase::Adapterに共通のロガーを用意しておく
13:56:07 そうしたら、テスト時に明示的にロガーを設定しなくてもよい
13:56:50 （つまり、DRYでない。かつ、プラグインの数だけロガーが作られることもない）
13:58:13 IRCやDiscordのアダプタが使われるときは、ジェネレータのロガーが差し替えられるので、その既定のロガーが使われることもない
2019/07/28 14:00:00
14:01:43 ああ、そうか、やはりプラグイン（のジェネレータ）のロガーはアダプタとは分ける意図だったのかな #155 (comment)
14:01:44 (Toybox) FetchTitle: ServerConnectionReport: ボットが出力するログの形式と、IRC からの応答のログの形式を分離した by koi-chan · Pull Request #155 · cre-ne-jp/rgrb · GitHub
14:02:11 これは別件とした方が良いと思います
14:06:06 もしこの3種類でログを分けるのならば、最終的には /var/log/rgrb/{{irc,discord}-{bot,plugins,com}.log のように複数のファイルに出力できるところまで持っていくと便利かも
14:06:47 ログを分割するとは、そういうイメージかなぁ
14:08:25 （標準出力に3種類のログが混ざって出力され、何についてのログか見分けるのが大変、ということだと捉えました）
14:10:05 （単にロガーを3分けるだけで標準出力に混ぜて出力
14:11:05 ミス→単にロガーを3種類に分けるだけで標準出力に混ぜて出力するのでは、出力書式が変わるだけで、今とあまり変わらないですよね？
14:12:17 もしログをファイルに出力できるようにする場合、おそらく設定ファイルで出力先のパスを指定できないと不便だと思うから、そのようにできる改修がさらに必要と考えました
14:13:58 …などと考えていたら、「アダプタやジェネレータでよく使う機能を共通のモジュールに含める」という今回の目的よりさらに先の話なのかな、と