We desire a consistent Go style in our code base. Even though the Google Go style guide and already provide many fundamental style decisions and best practices, there is still wiggle room for minor style inconsistency. It's more about keeping a consistent preference rather than right or wrong.

The list here won't repeat what's already in the Google Go style guide. Please reference that first. This list is also by no means thorough. If you find a style worth documenting, please reach out to one of the Go readability approvers to add your practices to this page.

The folder structure should mostly follow: https://github.com/golang-standards/project-layout. In our projects, we often need a couple of more things. Here is an example:

.
├── apis            // All public APIs
│   └── v1alpha1    // APIs should be versioned
├── sdk             // Sometimes we need sdk/client libs for different languages
│   ├── go          // DO NOT have a separate Go module
│   └── java
├── cmd
│   ├── my-cli
│   └── my-service
├── docs
├── pkg
├── protos          // Often we define public APIs in protos and generate them to the apis folder
│   └── v1alpha1    // APIs should be versioned; generated Go code should go to apis/v1alpha1
├── scripts
├── terraform       // For terraform modules
└── test            // For integration/e2e tests

We should use the following OSS dependencies for the corresponding tasks.

• https://github.com/sethvargo/go-envconfig for loading env config (prefer the wrapper https://github.com/abcxyz/pkg/tree/main/cfgloader)
• https://github.com/sethvargo/go-retry for generic retry logic
• https://github.com/lestrrat-go/jwx/v2 for all JWT operations (see usage example in https://github.com/abcxyz/jvs)

Prefer pointers for any structure types. Prefer concrete types for built-in structures unless you need to distinguish between an empty value and an unprovided value. Always use pointer receivers for functions.

• Godoc supports links (to symbols), lists, examples, etc. Find examples in https://pkg.go.dev/github.com/fluhus/godoc-tricks
  • Tip: Run godoc -http=:8080 and go to localhost:8080 to see visualized godoc
• Use // instead of /* */. If you use VS code, Cmd + / should comment out blocks in this style by default.
• Wrap comments at 80-character length. VS code extension like Rewrap can help you achieve that easily.

Flag variables should be prefixed with "flag" to differentiate from non-flag variables, e.g. var flagDebugMode bool.

• Standard imports first: Golang official imports, like "fmt"
• Custom imports, e.g. example.com/foo
• Aliased imports, e.g. foov2 example.com/foo/v2
• Blank imports, e.g. _ example.com/foo
• Dot imports, e.g. . example.com/foo

package main
import (
  "fmt"

  go "github.com/golang"

  "example.com/foo"

  _ "example.com/foo/blank"
  _ "github.com/golang/blank"

  . "example.com/foo/dot"
  . "github.com/golang/dot"
)

We want to use the native Go tooling go generate to do the job. In the Go package where you want to generate the code, add a package level comment similar to the following one:

//go:generate protoc -I../../../../third_party/googleapis -I../../../../protos/v1alpha1 --go_out=. --go-grpc_out=. --go_opt=paths=source_relative --go-grpc_opt=paths=source_relative audit_log_request.proto audit_log_agent.proto

// Package v1alpha1 contains versioned Lumberjack contracts, e.g. audit log
// entry type, config, etc.
package v1alpha1

See the context in: https://github.com/abcxyz/lumberjack/blob/main/clients/go/apis/v1alpha1/doc.go

You may need to tweak the paths in the protoc command.

Any binary that exposes a main function (CLIs, servers, etc) must follow the realMain pattern. The realMain pattern calls for a "thin" main() function that invokes realMain, with realMain accepting a context and returning an error type. The main() function should only do the most minimal setup required, such as establishing a context and logger:

package main

func main() {
  // Ensure the process responds to INT and TERM calls. By using a context,
  // downstream operations will terminate gracefully.
  ctx, done := signal.NotifyContext(context.Background(), syscall.SIGINT, syscall.SIGTERM)
  defer done()

  logger := logging.NewFromEnv("MY_SERVICE_")
  ctx = logging.WithLogger(ctx, logger)

  if err := realMain(ctx); err != nil {
    done()
    fmt.Fprintln(os.Stderr, err.Error())
    os.Exit(1)
  }
}

