コードを安全にこの世から抹消していく

ota42y

·

Follow

Published in

FiNC Tech Blog

·

7 min read

·

Jun 30, 2017

--

Share

TL;DR

• メンテを楽にするにはメンテしないのが良いよ
• 古いコードは積極的に消していくとメンテしなくて良いよ
• 迂闊に消すとバグるから慎重にね
• アクセスログを使うと消せそうなのが探しやすいよ
• エラー検知ツールで使ってないかチェックすると安全だよ
• 少しづつでも頑張るしかないよ

はじめに

長く使われているリポジトリでは、必然的にメンテするべきコード量が増えていきます。そのため、変更に対する影響範囲の増大やテスト・ビルド時間の増加が起き、開発速度が下がっていきます。

コードのメンテを最も楽にする方法の一つは、消してメンテしないことなので、古いコードは適切にリファクタリングしたり、使われていないコードは削除するなど、増加するコードに対抗していく必要があります。

特にAPIサーバの場合、互換性が崩れる変更なので/v1/meの古いエンドポイントはそのままにし、新しいバージョンからは/v2/meの新しいエンドポイントを使う…という用に、移行のために古いコードが残りやすいです。
このような場合、移行してもう使われなくなった/v1/meをメンテし続けるのはコストがかかるため、一定のタイミングで消してしまいたいです。

実際にコードをどう消していくか

使われていなさそうなコード、消したいコードがあった場合に、そのコードの状況によってどういう手順を取るべきかは変わってきます。

今回は実際に消した場合の手順をまとめて紹介します。Railsアプリケーションを想定していますが、Rails特有の部分はありません。

消せそうなコードを見付ける

何はともあれ消せそうなコードを見付けるところからです。探す方法はいくつかあり、APIであればバージョンの低いエンドポイントから順に見ていったり、アクセスログとrake routesの結果からアクセスログにないroutesを出す、偶然見付ける等方法は様々です。

今回の場合は古いAPIを見ていた際に、過去に削除した機能に関するメソッドを見付け (Task#my_tasks メソッドとします)、対応するコードが消せないかと考え調査を始めたところ、呼び出し元のエンドポイント(/v1/tasksとします)が使われていなさそうなのを発見しました。また、rake rouets等でエンドポイント一覧を出し、後述のアクセスログを調べる方法でアクセスが無いものを洗い出していくのも有効です。

# /v1/tasks
def index
 @tasks = Task.my_tasks(user)
end

アクセスログを調べる

弊社ではもともとデータ分析用にアクセスログをAmazon Redshiftに保存し、Redash上で扱える環境を整えており、SQLを書くだけで簡単にエンドポイントへのアクセスログを集計できます。

今回消したい/v1/tasksの過去のアクセスログを調べると、使わなくなったバージョンをリリースしてからほぼアクセスが無くなり、ここ数ヶ月では一度もアクセスが無いという事がわかりました。そのため、もうけして良いとみなし、エンドポイントを削除することにしました。

エラー通知サービスを利用して使っているかを調べる

/v1/tasks/のエンドポイント消すだけならば、controllerから消して終わりますが、エンドポイントを消したことにより、その中で呼び出しているTask#my_tasksも消せる可能性があります。

多くの場合、メソッドの呼び出し状況を調べれば消せるか判断出来ますが、コードが複雑に絡み合っている場合、一見使っていなさそうに見えても実は使っているのではないか…といった不安を払拭できません。

このような場合、エラー通知を利用することで安全に使っていないかを調べることができます。

以下のように専用のエラーを通知し、その後元の処理をするようなコードに変更して本番でしばらく様子をみます。意に反して使われていればエラーが飛ぶので気がつきますし、ユーザから見たときの挙動は変わらないため、安全に使用状況を調べられます。この時、大量にエラーログが出ると他のエラーが埋もれたり、通知処理によるボトルネックが起きるので、わかっている箇所に関しては先に消しておくのをお勧めします。

弊社ではSentryを利用しており、Ravenを使ってエラー通知を管理しているため、Raven.notifyメソッドに専用のエラーを渡しています。弊社ではSentryへはfluentdを経由して送っているので、万が一大量に呼ばれる所にログを仕掛けてしまっても、直接ログ通知サービスに送るのに比べて、ある程度は耐えられるという利点もあります。(ログファイルを経由して送るのに比べるとまだ不安は残りますが)

def my_tasks(me)
  Raven.notify(Task::MyTasksCalled.new)  all_tasks = Task.where(user: me) # 既存コード 
  all_tasks.each……..
end

コードを消す

コード上にも呼んでいるところが無く、本番環境でも全くエラーが飛んでこないようであれば、ほぼ安心してコードを削除することができます。

その他

消したいコードが使われている場合

あたり前ですが消せません。

…とはいえ、実装やパフォーマンス、設計面の失敗などで使っているコードを消したいという場合もあります。そのような場合は、時間はかかりますが使っている内容を調べ、リファクタリングをする、別のエンドポイントに移行してもらうなどをし、消せる状態に持っていきましょう。

アクセス方法を制御する

iOSやAndroidアプリの場合は、バージョンアップがユーザに委ねられるためコントロールしにくいですが、内部の別サーバなどinternalな通信であればコントロール可能です。FiNCではほぼ全てのサーバがRubyを使っているため、クライアントライブラリ用のgemを提供し、別のエンドポイントへの移行を簡単にすることや、リクエスト時に、どこからのどのバージョンのgemでアクセスしたかといった情報を収集するという手法を一部で試しています。ただし、まだ導入したばかりで現時点では移行が必要な状況が起きていないので、続報にご期待ください。

最後に

メンテコストを下げるためには、消せるコードは可能な限り消していき、コードサイズを小さく保っておくのが効果的です。ただし、迂闊にコードを消すとバグの原因になってしまいます。そのため、様々な情報を駆使し、影響範囲の小さいものや安全なコードから消していくことで、安心かつ安全にコードサイズを小さくしていくことができます。

おまけ

この記事は前に書いたRuboCopを無理なく既存プロジェクトに導入するという記事で書いたRuboCopの取り組みから生まれました。RuboCop導入時に古いコードから大量の違反が検出され、テストもないし修正するコストが高く、かつ一見使っていなさそうに見えたので、修正してメンテし続けるより消してしまった方が今後のメンテコストも浮くし良いのでは？という背景からうまれました。不要なコードを消していくことで、上記のようなLinterの導入もとても楽になるので、積極的に抹消していくことをお勧めします。