Go Style Guide in Vald

Introduction

This guideline includes the coding style for all Vald contributors and reviewers. Everyone should follow this guideline to keep the style consistent so everyone can understand and contribute to Vald easier once they learn this guideline. You should have the basic knowledge of how to write Go before contributing to Vald. If you found any bug please create a GitHub issue and we will work on it.

Please also read the Contribution guideline before you start contributing to Vald.

Code Formatting and Naming Convension

Code formatting and naming conventions affect coding readability and maintainability. Every developer has a different coding style, luckily Go provides tools to format source code and checking for the potential issue in the source code. We recommend using goimports to format the source code in Vald, and golangci-lint with --enable-all option. We suggest everyone install the plugin for your editor to format the code once you edit the code automatically, and we suggest using make update/goimports command if you want to format the source code manually.

But having tools to format source code doesn’t mean you do not need to care the formatting of the code, for example:

BadGood
badStr := "apiVersion: v1\n" +
   "kind: Service\n" +
   "metadata:\n" +
   "  name: grafana\n" +
   "spec:\n" +
   "  ports:\n" +
   "    - name: grafana\n" +
   "      port: 3000\n" +
   "      targetPort: 3000\n" +
   "      protocol: TCP\n"
goodStr := `apiVersion: v1
kind: Service
metadata:
  name: grafana
spec:
  ports:
    - name: grafana
      port: 3000
      targetPort: 3000
      protocol: TCP
`

Project Layout

The project layout includes the folder and the file structure in the project. We follow the project-layout example in Vald.

Packages

The package defines the context of the objects in the package, for example, the corresponding methods and structs belong to the corresponding package. Unlike other languages like Java, in Go we use the package name to declare which context of the object we are going to use. For example in time package, it defines all the objects about time like time.Now() method to get the current time.

Here is the naming conventions of the package:

  • All lower case.
// bad
package ioUtil

// good
package ioutil
  • No plurals.
// bad
package times

// good
package time

  • Should be the same as the folder name.
  • Should keep as simple as it should, and should contain only one specific context in the package.
// bad
package encodebase64

// good
package base64 // inside the encoding/base64 folder
  • Should not be too general, for example util or helper, which will cause all the objects from different contexts to be store in one package. If you want to name the package as util, please define the more specific package name more ioutil or httputil.

All packages should contain doc.go file under the package to describe what is the package is. For example, under the folder name called cache, should contains a file named doc.go, which contains the package documentation. Here is the example doc.go of the cache package.

// Package cache provides implementation of cache
package cache

General style

This section describes the general guideline for the Vald programming style, every Vald contributor should keep these general guidelines in mind while working on the implementation of Vald.

Order of declaration

Put the higher priority or frequently used declaration on the top of other declaration. It makes Vald easier to read and search the target source code in Vald.

For example, the interface declaration should have higher priority than struct or function declaration, hence it should be put above other declaration.

BadGood
type S struct {}

func (s *S) fn() {}

type I interface {}
type I interface {}

type S struct {}

func (s *S) fn() {}

Group similar definition

Group similar definitions such as struct or interface declaration. We should not group interface and struct declaration in the same block, for example:

BadGood
type (
    I interface {}
    I2 interface {}

    s struct {}
    s2 struct {}
)
type (
    I interface {}
    I2 interface {}
)

type (
    s struct {}
    s2 struct {}
)

Interfaces

The interface defines the program interface for usability and future extendability. Unlike other languages like Java, Go supports implicit interface implementation. The type implements do not need to specify the interface name; to “implements” the interface the structs only need to defined the methods the same as the interface, so please be careful to define the method name inside the interface.

The interface should be named as:

  • Use MixedCaps
// bad
type Roundtripper interface {
  // interface definition
}

// good
type RoundTripper interface {
  // interface definition
}
  • Use understandable common short form.
// bad
type ATSigner interface {
  // interface definition
}

// good
type AccessTokenSigner interface {
  // interface definition
}

// good
type HTTPServer interface {
  // interface definition
}

Structs

Structs in Go is the object definition, we can attach any fields and methods to the struct. The naming convention is the same as the interface one. If the structs are implementing the interface, the structs name should be related to the interface, for example:

type Listener interface {
   // interface definition
}

// Listener instance for file
type FileListener struct {
   // implement listener interface
}

// Listener instance for HTTP
type HTTPListener struct {
   // implement listener interface
}

Struct initialization

There are many ways to initialize structs in Go, base on the use case we can decide which way to initialize structs in Go. To initialize struct, it is suggested to use new(T) instead of &T{} unless you need to initialize with values. For example:

type Something struct {
    name string
}

// good
a := new(Something)

// bad
b := new(Something)
b.name = "Mary"

// use this syntax instead of b
c := &Something{
    name: "Mary",
}

To initialize complex structs, we can use functional option pattern. Please read server.go and option.go for the reference implementation. The options implementation should be separated as another file called option.go to improve the readability of the source code, and the method name should start with With word to differentiate with other methods.

Variables and Constant

The variable and the constant should be named as:

  • Use MixedCaps
// bad
yamlprocessor := new(something)

// good
yamlProcessor := new(something)
  • Use short form.
// bad
yamlString := "something"

// good
yamlStr := "something"

// in some case it is acceptable and actually if it is easier to read
s := new(something)

The variable and the constant name may lead to misunderstanding or confusion, so if the variable and constant name are different to understand, please write some comment even if it is a private member.

// somebody may not understand this variable, so write a simple comment to the variable definition
sac := new(something) // signed access token

If the multiple variables and the constants have the same grouping, please use the grouping name as the prefix of the variable and constant name.

// Same group of variable (error), so add a prefix `Err` to each error variables
ErrInvalidCacherType = New("invalid cacher type")
// ErrXXXXXXX

Methods

In this section, rules also apply to the function (without receiver). The method name should be named as:

  • Use MixedCaps.
// bad
func (s *something) somemethod() {}

// bad
func (s *something) some_method() {}

// good
func (s *something) someMethod() {}
  • Avoid using long function name.
  • Do not use short form unless the function name is too long.
// bad
func (s *something) genereateRandomNumber() int {}

// good
func (s *something) genRandNum() int {}
  • It should be understandable for everyone even if it is a private method.

Getter and Setter

The Getter and Setter are almost the same as other languages, but the naming convention of the Getter method is different from other languages. Instead of GetVar1(), the getter of Var1 should be the same as the variable name itself Var1().

// getter of the `signedTok`
func (s *something) SignedTok() string{
    return s.signedTok
}

// setter of the `signedTok`
func (s *something) SetSignedTok(st string) {
    s.signedTok = st
}

Unused Variables

An unused variable may increase the complexity of the source code, it may confuse the developer hence introduce a new bug. So please delete the unused variable.

Generally, the unused variable should be reported during compilation, but in some cases, the compiler may not report an error. This is an example of the unused variable declaration that does not cause a compilation error.

// In this case, this example are not using `port` field, but dose not cause a compilation error.
// So please delete `port` field of `server`.

type server struct {
    addr string
    port int  // you have to delete this field.
}

// The `port` field of `server` is not used.
srv := &server {
    addr: "192.168.33.10:1234",
}

if err := srv.Run(); err != nil {
    log.Fatal(err)
}

Error handling

All errors should define in internal/errors package. All errors should be start with Err prefix, and all errors should be handle if possible.

Please use internal/errgroup for synchronized error handling on multi-goroutine processing.

Error checking

All functions return error if the function can fail. It is very important to ensure the error checking is performed. To reduce human mistake that missing the error checking, please check the error using the following style:

BadGood
err := fn()
if err != nil {
    // handle error
}
if err := fn(); err != nil {
    // handle error
}

If you need the value outside the if statement, please use the following style:

BadGood
if conn, err := net.Dial("tcp", "localhost:80");  err != nil {
    // handle error
} else {
    // use the conn
}
conn, err := net.Dial("tcp", "localhost:80")
if err != nil {
    // handle error
}
// use the conn

Logging

We define our own logging interface in internal/log package. By default we use glg to do the logging internally. We defined the following logging levels.