func realMain(ctx context.Context) error {
  db, err := database.Open("...")
  if err != nil {
    return fmt.Errorf("failed to open database: %s", err)
  }
  defer db.Close()

  cache, err := redis.Open("...")
  if err != nil {
    return fmt.Errorf("failed to open redis: %s", err)
  }
  defer cache.Close()

  // ...
}

This is important for error handling, context cancellation (interrupts), and for ensuring that any defer statements properly run before termination.

Consider the following bad example, which just uses main:

// BAD - DO NOT USE
func main() {
  db, err := database.Open("...")
  if err != nil {
    log.Fatalf("failed to open database: %s", err)
  }
  defer db.Close()

  cache, err := redis.Open("...")
  if err != nil {
    log.Fatalf("failed to open redis: %s", err)
  }
  defer cache.Close()

  // ...

  log.Fatal(http.ListenAndServe("..."))
}

The defer statements to close the database and redis connections will never run. The call to log.Fatal immediately terminates the process and exits.

Always use structured logging with github.com/abcxyz/pkg/logging, which uses an opinionated instance of the log/slog Go standard library (Unless you are in one of the Exceptional cases below).

Exceptions to using github.com/abcxyz/pkg/logging:

• Using the Apache Beam Go SDK for a pipeline to be run on Dataflow: Dataflow has custom logging configurations that cause logging messages written with loggers from github.com/abcxyz/pkg/logging to always show as severity INFO in the Dataflow log window. The the Beam Go SDK provides a logging package that should be used instead. When using the sdk logger, log messages will appear with the appropriate severity in the Dataflow log window. See the following for additional context/info:
  • https://github.com/abcxyz/github-metrics-aggregator/pull/171
  • https://cloud.google.com/dataflow/docs/guides/logging

Logs should:

• Prefer the Context versions over the non-context versions:

  // BAD - AVOID
  logger.Info("this is a message")

  // GOOD
  logger.InfoContext(ctx, "this is a message")

• Use all lowercase strings without punctuation for log messages:

  // BAD - AVOID
  logger.InfoContext(ctx, "This is a Message.")

  // GOOD
  logger.InfoContext(ctx, "this is a message")

• Use lowercase strings and underscores for log keys in key=value pairs:

  // BAD - AVOID
  logger.InfoContext(ctx, "something happened", "Key", value, "FishFood", flakes)

  // GOOD
  logger.InfoContext(ctx, "something happened", "key", value, "fish_food", flakes)

• Use With for key=value pairs that should appear on all log messages:

  // BAD - AVOID
  logger.InfoContext(ctx, "something happened", "Key", value)
  logger.InfoContext(ctx, "something else happened", "Key", value)
  logger.InfoContext(ctx, "a third thing happened", "Key", value)

  // GOOD
  logger = logger.With("key", "value")
  logger.InfoContext(ctx, "something happened")
  logger.InfoContext(ctx, "something else happened")
  logger.InfoContext(ctx, "a third thing happened")

• Use the "logger" key to namespace logs (note: this is convention):

  // GOOD
  logger.InfoContext(ctx, "something happened", "logger", "doSomething")

Bundle user facing functionalities into a single binary as the entrypoint for each project. Benefits are:

For maintainers:

• Only need to maintain the build / containerization process for a single binary
• Significantly reduce the build time
• By leveraging the existing CLI package, it's easy to bake in consistency. E.g. consistent helper documentation, shared flags, etc.
• Helper documentation is now in code and will be updated accordingly when the behavior changes. It helps to reduce the amount of additional documentation required outside of code.

For users:

• Only need to operate a single binary. No need to worry about version matching.
• Binary usage is baked into the binary helper documentation and easily discoverable.

