マイクロサービスを横断したGoのコードレビュー

DMM.go #7の登壇資料です。
https://dmm.connpass.com/event/310876/

Transcript

1. © DMM.com マイクロサービスを横断した Goのコードレビュー DMM.go #7 プラットフォーム事業本部 いっぬ（@yuyu-hf） 1
2. © DMM.com 自己紹介 いっぬ（@yuyu_hf） ➢ 合同会社DMM.com（2019.04 - 現在） ◦ プラットフォーム事業本部 マイクロサービスアーキテクトグループ 認可チーム リードエンジニア ➢ 認可プロダクトをリプレースするお仕事 ◦ PHP -> Go ◦ MySQL -> TiDB Cloud ◦ オンプレ -> GKE/EKS ➢ 認可チームの採用 ◦ 2名(2023.03) -> 4名(2024.03) ◦ 引き続き採用しています 2
3. © DMM.com 今日話すこと DMMの共通基盤ではマイクロサービスを採用しています 今日は各マイクロサービスの開発チームに Goのコードレビューのサービスを提供した話をします サービスの内容や運用方法、実際にやってみてどうだったかについて話します 前回のDMM.go #6でpospomeさんが発表した内容の一部の具体的な話です 「組織のコード品質を向上させる “レビューシステム ”の取り組み」のレビューシステム Version 1.0の話 https://speakerdeck.com/pospome/zu-zhi-nokodopin-zhi-woxiang-shang-saseru-rebiyusisutemu-noqu-rizu-mi?slide=18 3
4. © DMM.com 今日話すこと ➢ 自チーム外へのGoのコードレビューを紹介 ◦ コードレビューを実施した背景 ◦ コードレビューするときのルール ◦ コードレビューした具体的な内容 ◦ コードレビューの内容をレビューしてもらう仕組み ◦ 半年に1回、利用者アンケートを実施 ➢ 自チーム外へのGoのコードレビューをやってみた感想 4
5. © DMM.com DMMプラットフォーム DMMプラットフォームとは DMMの各サービスで共通して使われる基盤です 会員の管理、ユーザーの認証 /認可、各種決済周り、 DMMポイントやtoreta+といった電子マネーの仕組みを提 供しています 5
6. © DMM.com DMMプラットフォームのGoのユースケース DMMプラットフォームでは技術戦略として Goを採用しています ➢ バックエンドAPIを開発するときはGoを使う ◦ 既存のアプリケーションは Goでリライトする ➢ マイクロサービス共通のエコシステムでも Goを採用した ◦ 負荷試験基盤で試験スクリプトを Goで書けるようにした 120名の開発組織を支える、技術マネジメントと選定 https://speakerdeck.com/pospome/120ming-nokai-fa-zu-zhi-wozhi-eru-ji-shu-manezimentotoxuan-ding DMMプラットフォームを支える負荷試験基盤 https://speakerdeck.com/yuyu_hf/cndt-2022-dmm-load-testing-platform-for-dmm-platform 6
7. © DMM.com 自チーム外にGoのコードレビューを始めた背景 昔のDMMプラットフォームはPHP、Java、Go、Scala、Kotlinなど様々なプログラミング言語で書かれたプロダクト が混在するカオスな状態 ↓ DMMプラットフォームの技術戦略として Goを採用 ↓ DMMプラットフォームでは Goを使った開発をするのが初めての人も多かった Goに詳しい人に技術支援してもらったほうが開発効率が上がるので、 Goに詳しい人にコードレビューしてもらお う！ ↓ マイクロサービスアーキテクトグループで先行して Goを採用していたので、自チーム外への Goのコードレビュー の仕組みを提供開始 7
8. © DMM.com 自チーム外にGoのコードレビューを提供 ➢ コードレビューの依頼方法 ◦ 各チームからプロダクト単位でコードレビューを任意で依頼してもらう ➢ コードレビューするときのルール ◦ コードレビューの指摘内容は強制ではなく提案 ◦ コードレビューはベストエフォートで行う ◦ PRのmerge後に追加のコードレビューや書いたコードレビューについて訂正する可能性がある ➢ 1週間に1度、上司がコードレビュー内容をレビューする 8
9. © DMM.com 実際にレビューした内容①（Goの書き方） コードレビューを開始してから半年くらいは Goの基本的な書き方についてレビューすることが多いです ➢ Goの文法 ➢ Goの書き方の紹介 ◦ Go本体のコード ◦ Goの有名なライブラリのコード ◦ コーディングスタイルガイド ➢ 名前付き戻り値の扱い方 ➢ 値レシーバ、ポインタレシーバの使い分け ➢ http.Response.BodyをCloseし忘れの注意 ➢ deprecatedのpackageを使っている場合の注意（ ex. io/ioutil package） ➢ etc… GoのNamed return valueについてメリデメを考える https://zenn.dev/yuyu_hf/articles/c7ab8e435509d2 9
10. © DMM.com レビュー例 Goの変数の命名方法についてレビュー 変数のスコープが短い or 型から何の用途で使われる変数なのかわかる場合のみ 変数名を簡易的な名前にしてもいい 10
11. © DMM.com レビュー例 名前付き戻り値の使い方 11 元のコード
12. © DMM.com 実際にレビューした内容②（一般的な開発の知識） ➢ プログラミング原則の説明 ◦ 変数や関数の命名 ◦ DRY ➢ コメントの書き方 ➢ モジュールの結合度 ➢ デザインパターンの紹介 ◦ ドメインモデリングパターン ◦ ファーストクラスコレクション ➢ トランザクションに関する実装方法 ➢ バリデーションの実装方法 ➢ MVCやオニオンアーキテクチャの導入支援 ➢ DDDの導入支援 ➢ etc… 12 社内のプロダクト開発でコメントを書くときの考え方 https://zenn.dev/yuyu_hf/articles/64061802544c87 ファーストクラスコレクション https://zenn.dev/yuyu_hf/articles/93ba0a734208b3 トランザクションを考慮した実装について考える https://zenn.dev/yuyu_hf/articles/df92f9cd0caa0f
13. © DMM.com レビュー例 変数や関数の命名 13
14. © DMM.com レビュー例 変数や関数の命名 14
15. © DMM.com レビュー例 ファーストクラスコレクションの使い方 15
16. © DMM.com レビュー例 トランザクション処理に関する実装 16
17. © DMM.com 実際にレビューした内容③（チーム間の知見の共有） 私がチーム間で知見を共有するハブとなって、別チームの知見やプロダクトの実装の紹介をしていました ➢ Goのライブラリの使い方 ➢ アクセスログを出力するミドルウェアの実装例 ➢ エラーハンドリングの仕組みの実装例 ➢ Datadogの分散トレーシング用の http clientの実装方法 ➢ MVCやオニオンアーキテクチャの Goでの実装例 ➢ etc… 17
18. © DMM.com コードレビューのレビュー コードレビューしたPull Requestをspreadsheetで管理しています 1週間に1回、コードレビューした内容について上司からレビューを受けます 18
19. © DMM.com コードレビューのレビュー コードレビューの指摘内容やコメントについて上司がレビューします よくレビューされた内容 ➢ レビューで提案したら提案のメリデメを必ず説明する ◦ 私のコメント「...にするのはどうでしょうか？」 上司「メリデメを説明しないと指摘されたから直すになってしまってこの先も同じレビューをし続けるこ とになりますよ(´・ω・`)」 ◦ 私のコメント「DDD的に...」 上司「デメリットが無いなら今の実装でいいですよ (´・ω・`)」 ➢ 抽象的な言葉を使わない。具体的に説明する ◦ 私のコメント「UI層の責務で...」 上司「責務って何ですか？ふわふわした言葉で説明しないでください (´・ω・`)」 19
20. © DMM.com 半年に1回利用者アンケートを実施 21名中17名が別の開発案件でもコードレビューをぜひ受けたいと回答しました 20
21. © DMM.com 半年に1回利用者アンケートを実施 21
22. © DMM.com 自チーム外のコードレビューをやって良かったこと ➢ Goを初めて書くチームに Goを書くときのノウハウを共有できた ◦ 半年くらいでGoの書き方に関するレビューはほとんど無くなった ➢ 品質の高いコードを書くノウハウを共有できた ◦ チーム内で品質の高いコードを維持するためのレビューができるようになった ◦ チーム内に有識者がいない設計手法を導入する支援ができた ➢ チームを横断したノウハウの共有ができた ◦ 問題に当たったときに同じ問題に当たった別のチームの解決策を共有して開発効率を上げた ➢ マイクロサービスを開発する各チームのコーディングレベルの課題が可視化された 22
23. © DMM.com 大変だったこと①（ハレーションが発生した） ハレーションの内容 基本的なプログラミング原則を指摘するレビューを口頭で実施したときに強く反論された 当時の対応 私の上司にエスカレーションして、レビュー先のチームとマネージャーを含めて話し合いが行われた 原因 & こうすればよかった コードレビューは提案だがレビューされた側に「強制」に見えてしまったかもしれない テキストコミュニケーションだけだと「提案感」が伝わらなかった可能性がある 普段から口頭でのコミュニケーションを併用して「提案感」をわかってもらう努力をすれば良かった 23
24. © DMM.com 大変だったこと②（時期によって残業が多かった） 多いときには4つのプロダクトを担当して 1日10PR以上を1人でレビューしていた PRが作成されたらその日中にレビューする必要があったため残業しました 原因 コードレビュー先のチーム内でコーディングルールができていない状態だったので、コードレビューでコーディング ルールを守ることを徹底していました コードレビューはベストエフォートなので、コードレビューが遅れると PRがマージされてしまいます コーディングルールを守ってないコードが追加されると、コーディングルールを守ってないコードを参考にしてコー ドを書く人が出てきます もっとこうすれば良かった PRをマージした後でもコードを改善してもらうように働きかければ良かった spreadsheetなどでコードを改善するための Issueを管理し開発のXX%を改善に当てるなどの提案をする 24
25. © DMM.com 今後の展望 1年くらいレビューしていると Goの書き方や品質の高いコードの書き方についてレビューすることが少なくなりまし た 今はコードレビューだけでなく開発生産性を上げるための取り組みを各チームに提供しています PRのリードタイムの削減 プルリクのリードタイムを 5分の1に劇的改善。科学的思考とオーナーシップで変革する DMM.comのエンジニア組織 https://blog.findy-team.io/posts/interview_dmm_01/ 開発生産性を上げることを専任で取り組むために Developer Productivity Team の創設 組織全体で開発生産性に取り組むために 専門チームを作った話 https://speakerdeck.com/pospome/zu-zhi-quan-ti-dekai-fa-sheng-chan-xing-niqu-rizu-mutameni-zhuan-men-timuwozuo-tutahua 25
26. © DMM.com 今日話したこと ➢ 自チーム外へのコードレビューを紹介 ◦ コードレビューを実施した背景 ◦ コードレビューするときのルール ◦ コードレビューした具体的な内容 ◦ コードレビューの内容をレビューしてもらう仕組み ◦ 半年に1回利用者アンケートを実施 ➢ 自チーム外へのコードレビューをやってみた感想 26