Log levelDescriptionExample situationExample message
DEBUGFine-grained information for debugging and diagnostic the application.
Enable this logging level in production environment may cause performance issue.
An entry that will insert into the database with details.
An HTTP request and response that will send and receive from the server with details.
User 1 will insert into the database, name: Mary, Age: 19.
An HTTP request will send to http://example.com, with the body:
key1:value1, key2:value2.
The response of the HTTP request: body: HTTPResponseBody
INFONormal application behavior, to trace what is happening inside the application.Inserted entry into the database without details.
HTTP requests sent to the server without details.
The server is started or stopped.
User 1 is inserted into the database.
The HTTP request is sent to XXX server successfully.
WARNThe message that indicates the application may having the issue or occurring unusual situation,
but does not affect the application behavior.
Someone should investigate the warning later.
Failed to insert entry into the the database, but success with the retry.
Failed to update the cache, and the cache is not important.
User 1 is successfully inserted into the database with retry,
retry count: 1, error: ErrMsg1, retry count: 2, error: ErrMsg2
ERRORThe message that indicates the application is having a serious issue or,
represent the failure of some important going on in the application.
It does not cause the application to go down.
Someone must investigate the error later.
Failed to insert an entry into the database, with retry count exceeded.
Failed to update the cache, and the cache is not important.
User 1 is failed to insert in the database, errors:
retry count: 1, error: ErrMsg1, retry count: 2, error: ErrMsg2, ….
FATALMessage that indicate the application is corrupting or having serious issue.
The application will go down after logging the fatal error.
Someone must investigate and resolve the fatal as soon as possible.
Failed to init the required cache during the application start.

Implementation

This section includes some examples of general implementation which is widely used in Vald. The implementation may differ based on your use case.

Functional Option

In Vald, the functional option pattern is widely used in Vald. You can refer to this section for more details of the use case of this pattern.

We provide the following errors to describe the error to apply the option.

ErrorDescription
errors.ErrInvalidOptionError to apply the option, and the error is ignorable
errors.ErrCriticalOptionCritical error to apply the option, the error cannot be ignored and should be handled

We strongly recommend the following implementation to set the value using functional option.

If an invalid value is set to the functional option, the ErrInvalidOption error defined in the internal/errors/option.go should be returned.

The name argument (the first argument) of the ErrInvalidOption error should be the same as the functional option name without the With prefix.

For example, the functional option name WithVersion should return the error with the argument name as version.

func WithVersion(version string) Option {
    return func(c *client) error {
        if len(version) == 0 {
            return errors.NewErrInvalidOption("version", version)
        }
        c.version = version
        return nil
    }
}

We recommend the following implementation to parse the time string and set the time to the target struct.

func WithTimeout(dur string) Option {
    func(c *client) error {
        if dur == "" {
            return errors.NewErrInvalidOption("timeout", dur)
        }
        d, err := timeutil.Parse(dur)
        if err != nil {
            return errors.NewErrInvalidOption("timeout", dur, err)
        }
        c.timeout = d
        return nil
    }
}

We recommend the following implementation to append the value to the slice if the value is not nil.

func WithHosts(hosts ...string) Option {
    return func(c *client) error {
        if len(hosts) == 0 {
            return errors.NewErrInvalidOption("hosts", hosts)
        }
        if c.hosts == nil {
            c.hosts = hosts
        } else {
            c.hosts = append(c.hosts, hosts...)
        }
        return nil
    }
}

If the functional option error is a critical error, we should return ErrCriticalOption error instead of ErrInvalidOption.

func WithConnectTimeout(dur string) Option {
    return func(c *client) error {
        if dur == "" {
            return errors.NewErrInvalidOption("connectTimeout", dur)
        }
        d, err := timeutil.Parse(dur)
        if err != nil {
            return errors.NewErrCriticalOption("connectTimeout", dur, err)
        }

        c.connectTimeout = d
        return nil
    }
}

In the caller side, we need to handle the error returned from the functional option.

If the option failed to apply, an error wrapped with ErrOptionFailed defined in the internal/errors/errors.go should be returned.

We recommend the following implementation to apply the options.