Bonus:

• As the original entry points (main functions) now become sub-commands, it encourages better unit testing (vs. we usually don't test main.go). See examples in jvs/pull/245.

We use goreleaser for Go project releases. For detailed usage, please refer to its public documentation. Here are the special practices we have adopted:

• Decouple the docker image release and SCM (github) release. This allows us to reuse goreleaser to build and push docker images without really creating a SCM release. To achieve this, we would need two goreleaser config files in the repo root:
  • goreleaser.docker.yaml contains the docker release logic. Use flag -f .goreleaser.docker.yaml to let goreleaser use this config.
  • gorelease.yaml contains the SCM (github) release logic. This is the default config goreleaser will look at.
• Docker multi-arch build and upload manifest list. See sample .goreleaser.docker.yaml.
  • goreleaser.docker.yaml should allow overriding container registry and image tag for non-release image build/push.

Servers should always listen on a port string, not int. Ports can be service names (e.g. "https"), so the following is technically valid code:

net.Listen("tcp", "localhost:https")

Furthermore, Go's standard library returns ports as strings. Accepting a port as a string avoids unnecessary type conversions.

No third party server frameworks are approved for use. This includes, but is not limited to Gin (and gin-contrib), Beego, fasthttp, Buffalo.

We use the Go standard library (net/http), for HTTP servers and request handling.

All servers must be gracefully stopped on termination. Servers should terminate on SIGINT and SIGTERM. The easiest way to ensure signals are propagated is to use a signal context that is passed to subsequent calls:

ctx, done := signal.NotifyContext(context.Background(), syscall.SIGINT, syscall.SIGTERM)
defer done()

openDatabase(ctx)
openRedis(ctx)

Our serving package provides methods for codifying this behavior:

ctx, done := signal.NotifyContext(context.Background(), syscall.SIGINT, syscall.SIGTERM)
defer done()

server, err := serving.New(port)
if err != nil {
  // TODO: handle error
}

// This will block until the provided context is cancelled.
if err := server.StartHTTP(ctx, httpServer); err != nil {
 // TODO: handle error
}

Note, when responding to requests, use the inbound http/Request.Context() instead. This ensures that the operation is canceled if the user terminates the request:

func (s *Server) HandleFoo() http.Handler {
  return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
    ctx := r.Context()
  })
}

Use Middleware to inject values (such as a logger) into the request context.

Middleware is a well-established concept in grpc (called "interceptors"), so this section focuses on HTTP middleware. Simply put, HTTP middleware is an http.Handler that accepts another http.Handler as input. For example, here is a middleware that injects a logger:

func WithLogging(l *logging.Logger, next http.Handler) http.Handler {
  return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
    ctx := r.Context()
    ctx = logging.WithLogger(ctx, l)
    r = r.WithContext(ctx)
    next.ServeHTTP(w, r)
  })
}

Middleware can also be trailing:

func WithLatencyRecorder() http.Handler {
  return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
    now := time.Now()
    defer log.Printf("request took %s", time.Since(now))
    next.ServeHTTP(w, r)
  })
}

Use middleware by wrapping any handlers:

mux := http.NewServeMux()
mux.Handle("/foo", handleFoo)

stack := http.Handler(mux)
stack = WithLogging(logger, stack)
stack = WithLatencyRecorder(stack)

// ...

srv.ListenAndServe(stack)

Do not use global http functions. This includes http.ListenAndServe or http.HandleFunc. Always create a dedicated http.Server and a dedicated http.ServeMux:

mux := http.NewServeMux()
mux.Handle("/foo", handleFoo())
mux.HandleFunc("/bar", handleBar)

server := &http.Server{
  Addr: ":" + port,
  Handler: mux,

  // ...
}

All HTTP serve functions must be named handleXX or HandleXX and return an http.Handler:

// good:
func handleFoo() http.Handler {
  return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
    // ...
  })
}

// bad: bad name
func foo() http.Handler

