新人が書いたGoのAWS Lambdaをレビューして学んだこと、困っていること

f:id:kidani_a:20190117094030j:plain

半年くらい前に来た新人が、先日Goで書いたAWS Lambdaのコードを見せてくれた。それをレビューした際に、色々学びというか気づいたことがあったのでメモしておく。

 

 

[AWS SDKの初期化はグローバルに]

docs.aws.amazon.com

すべての呼び出しで変数/オブジェクトの再初期化を制限します。代わりに、静的初期化/コンストラクタ、グローバル/静的変数、およびシングルトンを使用します。前の呼び出しで確立した接続 (HTTP やデータベースなど) をキープアライブにして再利用します。

 

しれっと書かれているが、ハンドラ関数の中で毎回の呼び出しごとにsvc := dynamodb.New(sess, config)とするのではなく、グローバルいうこと。Lambdaに触れて結構たつが、自分でもたまに忘れてやってしまうので自戒。

 

www.slideshare.net

西谷さんの資料でも43ページで説明されている。

 

検証記事も上がっていた。

recipe.kc-cloud.jp

 

関連して初期化周りでいうと、Lambdaのコンテナが再利用されるという認識がそもそもなかったようで、ハンドラ関数の中で毎回環境変数の値をログに出力していた。コンテナが使い回される間、環境変数は変わらないなので、init()関数などでバリデーションとログ出力を同時にやってはどうか、と提案した。

 

[ハンドラ関数とコアロジックを分離してテストを書く]

レビューしたコードは簡単に言うと次のような感じだった。

package main

import (
        ...(略)...
        "github.com/aws/aws-lambda-go/lambda"
)

type Request struct {
    ...(略)...
}

type Response struct {
    ...(略)...
}

func handler(req Request) (Response, error) {
    if !isValid(req) {
        return Response{}, errors.New("invalid")
    }
    
    // 条件分岐しつつ、エラーハンドリングしつつ、DBアクセスとかたくさん

    // いい感じにレスポンスを返す
    return Response{}, nil
}

func main() {
    lambda.Start(handler)
}

 

一部、バリデーションなどのロジックは別ファイルに切り出されてはいたものの、ハンドラ関数に色々なロジックが混ざっており、かつテストコードがない状態だった。

 

先ほどのAWS Lambda 関数を使用する際のベストプラクティスというドキュメントにも書かれている通り、これではやはり読みづらいし、そもそもテストが書きづらいしとあまりいいことがない。

 

テストコードはあったものの、処理をコピペしており大変そうな部分があったので、コードを分離した後のテストについては、先日のブログにも書いたVS Codeのツールを紹介しておいた。

kdnakt.hatenablog.com

 

それ以外にもGoのテストの基本についてはbudougumiさんのブログが勉強になる。テストデータのディレクトリや拡張子のルールは知らなかった。

budougumi0617.github.io

 

[Golang, AWS Lambdaで困っていること]

と、いくつかは改善の提案ができたものの、いくつか困っているところもある。

 

プライベート関数しかない.goファイル

Lambda限定の困りごとというわけではないが、多分開発途中で利用していたと思われる、プライベート関数しかないファイルがリポジトリに存在していた。go vetでチェックしても引っかからない。-unreachableはこういう時に使うんじゃないのか……とも思ったがそうでもないらしい。

他の静的解析ツールだったらチェックできたりするんだろうか。

golang.org

 

ともあれこのファイルに関する確認の過程で、Goのプライベート関数がJavaでいうパッケージ・プライベートと同じスコープになっている、という学びを得た。他の言語でも自分の最初の言語であるJavaをベースに勝手な理解をしてしまっている部分がありそうなので、気をつけなければ。 

 

適切なエラーをリターンする

こんな関数があった。

func myFunction(param string) (string, error) {
    err1 := check1(param)
    if err1 != nil { return "", err1 } err2 := check2(param)
if err2 != nil { return "", err1 } return param }

 

check2でerr2が返ってきているのに、リターンするエラーを間違えている。

これはどう解消するのが良いんだろう。if err1 := check1(param); err1 != nil { ... }にすればerr1のスコープを限定できるから一応防げている気はするがなんか冗長だ。

そういうもんだと思って慣れるしかないのだろうか。

 

返り値ハンドリング

エラーに限った話ではないが、ライブラリのメソッドを呼び出した際に、errorが返り値になっているにもかかわらず上記のようなif文を書いて例外処理を行なっていないようなケースも見られた。

go vetの返り値を利用していない判定はwell-knownな関数呼び出しのケースだけとのことなので、これは目でひとつずつ追っていくしかないのだろうか……。

 

あるいはやはり他の静的解析ツールも導入すべきか。

 

複数のLambda関数のまとめかた

レビューしたリポジトリには似たような処理をするLambda関数がいくつか存在していた。直感的には、1つのバイナリにまとめて、API Gatewayから渡ってくるリクエストパラメータを利用してLambdaのハンドラの中でswitch文とかで分岐させるとかすると色々捗りそう、と思ったけれどどうなんだろう。

 

1つにまとめた場合、良さそうなポイントとしては以下。

  • 監視する対象のLambda関数が減る
  • ロギングなどの社内ライブラリ的な共通のコードのバージョンアップが楽
  • Lambdaの同時実行数の制御を関数ごとに考えなくてよくなる

 

1つにまとめた場合、辛そうなポイントとしては以下。

  • バイナリのサイズが増えるのでコールドスタートが少し遅くなりそう
  • バージョンアップが同時になるので内部的に別の関数が影響を受ける恐れがある
  • トラブル対応時にまとめられた関数全体が影響を受ける

 

やはりトラブル対応などを考えると、とりあえずは別々のLambda関数にして安定させる方が良いのだろうか。 

 

[まとめ]

  • Lambdaは色々とハマりどころがあるのでAWSのドキュメントを確認すること
  • 困っているところに何か知見をお持ちの方がいればTwitterやブログで教えてもらえると嬉しい