1# Contributing to the LaunchDarkly Server-side SDK for Go
2
3LaunchDarkly has published an [SDK contributor's guide](https://docs.launchdarkly.com/sdk/concepts/contributors-guide) that provides a detailed explanation of how our SDKs work. See below for additional information on how to contribute to this SDK.
4
5## Submitting bug reports and feature requests
6
7The LaunchDarkly SDK team monitors the [issue tracker](https://github.com/launchdarkly/go-server-sdk/issues) in the SDK repository. Bug reports and feature requests specific to this SDK should be filed in this issue tracker. The SDK team will respond to all newly filed issues within two business days.
8
9## Submitting pull requests
10
11We encourage pull requests and other contributions from the community. Before submitting pull requests, ensure that all temporary or unintended code is removed. Don't worry about adding reviewers to the pull request; the LaunchDarkly SDK team will add themselves. The SDK team will acknowledge all pull requests within two business days.
12
13## Build instructions
14
15### Prerequisites
16
17This project should be built against the lowest supported Go version as described in [README.md](./README.md).
18
19### Building
20
21To build the SDK without running any tests:
22```
23make
24```
25
26If you wish to clean your working directory between builds, you can clean it by running:
27```
28make clean
29```
30
31To run the linter:
32```
33make lint
34```
35
36### Testing
37
38To build the SDK and run all unit tests:
39```
40make test
41```
42
43## Coding best practices
44
45The Go SDK can be used in high-traffic application/service code where performance is critical. There are a number of coding principles to keep in mind for maximizing performance. The benchmarks that are run in CI are helpful in measuring the impact of code changes in this regard.
46
47### Avoid heap allocations
48
49Go's memory model uses a mix of stack and heap allocations, with the compiler transparently choosing the most appropriate strategy based on various type and scope rules. It is always preferable, when possible, to keep ephemeral values on the stack rather than on the heap to avoid creating extra work for the garbage collector.
50
51- The most obvious rule is that anything explicitly allocated by reference (`x := &SomeType{}`), or returned by reference (`return &x`), will be allocated on the heap. Avoid this unless the object has mutable state that must be shared.
52- Casting a value type to an interface causes it to be allocated on the heap, since an interface is really a combination of a type identifier and a hidden pointer.
53- A closure that references any variables outside of its scope (including the method receiver, if it is inside a method) causes an object to be allocated on the heap containing the values or addresses of those variables.
54- Treating a method as an anonymous function (`myFunc := someReceiver.SomeMethod`) is equivalent to a closure.
55
56Allocations are counted in the benchmark output: "5 allocs/op" means that a total of 5 heap objects were allocated during each run of the benchmark. This does not mean that the objects were retained, only that they were allocated at some point.
57
58For any methods that should be guaranteed _not_ to do any heap allocations, the corresponding benchmarks should have names ending in `NoAlloc`. The `make benchmarks` target will automatically fail if allocations are detected in any benchmarks that have this name suffix.
59
60For a much (MUCH) more detailed breakdown of this behavior, you may use the option `GODEBUG=allocfreetrace=1` while running a unit test or benchmark. This provides the type and code location of literally every heap allocation during the run. The output is extremely verbose, so it is recommended that you:
61
621. use the Makefile helper `benchmark-allocs` (see below) to reduce the number of benchmark runs and avoid capturing allocations from the Go tools themselves;
632. search the stacktrace output to find the method you are actually testing (such as `BoolVariation`) rather than the benchmark function name, so you are not looking at actions that are just part of the benchmark setup;
643. consider writing a smaller temporary benchmark specifically for this purpose, since most of the existing benchmarks will iterate over a series of parameters.
65
66```bash
67BENCHMARK=BenchmarkMySampleOperation make benchmark-allocs
68```
69
70### Avoid `defer` in simple cases
71
72It's common to use `defer` to guarantee cleanup of some kind of temporary state when a method exits, such as releasing a lock. As convenient as this feature is, it should be avoided in high-traffic code paths _if it is safe to do so_, due to its small but consistent [runtime overhead](https://medium.com/i0exception/runtime-overhead-of-using-defer-in-go-7140d5c40e32).
73
74It is safe to avoid `defer` if:
75
761. there is only one possible return point from the function, _and_:
772. there is no possibility of a panic at any point where a premature exit would leave things in an unwanted state.
78
79Therefore, this optimization should be used only in small methods where the flow is simple and it is possible to prove that no panic can occur within the critical path.
80
81Less preferable:
82
83```go
84func (t *thing) getNumber() int {
85 t.lock.Lock()
86 defer t.lock.Unlock()
87 if t.isTwo {
88 return 2
89 }
90 return 1
91}
92```
93
94More preferable:
95
96```go
97func (t *thing) getNumber() int {
98 t.lock.Lock()
99 answer := 1
100 if t.isTwo {
101 answer = 2
102 }
103 t.lock.Unlock()
104 return answer
105}
106```
107
108Note that in this example, a panic _is_ possible on the first line if `t` is nil; but since the lock would not get locked in that scenario, things would still be left in a safe state even in that case.
109
110### Minimize overhead when logging at debug level
111
112The `Debug` logging level can be a useful diagnostic tool, and it is OK to log very verbosely at this level. However, to avoid slowing things down in the usual case where this log level is disabled, keep in mind:
113
1141. Avoid string concatenation; use `Printf`-style placeholders instead.
1152. Before calling `loggers.Debug` or `loggers.Debugf`, check `loggers.IsDebugEnabled()`. If it returns false, you should skip the `Debug` or `Debugf` call, since otherwise it can incur some overhead from converting the parameters of that call to `interface{}` values.
116
117### Test coverage
118
119It is important to keep unit test coverage as close to 100% as possible in this project. You can view the latest code coverage report in CircleCI, as `coverage.html` and `coverage.txt` in the artifacts for the latest Go version build. You can also generate this information locally with `make test-coverage`.
120
121The build will fail if there are any uncovered blocks of code, unless you explicitly add an override by placing a comment that starts with `// COVERAGE` somewhere within that block. Sometimes a gap in coverage is unavoidable, usually because the compiler requires us to provide a code path for some condition that in practice can't happen and can't be tested. Exclude these paths with a `// COVERAGE` comment.
View as plain text