// bad: does not return http.Handler
func handleFoo(w http.ResponseWriter, r *http.Request)

For more complex scenarios, create a dedicated server or controller package and expose a Routes function:

package controller

type Controller struct {
  // fields ...
}

func New(// ...) *Controller {
  return &Controller{
    // ...
  }
}

func (c *Controller) Routes() http.Handler {
  mux := http.NewServerMux()
  mux.Handle("/foo", c.HandleFoo())
  mux.Handle("/bar", c.HandleBar())
  return mux
}

func (c *Controller) HandleFoo() http.Handler
func (c *Controller) HandleBar() http.Handler

//
//
//

package main

func realMain(ctx context.Context) error {
  c := controller.New()

  server := &http.Server{
    Addr: ":" + port,
    Handler: c.Routes(),
  }
}

By architecting servers as defined above, you can test individual handlers:

func TestServerHandleFoo(t *testing.T) {
  t.Parallel()

  c := controller.New()

  r := httptest.NewRequest("GET", "/", nil)
  w := httptest.NewRecorder()

  c.HandleFoo()(w, r)

  // check w
}

HTTP and gRPC server should always implement health check API.

• gRPC health check API. See example.
• HTTP - return 200 on route /healthz. See example.

We prefer configuration to be aligned to the least common denominator across deployment environments. Since our servers and CLIs do not specifically target deployment platforms or runtimes, the least common denominator is the environment, specifically environment variables. This generally means declaring structs with env tags and parsing them using sethvargo/go-envconfig:

type MyConfig struct {
  DevMode bool   `env:"DEV_MODE, default=false"
  Port    string `env:"PORT, default=8080"
}

Since nearly every deployment target has the ability to set environment variables, this approach avoids dependencies on the filesystem or specific configuration formats.

As a general rule declare sane default values where possible. Our deliverables should work out of the box with as little configuration as possible. This is not always possible, so be pragmatic and supplement with documentation as needed.

Projects that require many environment variables could benefit from using a dotenv file (e.g. .env) for local developer productivity. This file is usually ignored from source control and defines an exported list of environment variables for the required configuration:

DEV_MODE="true"
PORT="8081"

There are multiple ways to use this file. The easiest is to "source" the file into your shell:

set -o allexport && source .env && set +o allexport

This will only affect your current shell session. If you open a new tab or close the terminal, you will need to source again. There are also editor plugins for IntelliJ and Visual Studio Code which will read this file automatically.

All packaged command line tools must use the github.com/abcxyz/pkg/cli package. Small scripts and short example code may use Go's standard library. No other CLI packages are permitted.

The CLI package is fairly opinionated, but we also adhere to the following conventions:

• Command structures must be named XXCommand.

• Command structures should be in the same package.

• Flag names must use dashes as separators (not underscores):

  // BAD: uses underscore
  f.StringVar(&cli.StringVar{
    Name: "first_name",
  })

  // GOOD
  f.StringVar(&cli.StringVar{
    Name: "first-name",
  })

Descriptions should start with an action verb and should not end in a period or any other punctuation:

// BAD: not an action verb
func (c *MyCommand) Desc() string {
  return "Cannot eat cookies"
}

// BAD: ends with punctuation
func (c *MyCommand) Desc() string {
  return "Eats some cookies. "
}

// GOOD
func (c *MyCommand) Desc() string {
  return "Eats some cookies"
}

The CLI will automatically truncate space and punctuation characters at the end of a description.

By convention, boolean flags should be false by default:

// BAD: default value is "true" and flag is positive
f.BoolVar(&cli.BoolVar{
  Name:    "enable-cookies",
  Default: true,
})

// GOOD
f.BoolVar(&cli.BoolVar{
  Name:    "disable-cookies",
  Default: false,
})

Try naming your flags to match this convention, because it reduces the cognitive overhead for users.

Some commands may share a common set of flags. For example, you may have a set of flags for specifying retry values that should be shared among some commands. To share flags, create a dedicated flags package and a structure in that package to house the flags:

