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

(相關資料圖)

阿里妹導讀

本文主要從我們?yōu)槭裁葱枰狢R？CR面臨哪些挑戰(zhàn)？CR的最佳實踐幾個方面分析，希望可以給讀者一些參考。

為什么需要CR？

代碼質(zhì)量

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

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

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

SmartBear研究了一個歷時3月，包括10名開發(fā)人員完成的10,000行代碼的工程，通過引入CR在接下來的6個月中可以 節(jié)省近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，對 團隊內(nèi)新人的快速成長 也很有幫助。

卓越工程

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

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

CR面臨哪些挑戰(zhàn)？

挑戰(zhàn)1：CR的代碼改動范圍過大

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

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

挑戰(zhàn)2：CR對評審者全局知識要求很高

CR一般由團隊內(nèi)資深的技術同學進行，對于大型復雜項目的CR需要評審者對編碼規(guī)范、分布式架構設計原則、業(yè)務知識有全面的了解，舉個例子，下面是某服務提供的等級查詢接口關鍵代碼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);        }        //查詢?yōu)榭?，或者已過期發(fā)送消息，刷新等級-       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，下面是代碼評審者必須掌握的業(yè)務和技術知識：

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

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

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

CASE1：一個業(yè)務使用3個時間穿越開關

背景

時間穿越是營銷類業(yè)務系統(tǒng)最常使用的工具之一，通過全局控制，可以提前測試某個在未來開始的業(yè)務功能。筆者接觸到的一個業(yè)務由2個服務A、B組成，A是一個老應用，使用了一個開關，后來B又在不同業(yè)務場景中使用了2個新的開關，在一次CR中發(fā)現(xiàn)了一個業(yè)務重復使用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）原則，最理想的方案是把該業(yè)務用到的時間穿越開關統(tǒng)一由一個服務提供，因為時間穿越工具是借助動態(tài)配置中心推送開關到本地，然后做內(nèi)存計算；如果統(tǒng)一成服務后，A和B都需要依賴遠程服務調(diào)用，而B是一個高并發(fā)的使用場景，會有較大性能損耗。

觀點2：富客戶端

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

觀點3：配置統(tǒng)一，重復代碼三處收攏為兩處

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

總結(jié)：

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

CASE2：SSR（服務端渲染）API穩(wěn)定性優(yōu)化

背景

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

// 提交任務ioTaskList.stream().forEach(t ->futures.add(pool.submit(() ->t.service.invoke())));// 阻塞獲取任務結(jié)果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())));// 阻塞獲取任務結(jié)果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 {            // 根據(jù)service類別構造降級資源            String resourceName = \"RESOURCE_\" + name();            entry = SphU.entry(resourceName);            try {                // 未觸發(fā)降級，正常調(diào)用后端服務                return realInvoke(context);            } catch (Exception e) {                // 業(yè)務異常，記錄錯誤日志，返回出錯信息                return failureResult(context);            }        } catch (BlockException e) {            // 被降級，可以fail fast或返回兜底數(shù)據(jù)            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實現(xiàn)，融合了服務類型和自定義key構造降級資源            String resourceName = \"RESOURCE_\" + key(context);            entry = SphU.entry(resourceName);            try {                // 未觸發(fā)降級，正常調(diào)用后端服務                return realInvoke(context);            } catch (Exception e) {                // 業(yè)務異常，記錄錯誤日志，返回出錯信息                return failureResult(context);            }        } catch (BlockException e) {            // 被降級，可以fail fast或返回兜底數(shù)據(jù)            return degradeResult(context);        } finally {            entry.exit();        }    }        public abstract String key(ServiceContext context);    public abstract T realInvoke();}

總結(jié)：

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

團隊由CR引發(fā)的技術深入討論和持續(xù)優(yōu)化形成了這套自動化降級能力，上圖是實際線上運行效果，可以看到系統(tǒng)隨著依賴數(shù)據(jù)源服務RT的抖動實現(xiàn)了自動化自適應降級和恢復。

CR有沒有最佳實踐？

Code Review的邊界

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

出發(fā)點：程序員的初心

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

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

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

看得見的手：Team Leader的重視

喊口號沒有用，只有躬身入局。身邊幾位參加晉升同學CR的評審官普遍感受是：代碼質(zhì)量分布通常會團隊化，不要指望個別優(yōu)秀的同學帶動團隊的整體水平提升。確實如此，代碼質(zhì)量需要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編碼規(guī)約：https://github.com/alibaba/p3c 五種Code Review反模式：https://blogs.oracle.com/javamagazine/post/five-code-review-antipatterns Capers Jones對軟件質(zhì)量的研究分享：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

關鍵詞：