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## Documentation
185
186Each (non-internal, non-test) package must be documented using
187[Go Doc Comments](https://go.dev/doc/comment),
188preferably in a `doc.go` file.
189
190Prefer using [Examples](https://pkg.go.dev/testing#hdr-Examples)
191instead of putting code snippets in Go doc comments.
192In some cases, you can even create [Testable Examples](https://go.dev/blog/examples).
193
194You can install and run a "local Go Doc site" in the following way:
195
196 ```sh
197 go install golang.org/x/pkgsite/cmd/pkgsite@latest
198 pkgsite
199 ```
200
201[`go.opentelemetry.io/otel/metric`](https://pkg.go.dev/go.opentelemetry.io/otel/metric)
202is an example of a very well-documented package.
203
204## Style Guide
205
206One of the primary goals of this project is that it is actually used by
207developers. With this goal in mind the project strives to build
208user-friendly and idiomatic Go code adhering to the Go community's best
209practices.
210
211For a non-comprehensive but foundational overview of these best practices
212the [Effective Go](https://golang.org/doc/effective_go.html) documentation
213is an excellent starting place.
214
215As a convenience for developers building this project the `make precommit`
216will format, lint, validate, and in some cases fix the changes you plan to
217submit. This check will need to pass for your changes to be able to be
218merged.
219
220In addition to idiomatic Go, the project has adopted certain standards for
221implementations of common patterns. These standards should be followed as a
222default, and if they are not followed documentation needs to be included as
223to the reasons why.
224
225### Configuration
226
227When creating an instantiation function for a complex `type T struct`, it is
228useful to allow variable number of options to be applied. However, the strong
229type system of Go restricts the function design options. There are a few ways
230to solve this problem, but we have landed on the following design.
231
232#### `config`
233
234Configuration should be held in a `struct` named `config`, or prefixed with
235specific type name this Configuration applies to if there are multiple
236`config` in the package. This type must contain configuration options.
237
238```go
239// config contains configuration options for a thing.
240type config struct {
241 // options ...
242}
243```
244
245In general the `config` type will not need to be used externally to the
246package and should be unexported. If, however, it is expected that the user
247will likely want to build custom options for the configuration, the `config`
248should be exported. Please, include in the documentation for the `config`
249how the user can extend the configuration.
250
251It is important that internal `config` are not shared across package boundaries.
252Meaning a `config` from one package should not be directly used by another. The
253one exception is the API packages. The configs from the base API, eg.
254`go.opentelemetry.io/otel/trace.TracerConfig` and
255`go.opentelemetry.io/otel/metric.InstrumentConfig`, are intended to be consumed
256by the SDK therefore it is expected that these are exported.
257
258When a config is exported we want to maintain forward and backward
259compatibility, to achieve this no fields should be exported but should
260instead be accessed by methods.
261
262Optionally, it is common to include a `newConfig` function (with the same
263naming scheme). This function wraps any defaults setting and looping over
264all options to create a configured `config`.
265
266```go
267// newConfig returns an appropriately configured config.
268func newConfig(options ...Option) config {
269 // Set default values for config.
270 config := config{/* […] */}
271 for _, option := range options {
272 config = option.apply(config)
273 }
274 // Perform any validation here.
275 return config
276}
277```
278
279If validation of the `config` options is also performed this can return an
280error as well that is expected to be handled by the instantiation function
281or propagated to the user.
282
283Given the design goal of not having the user need to work with the `config`,
284the `newConfig` function should also be unexported.
285
286#### `Option`
287
288To set the value of the options a `config` contains, a corresponding
289`Option` interface type should be used.
290
291```go
292type Option interface {
293 apply(config) config
294}
295```
296
297Having `apply` unexported makes sure that it will not be used externally.
298Moreover, the interface becomes sealed so the user cannot easily implement
299the interface on its own.
300
301The `apply` method should return a modified version of the passed config.
302This approach, instead of passing a pointer, is used to prevent the config from being allocated to the heap.
303
304The name of the interface should be prefixed in the same way the
305corresponding `config` is (if at all).
306
307#### Options
308
309All user configurable options for a `config` must have a related unexported
310implementation of the `Option` interface and an exported configuration
311function that wraps this implementation.
312
313The wrapping function name should be prefixed with `With*` (or in the
314special case of a boolean options `Without*`) and should have the following
315function signature.
316
317```go
318func With*(…) Option { … }
319```
320
321##### `bool` Options
322
323```go
324type defaultFalseOption bool
325
326func (o defaultFalseOption) apply(c config) config {
327 c.Bool = bool(o)
328 return c
329}
330
331// WithOption sets a T to have an option included.
332func WithOption() Option {
333 return defaultFalseOption(true)
334}
335```
336
337```go
338type defaultTrueOption bool
339
340func (o defaultTrueOption) apply(c config) config {
341 c.Bool = bool(o)
342 return c
343}
344
345// WithoutOption sets a T to have Bool option excluded.
346func WithoutOption() Option {
347 return defaultTrueOption(false)
348}
349```
350
351##### Declared Type Options
352
353```go
354type myTypeOption struct {
355 MyType MyType
356}
357
358func (o myTypeOption) apply(c config) config {
359 c.MyType = o.MyType
360 return c
361}
362
363// WithMyType sets T to have include MyType.
364func WithMyType(t MyType) Option {
365 return myTypeOption{t}
366}
367```
368
369##### Functional Options
370
371```go
372type optionFunc func(config) config
373
374func (fn optionFunc) apply(c config) config {
375 return fn(c)
376}
377
378// WithMyType sets t as MyType.
379func WithMyType(t MyType) Option {
380 return optionFunc(func(c config) config {
381 c.MyType = t
382 return c
383 })
384}
385```
386
387#### Instantiation
388
389Using this configuration pattern to configure instantiation with a `NewT`
390function.
391
392```go
393func NewT(options ...Option) T {…}
394```
395
396Any required parameters can be declared before the variadic `options`.
397
398#### Dealing with Overlap
399
400Sometimes there are multiple complex `struct` that share common
401configuration and also have distinct configuration. To avoid repeated
402portions of `config`s, a common `config` can be used with the union of
403options being handled with the `Option` interface.
404
405For example.
406
407```go
408// config holds options for all animals.
409type config struct {
410 Weight float64
411 Color string
412 MaxAltitude float64
413}
414
415// DogOption apply Dog specific options.
416type DogOption interface {
417 applyDog(config) config
418}
419
420// BirdOption apply Bird specific options.
421type BirdOption interface {
422 applyBird(config) config
423}
424
425// Option apply options for all animals.
426type Option interface {
427 BirdOption
428 DogOption
429}
430
431type weightOption float64
432
433func (o weightOption) applyDog(c config) config {
434 c.Weight = float64(o)
435 return c
436}
437
438func (o weightOption) applyBird(c config) config {
439 c.Weight = float64(o)
440 return c
441}
442
443func WithWeight(w float64) Option { return weightOption(w) }
444
445type furColorOption string
446
447func (o furColorOption) applyDog(c config) config {
448 c.Color = string(o)
449 return c
450}
451
452func WithFurColor(c string) DogOption { return furColorOption(c) }
453
454type maxAltitudeOption float64
455
456func (o maxAltitudeOption) applyBird(c config) config {
457 c.MaxAltitude = float64(o)
458 return c
459}
460
461func WithMaxAltitude(a float64) BirdOption { return maxAltitudeOption(a) }
462
463func NewDog(name string, o ...DogOption) Dog {…}
464func NewBird(name string, o ...BirdOption) Bird {…}
465```
466
467### Interfaces
468
469To allow other developers to better comprehend the code, it is important
470to ensure it is sufficiently documented. One simple measure that contributes
471to this aim is self-documenting by naming method parameters. Therefore,
472where appropriate, methods of every exported interface type should have
473their parameters appropriately named.
474
475#### Interface Stability
476
477All exported stable interfaces that include the following warning in their
478documentation are allowed to be extended with additional methods.
479
480> Warning: methods may be added to this interface in minor releases.
481
482These interfaces are defined by the OpenTelemetry specification and will be
483updated as the specification evolves.
484
485Otherwise, stable interfaces MUST NOT be modified.
486
487#### How to Change Specification Interfaces
488
489When an API change must be made, we will update the SDK with the new method one
490release before the API change. This will allow the SDK one version before the
491API change to work seamlessly with the new API.
492
493If an incompatible version of the SDK is used with the new API the application
494will fail to compile.
495
496#### How Not to Change Specification Interfaces
497
498We have explored using a v2 of the API to change interfaces and found that there
499was no way to introduce a v2 and have it work seamlessly with the v1 of the API.
500Problems happened with libraries that upgraded to v2 when an application did not,
501and would not produce any telemetry.
502
503More detail of the approaches considered and their limitations can be found in
504the [Use a V2 API to evolve interfaces](https://github.com/open-telemetry/opentelemetry-go/issues/3920)
505issue.
506
507#### How to Change Other Interfaces
508
509If new functionality is needed for an interface that cannot be changed it MUST
510be added by including an additional interface. That added interface can be a
511simple interface for the specific functionality that you want to add or it can
512be a super-set of the original interface. For example, if you wanted to a
513`Close` method to the `Exporter` interface:
514
515```go
516type Exporter interface {
517 Export()
518}
519```
520
521A new interface, `Closer`, can be added:
522
523```go
524type Closer interface {
525 Close()
526}
527```
528
529Code that is passed the `Exporter` interface can now check to see if the passed
530value also satisfies the new interface. E.g.
531
532```go
533func caller(e Exporter) {
534 /* ... */
535 if c, ok := e.(Closer); ok {
536 c.Close()
537 }
538 /* ... */
539}
540```
541
542Alternatively, a new type that is the super-set of an `Exporter` can be created.
543
544```go
545type ClosingExporter struct {
546 Exporter
547 Close()
548}
549```
550
551This new type can be used similar to the simple interface above in that a
552passed `Exporter` type can be asserted to satisfy the `ClosingExporter` type
553and the `Close` method called.
554
555This super-set approach can be useful if there is explicit behavior that needs
556to be coupled with the original type and passed as a unified type to a new
557function, but, because of this coupling, it also limits the applicability of
558the added functionality. If there exist other interfaces where this
559functionality should be added, each one will need their own super-set
560interfaces and will duplicate the pattern. For this reason, the simple targeted
561interface that defines the specific functionality should be preferred.
562
563### Testing
564
565The tests should never leak goroutines.
566
567Use the term `ConcurrentSafe` in the test name when it aims to verify the
568absence of race conditions.
569
570### Internal packages
571
572The use of internal packages should be scoped to a single module. A sub-module
573should never import from a parent internal package. This creates a coupling
574between the two modules where a user can upgrade the parent without the child
575and if the internal package API has changed it will fail to upgrade[^3].
576
577There are two known exceptions to this rule:
578
579- `go.opentelemetry.io/otel/internal/global`
580 - This package manages global state for all of opentelemetry-go. It needs to
581 be a single package in order to ensure the uniqueness of the global state.
582- `go.opentelemetry.io/otel/internal/baggage`
583 - This package provides values in a `context.Context` that need to be
584 recognized by `go.opentelemetry.io/otel/baggage` and
585 `go.opentelemetry.io/otel/bridge/opentracing` but remain private.
586
587If you have duplicate code in multiple modules, make that code into a Go
588template stored in `go.opentelemetry.io/otel/internal/shared` and use [gotmpl]
589to render the templates in the desired locations. See [#4404] for an example of
590this.
591
592[^3]: https://github.com/open-telemetry/opentelemetry-go/issues/3548
593
594## Approvers and Maintainers
595
596### Approvers
597
598- [Evan Torrie](https://github.com/evantorrie), Verizon Media
599- [Sam Xie](https://github.com/XSAM), Cisco/AppDynamics
600- [David Ashpole](https://github.com/dashpole), Google
601- [Chester Cheung](https://github.com/hanyuancheung), Tencent
602- [Damien Mathieu](https://github.com/dmathieu), Elastic
603- [Anthony Mirabella](https://github.com/Aneurysm9), AWS
604
605### Maintainers
606
607- [Aaron Clawson](https://github.com/MadVikingGod), LightStep
608- [Robert Pająk](https://github.com/pellared), Splunk
609- [Tyler Yahn](https://github.com/MrAlias), Splunk
610
611### Emeritus
612
613- [Gustavo Silva Paiva](https://github.com/paivagustavo), LightStep
614- [Josh MacDonald](https://github.com/jmacd), LightStep
615
616### Become an Approver or a Maintainer
617
618See the [community membership document in OpenTelemetry community
619repo](https://github.com/open-telemetry/community/blob/main/community-membership.md).
620
621[Approver]: #approvers
622[Maintainer]: #maintainers
623[gotmpl]: https://pkg.go.dev/go.opentelemetry.io/build-tools/gotmpl
624[#4404]: https://github.com/open-telemetry/opentelemetry-go/pull/4404
View as plain text