// pkg/command/flags/retry.go
package flags

type RetryFlags struct {
  flagRetryAttempts int
  flagRetryTimeout  time.Duration
}

func (r *RetryFlags) Register(set *cli.FlagSet) {
  f := set.NewSection("RETRY OPTIONS")

  f.IntVar(&cli.IntVar{
    // ...
  })

  f.DurationVar(&cli.DurationVar{
    // ...
  })

  // Encapsulate any validation inside the set:
  set.AfterFunc(func(existingErr error) error {
    var merr error
    if r.flagRetryAttempts <= 0 {
      merr = errors.Join(merr, fmt.Errorf("-retry-attempts must be positive"))
    }
    return merr
  })
}

See the persistent flags example in the CLI repo for a complete example.

By convention, section names should be all capital letters:

// BAD: uses lowercase letters
f.NewSection("My section")

// GOOD
f.NewSection("MY SECTION")

When providing an example value, ensure that the example value is different from the default value.

// BAD: default and example are the same
f.DurationVar(&cli.DurationVar{
  Default: 5*time.Minute,
  Example: "5m",
})

// GOOD
f.DurationVar(&cli.DurationVar{
  Default: 5*time.Minute,
  Example: "30s",
})

The CLI package exposes an AfterParse function, which can be useful for things like validation and setting deferred defaults.

func (c *MyCommand) Flags() *cli.FlagSet {
  set := c.NewFlagSet()

  f := set.NewSection("SERVER OPTIONS")

  f.StringVar(&cli.StringVar{
    Name:    "server-address",
    Example: "https://my.corp.server:8145",
    Default: "http://localhost:8145",
    EnvVar:  "CLI_SERVER_ADDRESS",
    Target:  &c.flagAddress,
    Usage:   "Endpoint, including protocol and port, the server.",
  })

  // Deprecated - use -server-address instead.
  f.StringVar(&cli.StringVar{
    Name:    "address",
    Default: "http://localhost:8145",
    Target:  &c.flagOldAddress,
    Hidden:  true,
  })

  // Each AfterParse will be invoked after flags have been parsed.
  set.AfterParse(func(existingErr error) error {
    // Example of deferred defaulting. At this point, it is safe to set values
    // of flags to other values.
    if c.flagOldAddress != "" {
      c.Errf("WARNING: -address is deprecated, use -server-address instead")
    }
    if c.flagAddress == "" {
      c.flagAddress = c.flagOldAddress
    }

    return nil
  })

  return set
}

See the AfterParse example in the CLI repo for more details.

Name the test in the format of Test[<Struct>_]<Function>[_<details>].

• Test helper functions should be prefixed with test to differentiate from non-test functions, e.g. testToken(testing.TB)
• Pass testing.TB whenever possible instead of *testing.T

Table-driven tests make it easier to write exhaustive test cases, especially for unit tests.

• Each test case should have a unique test name.
• Each test case should be an inline struct with a name field (do not use a map).
• Test case names must be only lowercase letters and underscores (this makes targeted testing with -run easier).

func TestPerson_GetName(t *testing.T) {
  t.Parallel()

  cases := []struct {
    name  string
    input string
  }{
    {
      name:  "with_an_apple",
      input: "...",
    },

    // github.com/abcxyz/pkg#5
    {
        name:  "with_a_banana",
        input: "...",
    },
  }

  for _, tc := range cases {
    t.Run(tc.name, func(t *testing.T) {
      t.Parallel()

      // ...test...
    })
  }
}

As much as possible tests should be capable of running in parallel with all other tests. This includes sub-tests and table driven tests. Tests are marked as parallelizable with t.Parallel(). If a test cannot be parallelized, that is likely a bad encapsulation. Dependencies on the environment should be abstracted into an interface to maximize parallelization.

