ログ出力が行われていない箇所がある

[Takym]

不具合の内容

• 例外の握り潰しを行っており、ログ出力を行っていない箇所があります。
• DebugLogger/FFImageLoadingLogger は LoggerService に対しログ出力を行っていません。
• System.Diagnostics.Debug にのみログ出力を行っている箇所があります。
• その他のログ出力を行うべき場所でログ出力を行っていない可能性があります。

期待される挙動

• 例外の握り潰しは無くし、LoggerService に対し正しくログ出力を行う様にします。
• DebugLogger/FFImageLoadingLogger の出力先を LoggerService に変更します。

動作環境

• デバイス：全て
• OS：全て
• バージョン：～1.2.4

その他

詳細は #148 をご確認ください。

追記

「正しい」という表現が曖昧とのご指摘がありましたので、修正しました。

追記2

その他の「ログ出力が行われていない箇所」は現時点ではまだ判明していません。

---

Internal IDs:

• NFR 2920

[Takym]

FFImageLoadingLogger も正しく出力していないみたいですね。

[b-wind]

コード自体に対する物では無いのですが、「正しく出力する」「正しく出力していない」という表現は人や状況によって中身が変わり易く、実際の対応内容もIssueタイトルから推測しにくくなってしまっています。

この Issue の場合、「個別に設定されているLog出力先を（COCOAの）LoggerServiceに統一する」等と書いて貰うと分かりやすくなると思います。

[Takym]

@b-wind さん
ご指摘通り概要に書いた「正しくログ出力を行っていない」「正しくログ出力を(中略)」の表現は曖昧でしたね。
題名については、下記の三つの意味合いを持っているため「ログ出力が行われていない箇所がある」としました。

• 例外の握り潰しを行っており、ログ出力を行っていない。
• DebugLogger/FFImageLoadingLogger は LoggerService に対しログ出力を行っていない。
• その他のログ出力を行うべき場所でログ出力を行っていない。

[b-wind]

下記の三つの意味合いを持っているため「ログ出力が行われていない箇所がある」としました

それぞれ別の問題に見えますね。
Issue も PR も分けて置いた方が良い様に思います。

一般論にはなりますが、1つの PR は可能な限り小さい方がレビューしやすいですし、取り込まれやすいですね。
https://khigashigashi.hatenablog.com/entry/2018/03/09/020359

[keiji]

現時点ではIssueは分けなくて大丈夫です。

Pull Requestに関しては、コードレビューの過程で変更する中で分離するのが妥当な点が見えてきたら、そのときにお願いする可能性があります。

[b-wind]

FFImageLoadingLogger はそもそも使われているのでしょうか？ 定義はあるけど呼び出し元が見当たらない。暗黙の呼び出しがあるかどうかはよく分かりません。
DebugLogger は WriteLine してるだけなので、標準出力に書き出しているはず。

DebugLogger は実際には FFImageLoading の Logger として設定。

cocoa/Covid19Radar/Covid19Radar.iOS/AppDelegate.cs

Lines 48 to 51 in 8052045

	global::FFImageLoading.ImageService.Instance.Initialize(new FFImageLoading.Config.Configuration()
	{
	Logger = new Covid19Radar.Services.DebugLogger()
	});

Android の方では該当の記述無し。

cocoa/Covid19Radar/Covid19Radar.Android/MainActivity.cs

Line 45 in 8052045

global::FFImageLoading.ImageService.Instance.Initialize(new FFImageLoading.Config.Configuration());

特に iOS と Android に差がありそうでも無いので、 Android の方でもLoggerを設定しておいた方が良いのかも。
（そもそも FFImageLoading のログは必要かは判断しかねます）

https://github.com/luberda-molinet/FFImageLoading/wiki/Advanced-Usage

[b-wind]

DebugLogger が ILoggerFacade を使っているところを見ると、Prism 用のLoggerとして作ったのかな。
（ただし、Prism 8 では ILoggerFacade は削除されている）

https://noxi515.hateblo.jp/entry/2018/04/28/221327

[Takym]

FFImageLoadingLogger（下記、画像ロガー）は現在使われておらず、DebugLogger は画像ロガーの機能も持っていますよね。その DebugLogger は Prism では使われていない様です。寧ろ、DebugLogger を削除し画像ロガーに置き換えた方が良さそうです。

[Takym]

#222 を作成した時にほぼ全てのクラスを確認した筈ですが、画像ロガーは削除しませんでしたね。何処からか参照されていた気がします。（見落としである可能性もあります）

[b-wind]

FFImageLoading のサンプルコードでは Android でも CustomLogger の設定をしている。

https://github.com/luberda-molinet/FFImageLoading/blob/1ff466232621d266ff5b55aae7309d52173f5710/samples/ImageLoading.Forms.Sample/Droid/MainActivity.cs#L29-L37

設定しない場合のデフォルトロガーはコレっぽい？
https://github.com/luberda-molinet/FFImageLoading/blob/v2.4.11/source/FFImageLoading.Shared/Helpers/MiniLogger.cs

[Takym]

画像ロガー関連については #226 取り込み後に Issue を建てる事とします。

[Takym]

FFImageLoadingLogger をFILと省略していましたが、判り難いと思いますので「画像ロガー」に書き換えました。

[b-wind]

FFImageLoading 自体は画像じゃなくて画像ローダーじゃ無いですかね。
Logger のクラス名は個人ルールに従うと FFILoadingLoggerAdaptor か MiniLoggerAdaptor にしたいところだけれど、どっかから指摘来そう。

[b-wind]

#226 の取り込み、まだ開発チームのレビューを経てないようなので取り込まれるかすら未定じゃ無いかなと、要らぬ心配をします。

[Takym]

僕は #226 の取り込みが完了して時間が空いてから、FFImageLoading 関連の Issue を建てようと思っていますが、必要であれば早めに立てて頂いても構いません。

[Takym]

#222 を作成した時にほぼ全てのクラスを確認した筈ですが、画像ロガーは削除しませんでしたね。何処からか参照されていた気がします。（見落としである可能性もあります）

現在確認しましたところ見落としでした。#115 で既に削除されている様なので問題はありませんが。

[b-wind]

どちらが先に検討されるのか分からないけれど、 #238 とは確実にコンフリクトしそう。
まぁ現状だと合わせるのもさほどの手間では無さそうですが。

[Takym]

全く同じ変更であれば衝突（コンフリクト）は発生しない筈ですので、時間がある時に DebugLogger と画像ロガーは削除する事にします。

追記

もし削除し忘れていたら催促して頂けると助かります。

[Takym]

LoggerService ではなく System.Diagnostics.Debug にログ出力している箇所を ExposureNotificationHandler などで発見しました。

こちらは DebugLogger の後に対応していく予定です。

[Takym]

DebugLogger と画像ロガーは削除しました。

[keiji]

LoggerService ではなく System.Diagnostics.Debug にログ出力している箇所を ExposureNotificationHandler などで発見しました。

ログは意図的に保存していない部分もあると思うので、ログレベルにご注意を！　TEKとか、Releaseビルドでは保存しない方が安全です。

[Takym]

Debug クラスへ出力している箇所は Debug レベルで LoggerService に出力する様に変更する予定です。

[Takym]

Debug.WriteLine の出力先を LoggerService.Debug に変更しました。

修正していて気が付いたのですが、LoggerService を使う事ができない箇所で Debug.WriteLine を使っていた箇所もありました。