三覺 阿里開發者 2023-07-04 09:02 發表于浙江

(相關資料圖)

阿里妹導讀

本文主要從我們為什么需要CR？CR面臨哪些挑戰？CR的最佳實踐幾個方面分析，希望可以給讀者一些參考。

為什么需要CR？

代碼質量

定性來看，大家都認可Code Review（后文簡稱CR）能 顯著改善代碼質量 ，但國內量化的研究結果比較少，以下引用業界比較知名的幾個定量研究結果：

Capers Jones分析了超過12,000個軟件開發項目，其中使用正式代碼審查的項目， 潛在缺陷發現率約在60-65%之間 ；大部分的測試，潛在缺陷發現率僅在30%左右。

Steve McConnel在《Code Complete》中提到：僅僅依靠軟件測試效能有限–單測平均缺陷發現率只有25%，功能測試35%，集成測試45%，相反， 設計和代碼審查可以達到55%到60%。

SmartBear研究了一個歷時3月，包括10名開發人員完成的10,000行代碼的工程，通過引入CR在接下來的6個月中可以 節省近6成的bug修復成本 。

技術交流

SmartBear研究報告中這段話比較能表達CR對技術交流的價值，引用如下：

Actually writing the source code, however, is a solitary activity . Since developers tend to create code in quiet places, collaboration is limited to occasional whiteboard drawings and a few shared interfaces. No one catches the obvious bugs; no one is making sure the documentation matches the code. Peer code review puts the collaborative element back into this phase of the software development process.

Google認為CR參與 塑造了公司的工程師文化 ，CR也與筆者所在部門一貫倡導的「 極致透明 」的文化相契合，資深同學的CR，對 團隊內新人的快速成長 也很有幫助。

卓越工程

Linux的創始人Linus Torvalds有句名言：Talk is cheap, Show me the code，代碼是程序員的作品，CR只是一種提升代碼質量的工具， 敢于Show出自己的代碼，并用開放的心態去優化完善 ，才是每個程序員審視自我，從優秀到卓越的關鍵所在。

既然CR好處這么多，大多團隊也在實踐，但為什么效果差強人意呢，主要是CR在大型項目實踐中面臨諸多挑戰。

CR面臨哪些挑戰？

挑戰1：CR的代碼改動范圍過大

筆者觀察，很多項目落實CR的最大挑戰是項目進度壓力很大，發布計劃倒排根本沒有給CR預留時間，所以大部分CR是在臨近提測前（甚至有些是邊測試邊進行）集中進行，面對動輒上千行的代碼變動，評審者需要花大量時間和代碼提交者交流了解業務邏輯，迫于時間壓力大多只會檢查最基本的編碼規范問題，而沒有達到CR預期的效果。SmartBear公司對 CR 節奏的研究指出：每次大于400行的CR每千行代碼缺陷發現率幾乎為零。

那么怎樣的提交粒度比較合適呢？筆者的經驗是和 單元測試case匹配 （這里問題來了，沒時間寫單元測試怎么辦，本文姊妹篇再來聊聊單元測試），完成一個功能，跑一個單元測試驗證邏輯，然后commit一次。如下圖，Aone（阿里內部的研發平臺）提供的功能內置支持按照提交版本分批DIFF，分批Review。

挑戰2：CR對評審者全局知識要求很高

CR一般由團隊內資深的技術同學進行，對于大型復雜項目的CR需要評審者對編碼規范、分布式架構設計原則、業務知識有全面的了解，舉個例子，下面是某服務提供的等級查詢接口關鍵代碼CR片段：

