Skip to content

Latest commit

 

History

History
66 lines (52 loc) · 5.53 KB

best_practices.md

File metadata and controls

66 lines (52 loc) · 5.53 KB
Raw

Best practices

  • Read more

  • All gRPC errors are returned as status messages (1, 2). They contain a code, a developer-facing message, and, in some cases, details about that error. For example, for InvalidArgument code details contain invalid argument name and a type of failed validation. grpc-gateway converts gRPC errors to HTTP response with some status code and JSON object in the body. The client should use code inside the body, not an HTTP status code.

    • Example:

  • All incoming gRPC messages should be validated.

  • Every long-running process (daemon) should have a special HTTP server (typically bound to 127.0.0.1) with the following debug endpoints:

    • /debug/metrics – Prometheus metrics with promhttp.Handler(). go-grpc-prometheus with EnableHandlingTimeHistogram should be used to expose gRPC stats.
    • /debug/vars – default expvar values: command line and memory stats.
    • /debug/requests and /debug/events – golang.org/x/net/trace facility.
    • /debug/pprof – profiling endpoint from net/http/pprof.

  • gRPC requests should be logged with grpclog.SetLoggerV2.

  • All our software should handle termination signals: SIGTERM, SIGINT. The first time signal is received, the program should start graceful shutdown, typically by canceling parent context. It also should stop handling those termination signals, so the second time signal is received default handler is invoked, and the program is terminated.

  • During program startup, most errors should be treated as fatal. If the program can't connect to an external resource, or can't bind to a port, or something like that, it should log an error and exit with non-zero status code. Process supervisor will restart it automatically.

    After startup, during normal program execution, most errors should be handled, logged and communicated to the user. For example, REST API call should return an appropriate HTTP status code with error details; full details should be logged. The caller can then handle the problem and retry the call. If an external resource becomes unavailable, the program should try to reconnect to it with proper backoff policy. The supervisor should still be used to handle crashes, panics, and bugs.

    • Issues detected at program initializtion phase - such as a missing external dependency or a port, which is already in use - is either a configuration or an environment problem, or a programming bug. Neither can be fixed by simply continuing. Attempts to make startup "smart" (re-read configuration file, re-parse command-line flags, etc.) significantly complicate it. On the other hand, when an error happens during normal program execution, there is usually a proper way to communicate it back to the user so they can retry.
    • Supervisor-level dependencies between services are not strictly required (due to restarts everything will work eventually), but are nice to have: they help avoid scarry errors in logs.
    • The big exception is exporters: we should follow practices established by the Prometheus community and not fail if the system under monitoring is not available. Other startup errors, like a missing certificate file, should still terminate the exporter.

Code style

  • gofumpt -s (note the -s)

  • make format

  • Follow Effective Go and CodeReviewComments wiki page.

  • golangci-lint checks style too.

  • Try to keep the code consistent

  • Our own customizations:

    • Group imports like this (imports in each group are sorted alphabetically):

      import (
    "stdlib/packages"

    "github.com/percona/something"
    "github.com/third-party/something"
    "golang.org/x/something"

    "imports/from/this/repo"
)

  • In addition, more specific variable naming conventions we try to follow are provided in this blogpost https://medium.com/@lynzt/variable-naming-conventions-in-go-89fe1ef17b0a:

    • Use camelCase
    • Acronyms should be all capitals, as in ServeHTTP
    • Single letter represents index: i, j, k
    • Short but descriptive names: cust not customer
    • Repeat letters to represent collection, slice, or array and use a single letter in loop (var tt []*Thing).
    • Avoid repeating the package name in the method name:
      • log.Info() // good
      • log.LogInfo() // bad
    • Don’t name methods like getters or setters
    • Follow the language rules when using compound nouns: ApplicationServer not ApplicationsServer. Exceptions to this rule may apply when the code is auto-generated.
    • Add er to Interface* (Exception: when we use Interface as a template to generate mocks)

  • If we need to make a set, it's better to use map[string]struct{} instead of map[string]bool or something else.

  • In case we need enums, it's better to create a new custom type inherited from string or iota.

  • To check if a string is empty or not, use str != "" instead of len(str) != 0