LoginSignup
194
156

More than 1 year has passed since last update.

【Java】こんなログは嫌だ!

Last updated at Posted at 2019-06-14

INDEX

  • 概要
  • こんなログは嫌だ!
  • あとがき

概要

普段、Javaをベースにしたシステム開発をしている事が多いのですが。。。
他人が作ったシステムとか、OSSとか問題が起きたので「さあ、ログを見よう」と思ってもさっぱり意味不明なログしか出力されてない事が多くて叫びたくなるので、叫ぶ代わりに書き殴ってみようと思います。

D9DEjb0UcAAqJtI.jpg

こんなログは嫌だ!

1. そもそも出していない!

HogeService service = new HogeService();
service.init("aaa", "bbb");
if (!service.isInitError()) {
    service.execute();
}
  • まずは何か出力してみようや…

2. 全部【System.out.println("~")】だ!

if (service.isError()) {
    System.out.println("appropriate error message.");
}
  • コンソールに垂れ流すな!
  • ファイルに残ってないかも知れないぞ!

3. 例外を握りつぶしている!

try {
    ~()~
} catch (HogeException e) {
    // TODO 自動生成された catch ブロック
    e.printStackTrace();
}
  • だからコンソールに垂れ流s(ry
    • 画面コンソールに垂れ流すくらいならどうでも良いんだ…。勝手にシスログ出力なんてした日には面倒事に巻き込まれてしまう奴だ
    • あ、あとタイムスタンプが出ないかもよ?
  • IDEが自動生成したコードまんま残ってるやん!
  • 処理を続行するのか、中断させるのかの判断も必要だし、基本やっちゃダメなやつ

4. 例外を握りつぶしていないと主張されたが、やっぱり潰されていた!

try {
    ~()~
} catch (HogeException e) {
    System.out.println("error message.");
}
  • そろそろロガー使お?
    • 「レビューで言われたログ対応しました。確認お願いします!」と自称経験者から言われた時は、もう指摘せずにこっちでさっさと直してしまおうかと悩んだ。
  • ちなみに↑のサンプルだと【e.printStackTrace()】より情報少ないからね?

5. ロガーのインスタンスを作っているだけだ!

public class HogeService {
    private Logger logger = LoggerFactory.getLog(HogeService.class);
    // ↑ これ以降、loggerインスタンスを使っている箇所がない
    ~()~
}
  • 惜しい!もう少しでロガーを使ったログ出力が出来るぞ!!
  • コピペされたコードにありがち

6. そもそも他人任せだ!

public class HogeService {
    public void execute() {
        try {
            ~()~
        } catch (HogeServiceException e) {
            throw new RuntimeException(e);
        }
    }
}
  • 本当は【あり】な実装なんだけど…
  • いや、例外キャッチ強制を無意味に強制無しにするのはどうか…いやいや…
  • 続く

7. 当然、呼出元でキャッチされていない

HogeService service = new HogeService();
service.execute();
  • 続き
  • ああ、やっぱり取ってなかった。悪い予感はしてたんだよ。
  • 動きが分かっててやらないと、WEBシステムならブラウザに白い画面に【Internal Server Error】とか出ちゃう

8. 全部デバッグレベルだ!

HogeService hs = new HogeService();
hs.execute();
if (hs.isError()) {
    logger.debug("fatal error.");
}
FugaService fs = new FugaService();
fs.execute();
if (fs.isError()) {
    logger.debug("critical error.");
}
  • これ本番環境でログに出ない奴だから!普通は「infoレベルで運用」とかだから!
    • メッセージだけやばい感じにしても意味ないから!!
  • これに加えてログローテーションを仕込んでいないとある日ディスクフルに見舞われるが、きっちりディスクサイズ等を監視に入れておかないと原因特定まで時間がかかるぞ…クックック。

9. 全部エラーレベルだ!

String userName = request.getParamter("userName");
if (StringUtils.isEmpty(userName)) {
    logger.error("validation error.");
}
service = new HogeService();
service.execute();
if (service.isError()) {
    logger.error("minor error.");
}
  • これお客さんからの電話止まらなくなる奴だから!
  • 入力チェックをエラーレベルとかやり過ぎで、ユーザが変な文字入れる度に監視に引っかかっちゃうから!

10. エラーが発生した事しか分からない

String arg = request.getParameter("hoge");
try {
    return Integer.parseInt(arg);
} catch (NumberFormatException e) {
    logger.info("An error has occurred.");
}
  • エラーが起きた。そんな事は分かってるんだ。
  • 本番環境でデバッガを走らせるつもりなのかい?

11. エラーの原因が分からない

UserService us = new UserService();
UserInfo user = us.getUser(request.getSession());
String age = request.getParameter("age");
try {
    user.setAge(Integer.parseInt(age));
    us.update(user);
} catch (NumberFormatException e) {
    logger.info("conversion(string -> int) error.");
}
  • 文字を数字に変換したかったんだろ?もう少しだ!もう少し情報があると原因究明に役立つんだ!
  • "age"の中にはどんな値が入っていたんだい?

12. 誰がエラーを起こしたのか分からない

UserService us = new UserService();
UserInfo user = us.getUser(request.getSession());
String age = request.getParameter("age");
try {
    user.setAge(Integer.parseInt(age));
    us.update(user);
} catch (InvalidUserInfoException e) {
    logger.error("User data is invalid. / age=[" + age + "]");
}
  • 物によっては誰がやらかしたのかを特定する必要もある
    • ADに入っているユーザ情報が間違ってたとか
    • logger.error("User data is invalid. / age=[" + age + "], userName=[" + user.getName() + "]");
  • そういやObjectは、文字列として出力しようとすると、toString()が呼ばれるので、【UserInfo#toString()】をOverrideしておけば綺麗に出力できるが注意も必要
    • そういや、lombokとかはアノテーションで自動生成してくれたな。フィールド事に除外設定も可能。スゴイ

13. そうは言っても、これは出し過ぎだ!

UserService us = new UserService();
UserInfo user = us.getUser(request.getSession());
String age = request.getParameter("age");
try {
    user.setAge(Integer.parseInt(age));
    us.update(user);
} catch (InvalidUserInfoException e) {
    logger.error("User data is invalid. / userName=" + user.getUserName()
        + ", age=" + user.getAge()
        + ", email=" + user.getEmail()
        + ", password=" + user.getPassword() + "]";
}
  • 本当は名前とかだけで特定出来たのに、余計なものも出しちゃってセキュリティ部門から怒られる奴
  • 追記分、Kilisameさん感謝!

14. 使わないメッセージを作っている

StringBuilder msg = new StringBuilder();
HogeService hs = new HogeService();
hs.execute();
if (hs.isError()) {
    msg.append("hoge's error.").append('\n');
}
FugaService fs = new FugaService();
fs.execute();
if (fs.isError()) {
    msg.append("fuga's error.").append('\n');
}
if (logger.isWarning()) {
    logger.warn(msg);
}
  • これはほぼイチャモンで、実装テクニックだから運用保守マンはスルーでOK。
  • 以下の流れで実装すると、ログレベルがerrorの時、メッセージ生成コストが無駄になってしまう
    • 処理に応じてメッセージを組み立てる
    • ログレベルを調べる
    • warnレベル以上だったらログ出力する
  • 対処としてはこんな感じか
    • Javaバージョン関わらずに出来るのは、ログレベルの判定箇所を工夫するなど
    • Java8以降なら、Supplierクラスで遅延実行するロガーに挑戦しても良いと思う

あとがき

  • 何か気がついたら追記するかも知れません。
  • 6/18 Kilisameさんのコメントが個人的にあるあるだったので、「そうは言っても、これは出し過ぎだ!)」を追加して後ろの番号ずらしました。ありがとうございました!
  • では ノシ
194
156
11

Register as a new user and use Qiita more conveniently

  1. You get articles that match your needs
  2. You can efficiently read back useful information
  3. You can use dark theme
What you can do with signing up
194
156