func TestPerson_GetName(t *testing.T) {
  t.Parallel()

  cases := []struct {
    name  string
  }{
    // ...
  }

  for _, tc := range cases {
    t.Run(tc.name, func(t *testing.T) {
      t.Parallel()å

      // ...test...
    })
  }
}

If a test cannot be parallelized, it must be documented inline as to why it cannot be parallelized, and the conditions under which it could be parallelized in the future.

// This test cannot be parallelized because the pizza service is not threadsafe.
func TestSendPizza(t *testing.T) {
  // ...test...
}

The default go test ./... command should run all test files. Integration and functional tests that have dependencies (i.e. credentials or configuration) should call t.Skip() with an informative message. On CI, tests with missing dependencies should fail.

If a test creates a resource, it must also delete that resource. This includes environment variables, files, and external Cloud resources. The easiest way to ensure a resources are cleaned is to use t.Cleanup():

func TestFile_Read(t *testing.T) {
  t.Parallel()

  f, err := os.CreateTemp("", "")
  if err != nil {
    t.Fatal(err)
  }
  t.Cleanup(func() {
    if err := os.RemoveAll(f.Name()); err != nil {
      t.Fatal(err)
    }
  })
}

func TestObject_Get(t *testing.T) {
  t.Parallel()

  obj, err := sdk.CreateObject(&sdk.CreateObjectRequest{
    name: "my-obj",
  })
  if err != nil {
    t.Fatal(err)
  }
  t.Cleanup(func() {
    if err := sdk.DeleteObject(&sdk.DeleteObjectRequest{
      name: obj.Name,
    }); err != nil {
      t.Fatal(err)
    }
  })
}

Minimize dependencies on external systems where possible. Tests should create and destroy the resources they need for execution (see also: "#clean-up").

In some cases, it may be necessary to have a prerequisite setup for tests, depending on the boundary for the tool. For tests that depend on GCP, project creation and service enablement should likely be handled outside the band of individual tests. This should be codified as Infrastructure as Code (IaC) using Terraform. The Terraform should be run once to configure a CI environment, not once per CI run.

More expensive operations that should be isolated for each test run should configure using the TestMain pattern:

var testDatabase *database.Database

func TestMain(m *testing.M) {
  testDatabase = expensiveOperationThatConfiguresADatabase()

  defer func() {
    testDatabase.Destroy()
  }()

  os.Exit(m.Run())
}

The output of one test cannot influence the output of another test. On CI, the go test command must, at minimum, be run with:

go test -shuffle=on -count=1 -race -timeout=10m ./...

• -count=1 - to ensure no cached tests.
• -race - to enable the race detector.
• -shuffle=on - to enable randomized test ordering.
• -timeout=XXm - to prevent run-away tests.

If you're developing a HTTP or gRPC service, it's better to unit test them with local but real connection. In the same spirit, if your code under test directly depends on a HTTP or gRPC service, it's better to implement a fake and connect to it with local but real connection instead of entirely stubbing it out. Such way to test is closer to reality and provides better coverage.

• HTTP - Use https://pkg.go.dev/net/http/httptest. If you need a fake, write a fake server rather than client.
• gRPC - We have a testutil to start a gRPC server locally on a random port. If you need a fake, write a fake server and remember to embed the generated UnimplementedXxxServer. See an example here.

Prefer using fmt.Errorf to create errors even if no formatting is required. This helps to reduce the cognitive load to remember other ways to create errors (e.g. errors.New).

fmt.Errorf("err no format")
// Instead of errors.New("err no format")

For any errors that returned from a different package, we should always wrap the error:

if err := externalPkg.Do(); err != nil {
  return fmt.Errorf("external pkg do failed: %w", err)
}

For abcxyz Go projects, we use pkg/.golangci.yml for lint configuration. To run the linter locally:

# Install golangci-lint if you haven't done so
# go install github.com/golangci/golangci-lint/cmd/golangci-lint@latest

golangci-lint run -c <(curl https://raw.githubusercontent.com/abcxyz/pkg/main/.golangci.yml)