func New(opts ...Option) (Server, error) {
    srv := new(server)
    for _, opt := range opts {
        if err := opt(srv); err != nil {
            werr := errors.ErrOptionFailed(err, reflect.ValueOf(opt))

            e := new(errors.ErrCriticalOption)
			if errors.As(err, &e) {
                log.Error(werr)
                return nil, werr
            }
            log.Warn(werr)
        }
    }
}

Program comments

Program comments make easier to understand the source code. We suggest not to write many comments inside the source code unless the source code is very complicated and confusing; otherwise we should divide the source code into methods to keep the readability and usability of the source code.

Everyone should write the comments to all the public objects on your source code, like public packages, interface, structs, methods, and even public constant and variable. The godoc will be generated base on the comment of source code.

Documentation

Documentation is generated from the program comments. Please refer to Godoc for the program documentation.

Internal packages

Vald implements its internal package to extend and customize the functionality of the standard library and third-party library. We should use the internal package instead of standard libray to implement Vald. Please refer to godoc for the internal package document.

Dependency management and Build

We should use go mod tidy to manage the go.mod file in the project.

Test

The testing guideline has 3 important rules for the coding quality and readability:

  1. Use Table-Driven-Test
  2. Keep code coverage over 85%
    • test coverage != high testing quality, but low coverage means bad testing quality
    • check with the following commands go test -cover ./...
  3. Test all use cases and error cases

Table-Driven-Test

Use table-driven tests with subtests to avoid duplicating code.

BadGood
## case 1
host, port, err := net.SplitHostPort("192.0.2.0:8000")
if err != nil {
    t.Errorf("error is not nil: %v", err)
}

if want, got := "192.0.2.0", host; want != got {
    t.Errorf("host is not equals. want: %s, but got: %s", want: %s, got)
}

if want, got := "8000", port; want != got {
    t.Errorf("port is not equals. want: %s, but got: %s", want: %s, got)
}

## case2
host, port, err = net.SplitHostPort("192.0.2.0:http")
if err != nil {
    t.Errorf("error is not nil: %v", err)
}

if want, got := "192.0.2.0", host; want != got {
    t.Errorf("host is not equals. want: %s, but got: %s", want: %s, got)
}

if want, got := "http", port; want != got {
    t.Errorf("port is not equals. want: %s, but got: %s", want: %s, got)
}
tests := []struct {
    str string
    wantHost string
    wantPort string
} {
    ## case 1
    {
        str: "192.0.2.0:8000",
        wantHost: "192.0.2.0",
        wantPort: "8000",
    },
    ## case 2
    {
        str: "192.0.2.0:8000",
        wantHost: "192.0.2.0",
        wantPort: "http",
    },
}

for _, tt := range tests {
    t.Run(tt.str, func(tt *testing.T) {
        host, port, err := net.SplitHostPort(tt.str)
        if err != nil {
            t.Errorf("error is not nil: %v", err)
        }
        if want, got := tt.wantHost, host; want != got {
            t.Errorf("host is not equals. want: %s, but got: %s", want: %s, got)
        }
        if want, got := tt.wantPort, port; want != got {
            t.Errorf("port is not equals. want: %s, but got: %s", want: %s, got)
        }
    })
}

Table-Driven-Test makes it easy to add a new test case.

We define the test case table as map[string]func(*testing.T)test, which is referred to the test case name and the test case implementation tt.

tests := map[string]func(t *testing.T) test {
    "test case name": func(tt *testing.T) test {
        return test {
            args: args {
                host: "host",
                port: "port",
            },
            field: field {
                timeout: 1 * time.Second,
            },
        }
    }
}

The steps to create a Table-Driven-Test

  1. args structure

    If there are two or more arguments to be passed to the method, create a args structure. If there is only one argument, do not create an args structure.

    type args struct {
        host string
        port string
    }
    
  2. field structure

    If you create an object and test its methods, create a field struct if the object has two or more fields to initialize. If there is only one field, do not create field structure.

    type field struct {
        host string
        port string
    }
    
  3. test structure

    test structure has args and field structure and checkFunc function. If you need one of args and field structure, create field and args structure. The checkFunc function is used to check the return value of the function being tested.

    type test struct {
        args args
        field field
        checkFunc func(t *testing.T, err error)
    }
    

