1# Contributing to opentelemetry-go
2
3The Go special interest group (SIG) meets regularly. See the
4OpenTelemetry
5[community](https://github.com/open-telemetry/community#golang-sdk)
6repo for information on this and other language SIGs.
7
8See the [public meeting
9notes](https://docs.google.com/document/d/1E5e7Ld0NuU1iVvf-42tOBpu2VBBLYnh73GJuITGJTTU/edit)
10for a summary description of past meetings. To request edit access,
11join the meeting or get in touch on
12[Slack](https://cloud-native.slack.com/archives/C01NPAXACKT).
13
14## Development
15
16You can view and edit the source code by cloning this repository:
17
18```sh
19git clone https://github.com/open-telemetry/opentelemetry-go.git
20```
21
22Run `make test` to run the tests instead of `go test`.
23
24There are some generated files checked into the repo. To make sure
25that the generated files are up-to-date, run `make` (or `make
26precommit` - the `precommit` target is the default).
27
28The `precommit` target also fixes the formatting of the code and
29checks the status of the go module files.
30
31Additionally, there is a `codespell` target that checks for common
32typos in the code. It is not run by default, but you can run it
33manually with `make codespell`. It will set up a virtual environment
34in `venv` and install `codespell` there.
35
36If after running `make precommit` the output of `git status` contains
37`nothing to commit, working tree clean` then it means that everything
38is up-to-date and properly formatted.
39
40## Pull Requests
41
42### How to Send Pull Requests
43
44Everyone is welcome to contribute code to `opentelemetry-go` via
45GitHub pull requests (PRs).
46
47To create a new PR, fork the project in GitHub and clone the upstream
48repo:
49
50```sh
51go get -d go.opentelemetry.io/otel
52```
53
54(This may print some warning about "build constraints exclude all Go
55files", just ignore it.)
56
57This will put the project in `${GOPATH}/src/go.opentelemetry.io/otel`. You
58can alternatively use `git` directly with:
59
60```sh
61git clone https://github.com/open-telemetry/opentelemetry-go
62```
63
64(Note that `git clone` is *not* using the `go.opentelemetry.io/otel` name -
65that name is a kind of a redirector to GitHub that `go get` can
66understand, but `git` does not.)
67
68This would put the project in the `opentelemetry-go` directory in
69current working directory.
70
71Enter the newly created directory and add your fork as a new remote:
72
73```sh
74git remote add <YOUR_FORK> git@github.com:<YOUR_GITHUB_USERNAME>/opentelemetry-go
75```
76
77Check out a new branch, make modifications, run linters and tests, update
78`CHANGELOG.md`, and push the branch to your fork:
79
80```sh
81git checkout -b <YOUR_BRANCH_NAME>
82# edit files
83# update changelog
84make precommit
85git add -p
86git commit
87git push <YOUR_FORK> <YOUR_BRANCH_NAME>
88```
89
90Open a pull request against the main `opentelemetry-go` repo. Be sure to add the pull
91request ID to the entry you added to `CHANGELOG.md`.
92
93Avoid rebasing and force-pushing to your branch to facilitate reviewing the pull request.
94Rewriting Git history makes it difficult to keep track of iterations during code review.
95All pull requests are squashed to a single commit upon merge to `main`.
96
97### How to Receive Comments
98
99* If the PR is not ready for review, please put `[WIP]` in the title,
100 tag it as `work-in-progress`, or mark it as
101 [`draft`](https://github.blog/2019-02-14-introducing-draft-pull-requests/).
102* Make sure CLA is signed and CI is clear.
103
104### How to Get PRs Merged
105
106A PR is considered **ready to merge** when:
107
108* It has received two qualified approvals[^1].
109
110 This is not enforced through automation, but needs to be validated by the
111 maintainer merging.
112 * The qualified approvals need to be from [Approver]s/[Maintainer]s
113 affiliated with different companies. Two qualified approvals from
114 [Approver]s or [Maintainer]s affiliated with the same company counts as a
115 single qualified approval.
116 * PRs introducing changes that have already been discussed and consensus
117 reached only need one qualified approval. The discussion and resolution
118 needs to be linked to the PR.
119 * Trivial changes[^2] only need one qualified approval.
120
121* All feedback has been addressed.
122 * All PR comments and suggestions are resolved.
123 * All GitHub Pull Request reviews with a status of "Request changes" have
124 been addressed. Another review by the objecting reviewer with a different
125 status can be submitted to clear the original review, or the review can be
126 dismissed by a [Maintainer] when the issues from the original review have
127 been addressed.
128 * Any comments or reviews that cannot be resolved between the PR author and
129 reviewers can be submitted to the community [Approver]s and [Maintainer]s
130 during the weekly SIG meeting. If consensus is reached among the
131 [Approver]s and [Maintainer]s during the SIG meeting the objections to the
132 PR may be dismissed or resolved or the PR closed by a [Maintainer].
133 * Any substantive changes to the PR require existing Approval reviews be
134 cleared unless the approver explicitly states that their approval persists
135 across changes. This includes changes resulting from other feedback.
136 [Approver]s and [Maintainer]s can help in clearing reviews and they should
137 be consulted if there are any questions.
138
139* The PR branch is up to date with the base branch it is merging into.
140 * To ensure this does not block the PR, it should be configured to allow
141 maintainers to update it.
142
143* It has been open for review for at least one working day. This gives people
144 reasonable time to review.
145 * Trivial changes[^2] do not have to wait for one day and may be merged with
146 a single [Maintainer]'s approval.
147
148* All required GitHub workflows have succeeded.
149* Urgent fix can take exception as long as it has been actively communicated
150 among [Maintainer]s.
151
152Any [Maintainer] can merge the PR once the above criteria have been met.
153
154[^1]: A qualified approval is a GitHub Pull Request review with "Approve"
155 status from an OpenTelemetry Go [Approver] or [Maintainer].
156[^2]: Trivial changes include: typo corrections, cosmetic non-substantive
157 changes, documentation corrections or updates, dependency updates, etc.
158
159## Design Choices
160
161As with other OpenTelemetry clients, opentelemetry-go follows the
162[OpenTelemetry Specification](https://opentelemetry.io/docs/specs/otel).
163
164It's especially valuable to read through the [library
165guidelines](https://opentelemetry.io/docs/specs/otel/library-guidelines).
166
167### Focus on Capabilities, Not Structure Compliance
168
169OpenTelemetry is an evolving specification, one where the desires and
170use cases are clear, but the method to satisfy those uses cases are
171not.
172
173As such, Contributions should provide functionality and behavior that
174conforms to the specification, but the interface and structure is
175flexible.
176
177It is preferable to have contributions follow the idioms of the
178language rather than conform to specific API names or argument
179patterns in the spec.
180
181For a deeper discussion, see
182[this](https://github.com/open-telemetry/opentelemetry-specification/issues/165).
183
184## Tests
185
186Each functionality should be covered by tests.
187
188Performance-critical functionality should also be covered by benchmarks.
189
190- Pull requests adding a performance-critical functionality
191should have `go test -bench` output in their description.
192- Pull requests changing a performance-critical functionality
193should have [`benchstat`](https://pkg.go.dev/golang.org/x/perf/cmd/benchstat)
194output in their description.
195
196## Documentation
197
198Each (non-internal, non-test) package must be documented using
199[Go Doc Comments](https://go.dev/doc/comment),
200preferably in a `doc.go` file.
201
202Prefer using [Examples](https://pkg.go.dev/testing#hdr-Examples)
203instead of putting code snippets in Go doc comments.
204In some cases, you can even create [Testable Examples](https://go.dev/blog/examples).
205
206You can install and run a "local Go Doc site" in the following way:
207
208 ```sh
209 go install golang.org/x/pkgsite/cmd/pkgsite@latest
210 pkgsite
211 ```
212
213[`go.opentelemetry.io/otel/metric`](https://pkg.go.dev/go.opentelemetry.io/otel/metric)
214is an example of a very well-documented package.
215
216### README files
217
218Each (non-internal, non-test, non-documentation) package must contain a
219`README.md` file containing at least a title, and a `pkg.go.dev` badge.
220
221The README should not be a repetition of Go doc comments.
222
223You can verify the presence of all README files with the `make verify-readmes`
224command.
225
226## Style Guide
227
228One of the primary goals of this project is that it is actually used by
229developers. With this goal in mind the project strives to build
230user-friendly and idiomatic Go code adhering to the Go community's best
231practices.
232
233For a non-comprehensive but foundational overview of these best practices
234the [Effective Go](https://golang.org/doc/effective_go.html) documentation
235is an excellent starting place.
236
237As a convenience for developers building this project the `make precommit`
238will format, lint, validate, and in some cases fix the changes you plan to
239submit. This check will need to pass for your changes to be able to be
240merged.
241
242In addition to idiomatic Go, the project has adopted certain standards for
243implementations of common patterns. These standards should be followed as a
244default, and if they are not followed documentation needs to be included as
245to the reasons why.
246
247### Configuration
248
249When creating an instantiation function for a complex `type T struct`, it is
250useful to allow variable number of options to be applied. However, the strong
251type system of Go restricts the function design options. There are a few ways
252to solve this problem, but we have landed on the following design.
253
254#### `config`
255
256Configuration should be held in a `struct` named `config`, or prefixed with
257specific type name this Configuration applies to if there are multiple
258`config` in the package. This type must contain configuration options.
259
260```go
261// config contains configuration options for a thing.
262type config struct {
263 // options ...
264}
265```
266
267In general the `config` type will not need to be used externally to the
268package and should be unexported. If, however, it is expected that the user
269will likely want to build custom options for the configuration, the `config`
270should be exported. Please, include in the documentation for the `config`
271how the user can extend the configuration.
272
273It is important that internal `config` are not shared across package boundaries.
274Meaning a `config` from one package should not be directly used by another. The
275one exception is the API packages. The configs from the base API, eg.
276`go.opentelemetry.io/otel/trace.TracerConfig` and
277`go.opentelemetry.io/otel/metric.InstrumentConfig`, are intended to be consumed
278by the SDK therefore it is expected that these are exported.
279
280When a config is exported we want to maintain forward and backward
281compatibility, to achieve this no fields should be exported but should
282instead be accessed by methods.
283
284Optionally, it is common to include a `newConfig` function (with the same
285naming scheme). This function wraps any defaults setting and looping over
286all options to create a configured `config`.
287
288```go
289// newConfig returns an appropriately configured config.
290func newConfig(options ...Option) config {
291 // Set default values for config.
292 config := config{/* […] */}
293 for _, option := range options {
294 config = option.apply(config)
295 }
296 // Perform any validation here.
297 return config
298}
299```
300
301If validation of the `config` options is also performed this can return an
302error as well that is expected to be handled by the instantiation function
303or propagated to the user.
304
305Given the design goal of not having the user need to work with the `config`,
306the `newConfig` function should also be unexported.
307
308#### `Option`
309
310To set the value of the options a `config` contains, a corresponding
311`Option` interface type should be used.
312
313```go
314type Option interface {
315 apply(config) config
316}
317```
318
319Having `apply` unexported makes sure that it will not be used externally.
320Moreover, the interface becomes sealed so the user cannot easily implement
321the interface on its own.
322
323The `apply` method should return a modified version of the passed config.
324This approach, instead of passing a pointer, is used to prevent the config from being allocated to the heap.
325
326The name of the interface should be prefixed in the same way the
327corresponding `config` is (if at all).
328
329#### Options
330
331All user configurable options for a `config` must have a related unexported
332implementation of the `Option` interface and an exported configuration
333function that wraps this implementation.
334
335The wrapping function name should be prefixed with `With*` (or in the
336special case of a boolean options `Without*`) and should have the following
337function signature.
338
339```go
340func With*(…) Option { … }
341```
342
343##### `bool` Options
344
345```go
346type defaultFalseOption bool
347
348func (o defaultFalseOption) apply(c config) config {
349 c.Bool = bool(o)
350 return c
351}
352
353// WithOption sets a T to have an option included.
354func WithOption() Option {
355 return defaultFalseOption(true)
356}
357```
358
359```go
360type defaultTrueOption bool
361
362func (o defaultTrueOption) apply(c config) config {
363 c.Bool = bool(o)
364 return c
365}
366
367// WithoutOption sets a T to have Bool option excluded.
368func WithoutOption() Option {
369 return defaultTrueOption(false)
370}
371```
372
373##### Declared Type Options
374
375```go
376type myTypeOption struct {
377 MyType MyType
378}
379
380func (o myTypeOption) apply(c config) config {
381 c.MyType = o.MyType
382 return c
383}
384
385// WithMyType sets T to have include MyType.
386func WithMyType(t MyType) Option {
387 return myTypeOption{t}
388}
389```
390
391##### Functional Options
392
393```go
394type optionFunc func(config) config
395
396func (fn optionFunc) apply(c config) config {
397 return fn(c)
398}
399
400// WithMyType sets t as MyType.
401func WithMyType(t MyType) Option {
402 return optionFunc(func(c config) config {
403 c.MyType = t
404 return c
405 })
406}
407```
408
409#### Instantiation
410
411Using this configuration pattern to configure instantiation with a `NewT`
412function.
413
414```go
415func NewT(options ...Option) T {…}
416```
417
418Any required parameters can be declared before the variadic `options`.
419
420#### Dealing with Overlap
421
422Sometimes there are multiple complex `struct` that share common
423configuration and also have distinct configuration. To avoid repeated
424portions of `config`s, a common `config` can be used with the union of
425options being handled with the `Option` interface.
426
427For example.
428
429```go
430// config holds options for all animals.
431type config struct {
432 Weight float64
433 Color string
434 MaxAltitude float64
435}
436
437// DogOption apply Dog specific options.
438type DogOption interface {
439 applyDog(config) config
440}
441
442// BirdOption apply Bird specific options.
443type BirdOption interface {
444 applyBird(config) config
445}
446
447// Option apply options for all animals.
448type Option interface {
449 BirdOption
450 DogOption
451}
452
453type weightOption float64
454
455func (o weightOption) applyDog(c config) config {
456 c.Weight = float64(o)
457 return c
458}
459
460func (o weightOption) applyBird(c config) config {
461 c.Weight = float64(o)
462 return c
463}
464
465func WithWeight(w float64) Option { return weightOption(w) }
466
467type furColorOption string
468
469func (o furColorOption) applyDog(c config) config {
470 c.Color = string(o)
471 return c
472}
473
474func WithFurColor(c string) DogOption { return furColorOption(c) }
475
476type maxAltitudeOption float64
477
478func (o maxAltitudeOption) applyBird(c config) config {
479 c.MaxAltitude = float64(o)
480 return c
481}
482
483func WithMaxAltitude(a float64) BirdOption { return maxAltitudeOption(a) }
484
485func NewDog(name string, o ...DogOption) Dog {…}
486func NewBird(name string, o ...BirdOption) Bird {…}
487```
488
489### Interfaces
490
491To allow other developers to better comprehend the code, it is important
492to ensure it is sufficiently documented. One simple measure that contributes
493to this aim is self-documenting by naming method parameters. Therefore,
494where appropriate, methods of every exported interface type should have
495their parameters appropriately named.
496
497#### Interface Stability
498
499All exported stable interfaces that include the following warning in their
500documentation are allowed to be extended with additional methods.
501
502> Warning: methods may be added to this interface in minor releases.
503
504These interfaces are defined by the OpenTelemetry specification and will be
505updated as the specification evolves.
506
507Otherwise, stable interfaces MUST NOT be modified.
508
509#### How to Change Specification Interfaces
510
511When an API change must be made, we will update the SDK with the new method one
512release before the API change. This will allow the SDK one version before the
513API change to work seamlessly with the new API.
514
515If an incompatible version of the SDK is used with the new API the application
516will fail to compile.
517
518#### How Not to Change Specification Interfaces
519
520We have explored using a v2 of the API to change interfaces and found that there
521was no way to introduce a v2 and have it work seamlessly with the v1 of the API.
522Problems happened with libraries that upgraded to v2 when an application did not,
523and would not produce any telemetry.
524
525More detail of the approaches considered and their limitations can be found in
526the [Use a V2 API to evolve interfaces](https://github.com/open-telemetry/opentelemetry-go/issues/3920)
527issue.
528
529#### How to Change Other Interfaces
530
531If new functionality is needed for an interface that cannot be changed it MUST
532be added by including an additional interface. That added interface can be a
533simple interface for the specific functionality that you want to add or it can
534be a super-set of the original interface. For example, if you wanted to a
535`Close` method to the `Exporter` interface:
536
537```go
538type Exporter interface {
539 Export()
540}
541```
542
543A new interface, `Closer`, can be added:
544
545```go
546type Closer interface {
547 Close()
548}
549```
550
551Code that is passed the `Exporter` interface can now check to see if the passed
552value also satisfies the new interface. E.g.
553
554```go
555func caller(e Exporter) {
556 /* ... */
557 if c, ok := e.(Closer); ok {
558 c.Close()
559 }
560 /* ... */
561}
562```
563
564Alternatively, a new type that is the super-set of an `Exporter` can be created.
565
566```go
567type ClosingExporter struct {
568 Exporter
569 Close()
570}
571```
572
573This new type can be used similar to the simple interface above in that a
574passed `Exporter` type can be asserted to satisfy the `ClosingExporter` type
575and the `Close` method called.
576
577This super-set approach can be useful if there is explicit behavior that needs
578to be coupled with the original type and passed as a unified type to a new
579function, but, because of this coupling, it also limits the applicability of
580the added functionality. If there exist other interfaces where this
581functionality should be added, each one will need their own super-set
582interfaces and will duplicate the pattern. For this reason, the simple targeted
583interface that defines the specific functionality should be preferred.
584
585See also:
586[Keeping Your Modules Compatible: Working with interfaces](https://go.dev/blog/module-compatibility#working-with-interfaces).
587
588### Testing
589
590The tests should never leak goroutines.
591
592Use the term `ConcurrentSafe` in the test name when it aims to verify the
593absence of race conditions. The top-level tests with this term will be run
594many times in the `test-concurrent-safe` CI job to increase the chance of
595catching concurrency issues. This does not apply to subtests when this term
596is not in their root name.
597
598### Internal packages
599
600The use of internal packages should be scoped to a single module. A sub-module
601should never import from a parent internal package. This creates a coupling
602between the two modules where a user can upgrade the parent without the child
603and if the internal package API has changed it will fail to upgrade[^3].
604
605There are two known exceptions to this rule:
606
607- `go.opentelemetry.io/otel/internal/global`
608 - This package manages global state for all of opentelemetry-go. It needs to
609 be a single package in order to ensure the uniqueness of the global state.
610- `go.opentelemetry.io/otel/internal/baggage`
611 - This package provides values in a `context.Context` that need to be
612 recognized by `go.opentelemetry.io/otel/baggage` and
613 `go.opentelemetry.io/otel/bridge/opentracing` but remain private.
614
615If you have duplicate code in multiple modules, make that code into a Go
616template stored in `go.opentelemetry.io/otel/internal/shared` and use [gotmpl]
617to render the templates in the desired locations. See [#4404] for an example of
618this.
619
620[^3]: https://github.com/open-telemetry/opentelemetry-go/issues/3548
621
622### Ignoring context cancellation
623
624OpenTelemetry API implementations need to ignore the cancellation of the context that are
625passed when recording a value (e.g. starting a span, recording a measurement, emitting a log).
626Recording methods should not return an error describing the cancellation state of the context
627when they complete, nor should they abort any work.
628
629This rule may not apply if the OpenTelemetry specification defines a timeout mechanism for
630the method. In that case the context cancellation can be used for the timeout with the
631restriction that this behavior is documented for the method. Otherwise, timeouts
632are expected to be handled by the user calling the API, not the implementation.
633
634Stoppage of the telemetry pipeline is handled by calling the appropriate `Shutdown` method
635of a provider. It is assumed the context passed from a user is not used for this purpose.
636
637Outside of the direct recording of telemetry from the API (e.g. exporting telemetry,
638force flushing telemetry, shutting down a signal provider) the context cancellation
639should be honored. This means all work done on behalf of the user provided context
640should be canceled.
641
642## Approvers and Maintainers
643
644### Triagers
645
646- [Cheng-Zhen Yang](https://github.com/scorpionknifes), Independent
647
648### Approvers
649
650### Maintainers
651
652- [Damien Mathieu](https://github.com/dmathieu), Elastic
653- [David Ashpole](https://github.com/dashpole), Google
654- [Robert Pająk](https://github.com/pellared), Splunk
655- [Sam Xie](https://github.com/XSAM), Cisco/AppDynamics
656- [Tyler Yahn](https://github.com/MrAlias), Splunk
657
658### Emeritus
659
660- [Aaron Clawson](https://github.com/MadVikingGod)
661- [Anthony Mirabella](https://github.com/Aneurysm9)
662- [Chester Cheung](https://github.com/hanyuancheung)
663- [Evan Torrie](https://github.com/evantorrie)
664- [Gustavo Silva Paiva](https://github.com/paivagustavo)
665- [Josh MacDonald](https://github.com/jmacd)
666- [Liz Fong-Jones](https://github.com/lizthegrey)
667
668### Become an Approver or a Maintainer
669
670See the [community membership document in OpenTelemetry community
671repo](https://github.com/open-telemetry/community/blob/main/guides/contributor/membership.md).
672
673[Approver]: #approvers
674[Maintainer]: #maintainers
675[gotmpl]: https://pkg.go.dev/go.opentelemetry.io/build-tools/gotmpl
676[#4404]: https://github.com/open-telemetry/opentelemetry-go/pull/4404