- public Level queryLevel(LevelQueryRequest request) {+ public Level queryLevelWithExpireRefresh(LevelQueryRequest request) {    Level result = levelRepository.findLevelWithoutInit(request.getId());    if (null == result || isExpired(result.getEndTime())) {        // 如果等級為空，兜底返回L0；等級已過期，實時返回默認等級，并異步刷新        if (result == null) {            result = levelRepository.buildInitLevel(request.getId(), LevelEnum.L0);        }        //查詢為空，或者已過期發送消息，刷新等級-       LevelRefreshRequest refreshRequest = buildRefreshRequest(request);-       levelWriteService.refreshLevel(message.getId(), refreshRequest);        +       RefreshMessage refreshMessage = buildRefreshMessage(request);+       refreshMessageProducer.sendMessage(refreshMessage);    }    return result;}- public class RefreshMessageListener extends AbstractMessageListener {+ public class RefreshMessageListener extends AbstractOrderlyMessageListener {    @Autowired-    private LevelWriteService levelWriteService; +    private LevelWriteRegionalService levelWriteRegionalService;    @Override    protected boolean process(String tags, String msgId, String receivedMsg) {        RefreshMessage message = JSON.parseObject(receivedMsg, RefreshMessage.class);        if (message == null || message.getId() == null) {            log.warn(\"message is invalid, ignored, src={}\", receivedMsg);            return true;        }        LevelRefreshRequest refreshRequest = buildRefreshRequest(message);-       levelWriteService.refreshLevel(message.getId(), refreshRequest);+       levelWriteRegionalService.refreshLevel(message.getId(), refreshRequest);        return true;    }}

面對上面代碼改動，要進行富有成效的CR，下面是代碼評審者必須掌握的業務和技術知識：

為什么存在等級為空的情況？ 為什么要設計成讀時寫？ 為什么不是直接計算等級，而需要用消息隊列？ 為什么要用Regional（區域化）接口和有序消息刷新等級？

挑戰3：CR價值最大化需要團隊具備卓越工程基因

前文提到CR有助于團隊內的技術交流，下面是幾個筆者親歷的Case，通過對典型CR問題的廣泛討論不僅提升了業務代碼的質量，而且探索到了技術創新點，逐步建立起團隊追求技術卓越的氛圍：

CASE1：一個業務使用3個時間穿越開關

背景

時間穿越是營銷類業務系統最常使用的工具之一，通過全局控制，可以提前測試某個在未來開始的業務功能。筆者接觸到的一個業務由2個服務A、B組成，A是一個老應用，使用了一個開關，后來B又在不同業務場景中使用了2個新的開關，在一次CR中發現了一個業務重復使用3個不同開關的問題，由此展開了一次討論。

private static final String CODE = \"BENEFIT_TIME_THROUGH\";public Date driftedNow(String userId) {    try {        TimeMockResultresult = timeThroughService.getFutureTime(CODE, userId);        if (result.isSuccess()) {            return new Date(result.getData());        }    } catch (Throwable t) {        log.error(\"timeThroughService error. userId={}\", userId, t);    }    return new Date();}

觀點1：徹底服務化

按照DRY（Don"t Repeat Yourself）原則，最理想的方案是把該業務用到的時間穿越開關統一由一個服務提供，因為時間穿越工具是借助動態配置中心推送開關到本地，然后做內存計算；如果統一成服務后，A和B都需要依賴遠程服務調用，而B是一個高并發的使用場景，會有較大性能損耗。

觀點2：富客戶端

把統一開關包裝成一個三方庫，獨立提供jar包供A、B服務分別依賴，這樣解決了前面方案的性能消耗問題。但兩個應用需要同步做更新和升級。

觀點3：配置統一，重復代碼三處收攏為兩處

該觀點認為徹底服務化和富客戶端屬于兩種極端，可以采取折中方案容忍部分代碼重復，但使用相同的時間穿越開關。

總結：

這個Case的討論涉及到一個公共邏輯抽取的方案權衡問題，進程內調用的富客戶端性能損耗低，但后期維護和升級困難，而且過于復雜的客戶端邏輯容易引發依賴方包沖突、啟動耗時增加等問題；徹底服務化只需要保持接口契約一致可以實現較快迭代，但對服務提供者SLA要求高；為了平衡前兩者的問題，微服務架構中的SideCar模式則是在功能性的應用容器旁部署另一個非功能性容器，使得開發團隊可以對主應用和SideCar進行獨立管理。關于這個問題網上有很多討論內容讀者可以進一步了解學習。

CASE2：SSR（服務端渲染）API穩定性優化

背景

上圖是一個典型的服務端渲染服務架構，SSR服務通過加載配置，對每個模塊進行獨立數據組裝，并整體返回結果到端側。一般應用在電商系統復雜只讀頁面的動態搭建，如首頁、商品詳情頁、導購頻道等。下面是組裝數據部分的待評審代碼片段。

// 提交任務ioTaskList.stream().forEach(t ->futures.add(pool.submit(() ->t.service.invoke())));// 阻塞獲取任務結果futures.stream().forEach(f ->{    try {        result.add(f.get());    } catch (Exception e) {        log.error(e.getMessage(), e);    }});

Step1：增加固定超時控制

// 提交任務ioTaskList.stream().forEach(t ->futures.add(pool.submit(() ->t.service.invoke())));// 阻塞獲取任務結果futures.stream().forEach(f ->{    try {-       result.add(f.get());+       result.add(f.get(1000, TimeUnit.MICROSECONDS));    } catch (Exception e) {        log.error(e.getMessage(), e);    }});

Step2：自適應超時控制

public abstract class BaseServiceimplements Service {    @Override    public T invoke(ServiceContext context) {        Entry entry = null;        try {            // 根據service類別構造降級資源            String resourceName = \"RESOURCE_\" + name();            entry = SphU.entry(resourceName);            try {                // 未觸發降級，正常調用后端服務                return realInvoke(context);            } catch (Exception e) {                // 業務異常，記錄錯誤日志，返回出錯信息                return failureResult(context);            }        } catch (BlockException e) {            // 被降級，可以fail fast或返回兜底數據            return degradeResult(context);        } finally {            entry.exit();        }    }    public abstract T realInvoke();}

Step3：自適應超時控制+自定義資源key

public abstract class BaseServiceimplements Service {    @Override    public T invoke(ServiceContext context) {        Entry entry = null;        try {            // 這里的key由service實現，融合了服務類型和自定義key構造降級資源            String resourceName = \"RESOURCE_\" + key(context);            entry = SphU.entry(resourceName);            try {                // 未觸發降級，正常調用后端服務                return realInvoke(context);            } catch (Exception e) {                // 業務異常，記錄錯誤日志，返回出錯信息                return failureResult(context);            }        } catch (BlockException e) {            // 被降級，可以fail fast或返回兜底數據            return degradeResult(context);        } finally {            entry.exit();        }    }        public abstract String key(ServiceContext context);    public abstract T realInvoke();}

總結：

Step1很容易理解，增加1000ms超時設置可以避免某個數據源嚴重超時導致整個渲染API不穩定，做到fail fast，但核心挑戰在于多長的超時時間算合理；Step2通過依賴降級組件，根據不同數據源服務設置不同超時時間，實現了自適應超時控制；Step3相比Step2改動非常小，不了解業務背景可能不清楚它們的區別，Step2的降級控制作用在服務類別上，比如營銷服務、推薦服務各自觸發降級，但還有一類數據源服務其實是網關類型，內部耗時會根據某個或某些參數不同有較大差異，例如TPP（阿里內部的算法平臺，不同算法邏輯共享一個網關API，但不同算法復雜度耗時差異巨大）服務就是一個典型，所以Step3允許自定義key()實現更精細的超時控制。

團隊由CR引發的技術深入討論和持續優化形成了這套自動化降級能力，上圖是實際線上運行效果，可以看到系統隨著依賴數據源服務RT的抖動實現了自動化自適應降級和恢復。

CR有沒有最佳實踐？

Code Review的邊界

對于什么是一個好代碼，上圖從可靠、可維護和功能完備做了劃分。筆者認為CR并非包治百病的銀彈，它也有它的能力邊界。把設計方案交給設計評審，把業務邏輯驗證交給單測；把編碼規范交給靜態代碼掃描（Static Code Analysis），剩下部分再由Peer Review做最后一道把關。CR引發的技術傳承、技術交流以及由此形成的追求卓越的團隊文化，才是它的最大價值。

出發點：程序員的初心

歸根結底，程序員的好奇心和匠心才是提升代碼質量的根本，目前筆者所在部門已經在晉升考核中增加了CR環節，爛代碼會被一票否決，這就需要日常工作中不斷追求技術卓越，在平時多下功夫。

看不見的手：自動代碼掃描

之前在某社區看到有個熱帖討論程序員的工作是不是勞動密集型，某個回帖比較形象「我們的工作本應是CPU密集型，結果卻成了IO密集型」。基本的編碼規范完全可以借助代碼自動化掃描識別，而這個占比也是比較高，可以有效降低CR成本。業界的CheckStyle、FindBug都有完善的CI/CD插件支持，阿里云也提供了IDE 智能編碼插件 ，內置了編碼規范支持。

看得見的手：Team Leader的重視

喊口號沒有用，只有躬身入局。身邊幾位參加晉升同學CR的評審官普遍感受是：代碼質量分布通常會團隊化，不要指望個別優秀的同學帶動團隊的整體水平提升。確實如此，代碼質量需要Team Leader高頻參與CR，技術文化的形成需要主管以身作則。

參考：

https://en.wikipedia.org/wiki/Code_review https://smartbear.com/learn/code-review/agile-code-review-process/ Lessons From Google: How Code Reviews Build Company Culture 阿里巴巴Java編碼規約：https://github.com/alibaba/p3c 五種Code Review反模式：https://blogs.oracle.com/javamagazine/post/five-code-review-antipatterns Capers Jones對軟件質量的研究分享：http://sqgne.org/presentations/2012-13/Jones-Sep-2012.pdf Modern Code Review: A Case Study at Google：https://sback.it/publications/icse2018seip.pdf 智能編碼插件：https://developer.aliyun.com/tool/cosy

關鍵詞：