Example:

type args struct {
    addr string
    txt string
}

type field struct {
    timeout time.Duration
}

type test struct {
    args args
    field field
    checkFunc func(t *testing.T, err error)
}

tests := map[string]func(*testing.T) test {
    "test name": func(tt *testing.T) test {
        tt.Helper()

        return test {
            args: args {
                host: "host",
                port: "port",
            },
            field: field {
                host: "host",
                port: "port",
            },
            checkFunc func(tt *testing.T, err error) {
                t.Helper()
                if err != nil {
                    tt.Errorf("error is not nil: %v", err)
                }
            },
        }
    }
}

for name, fn := range tests {
    t.Run(name, func(tt *tesint.T) {
        test := fn(tt)

        c := client {
            timeout: test.field.timeout,
        }

        err := c.Send(test.args.addr, test.args.txt)
        test.checkFunc(tt, err)
    })
}

Generate test code

We implement our own gotests template to generate test code. If you want to install gotest tools, please execute the following command under the project root directory.

make gotests/install

If you use the following command to generate the missing test code.

make make gotests/gen

After the command above executed, the file *target*_test.go will be generated for each Go source file. The test code generated follows the table-driven test format. You can implement your test code under the tests variable generated following the table-driven test format.

Customize test case

We do not suggest to modify the generated code other than the tests variable, but in some cases, you may need to modify the generated code to meet your requirement, for example:

  1. init() function

    init() function is executed automatically before the test is started. You may need to initialize some singleton before your test cases are executed. For example, Vald uses glg library for logging by default, if the logger is not initialized before the test, the nil pointer error may be thrown during the test is running. You may need to implement init() function like:

    func init() {
        log.Init()
    }
    

    And place it on the header of the test file.

  2. goleak option

    By default, the generated test code will use goleak library to test if there is any Goroutine leak. Sometimes you may want to skip the detection, for example, Vald uses fastime library but the internal Goroutine is not closed due to the needs of the library. To skip the goleak detection we need to create the following variable to store the ignore function.

    var (
        // Goroutine leak is detected by `fastime`, but it should be ignored in the test because it is an external package.
        goleakIgnoreOptions = []goleak.Option{
            goleak.IgnoreTopFunction("github.com/kpango/fastime.(*Fastime).StartTimerD.func1"),
        }
    )
    

    And modify the generated test code.

    // before
    for _, test := range tests {
        t.Run(test.name, func(tt *testing.T) {
            defer goleak.VerifyNone(tt)
    
    // after
    for _, test := range tests {
        t.Run(test.name, func(tt *testing.T) {
            // modify the following line
            defer goleak.VerifyNone(tt, goleakIgnoreOptions...)
    
  3. Defer function

    By default the template provides beforeFunc() and afterFunc() to initialize and finalize the test case, but in some case, it may not support your use case. For example recover() function only works in defer() function, if you need to use recover() function to handle the panic in your test code, you may need to implement your custom defer() function and change the generated test code.

    For example:

    for _, test := range tests {
      t.Run(test.name, func(tt *testing.T) {
        defer goleak.VerifyNone(tt, goleakIgnoreOptions...)
    
        // insert your defer function here
        defer func(w want, tt *testing.T) {
            // implement your defer func logic
            if err:= recover(); err != nil {
                // check the panic
            }
        }(test.want, tt)
    
        if test.beforeFunc != nil {
            test.beforeFunc(test.args)
        }
        // generated test code
    
  4. Unused fields

    By default, the template provides fields structure to initialize object of the test target. But in some cases, not all fields are needed, so please delete the unnecessary fields. For example, the following struct and the corresponding function:

    type server struct {
        addr string
        port int
    }
    func (s *server) Addr() string {
        return s.addr
    }
    

    And the generated test code is:

    func Test_server_Addr(t *testing.T) {
        type fields struct {
            addr string
            port int
        }
        type want struct {
            // generated test code
    

    Since the port variable is not used in this test case, you can delete the port definition in the test case.

    func Test_server_Addr(t *testing.T) {
        type fields struct {
            addr string
            // port int   <-- this line should be deleted
        }
        type want struct {
            // generated test code