| # Contributing to opentelemetry-go |
| |
| The Go special interest group (SIG) meets regularly. See the |
| OpenTelemetry |
| [community](https://github.com/open-telemetry/community#golang-sdk) |
| repo for information on this and other language SIGs. |
| |
| See the [public meeting |
| notes](https://docs.google.com/document/d/1E5e7Ld0NuU1iVvf-42tOBpu2VBBLYnh73GJuITGJTTU/edit) |
| for a summary description of past meetings. To request edit access, |
| join the meeting or get in touch on |
| [Slack](https://cloud-native.slack.com/archives/C01NPAXACKT). |
| |
| ## Development |
| |
| You can view and edit the source code by cloning this repository: |
| |
| ```sh |
| git clone https://github.com/open-telemetry/opentelemetry-go.git |
| ``` |
| |
| Run `make test` to run the tests instead of `go test`. |
| |
| There are some generated files checked into the repo. To make sure |
| that the generated files are up-to-date, run `make` (or `make |
| precommit` - the `precommit` target is the default). |
| |
| The `precommit` target also fixes the formatting of the code and |
| checks the status of the go module files. |
| |
| Additionally, there is a `codespell` target that checks for common |
| typos in the code. It is not run by default, but you can run it |
| manually with `make codespell`. It will set up a virtual environment |
| in `venv` and install `codespell` there. |
| |
| If after running `make precommit` the output of `git status` contains |
| `nothing to commit, working tree clean` then it means that everything |
| is up-to-date and properly formatted. |
| |
| ## Pull Requests |
| |
| ### How to Send Pull Requests |
| |
| Everyone is welcome to contribute code to `opentelemetry-go` via |
| GitHub pull requests (PRs). |
| |
| To create a new PR, fork the project in GitHub and clone the upstream |
| repo: |
| |
| ```sh |
| go get -d go.opentelemetry.io/otel |
| ``` |
| |
| (This may print some warning about "build constraints exclude all Go |
| files", just ignore it.) |
| |
| This will put the project in `${GOPATH}/src/go.opentelemetry.io/otel`. |
| Alternatively, you can use `git` directly with: |
| |
| ```sh |
| git clone https://github.com/open-telemetry/opentelemetry-go |
| ``` |
| |
| (Note that `git clone` is *not* using the `go.opentelemetry.io/otel` name - |
| that name is a kind of a redirector to GitHub that `go get` can |
| understand, but `git` does not.) |
| |
| This will add the project as `opentelemetry-go` within the current directory. |
| |
| Enter the newly created directory and add your fork as a new remote: |
| |
| ```sh |
| git remote add <YOUR_FORK> git@github.com:<YOUR_GITHUB_USERNAME>/opentelemetry-go |
| ``` |
| |
| Check out a new branch, make modifications, run linters and tests, update |
| `CHANGELOG.md`, and push the branch to your fork: |
| |
| ```sh |
| git checkout -b <YOUR_BRANCH_NAME> |
| # edit files |
| # update changelog |
| make precommit |
| git add -p |
| git commit |
| git push <YOUR_FORK> <YOUR_BRANCH_NAME> |
| ``` |
| |
| Open a pull request against the main `opentelemetry-go` repo. Be sure to add the pull |
| request ID to the entry you added to `CHANGELOG.md`. |
| |
| Avoid rebasing and force-pushing to your branch to facilitate reviewing the pull request. |
| Rewriting Git history makes it difficult to keep track of iterations during code review. |
| All pull requests are squashed to a single commit upon merge to `main`. |
| |
| ### How to Receive Comments |
| |
| * If the PR is not ready for review, please put `[WIP]` in the title, |
| tag it as `work-in-progress`, or mark it as |
| [`draft`](https://github.blog/2019-02-14-introducing-draft-pull-requests/). |
| * Make sure CLA is signed and CI is clear. |
| |
| ### How to Get PRs Merged |
| |
| A PR is considered **ready to merge** when: |
| |
| * It has received two qualified approvals[^1]. |
| |
| This is not enforced through automation, but needs to be validated by the |
| maintainer merging. |
| * At least one of the qualified approvals needs to be from an |
| [Approver]/[Maintainer] affiliated with a different company than the author |
| of the PR. |
| * PRs introducing changes that have already been discussed and consensus |
| reached only need one qualified approval. The discussion and resolution |
| needs to be linked to the PR. |
| * Trivial changes[^2] only need one qualified approval. |
| |
| * All feedback has been addressed. |
| * All PR comments and suggestions are resolved. |
| * All GitHub Pull Request reviews with a status of "Request changes" have |
| been addressed. Another review by the objecting reviewer with a different |
| status can be submitted to clear the original review, or the review can be |
| dismissed by a [Maintainer] when the issues from the original review have |
| been addressed. |
| * Any comments or reviews that cannot be resolved between the PR author and |
| reviewers can be submitted to the community [Approver]s and [Maintainer]s |
| during the weekly SIG meeting. If consensus is reached among the |
| [Approver]s and [Maintainer]s during the SIG meeting the objections to the |
| PR may be dismissed or resolved or the PR closed by a [Maintainer]. |
| * Any substantive changes to the PR require existing Approval reviews be |
| cleared unless the approver explicitly states that their approval persists |
| across changes. This includes changes resulting from other feedback. |
| [Approver]s and [Maintainer]s can help in clearing reviews and they should |
| be consulted if there are any questions. |
| |
| * The PR branch is up to date with the base branch it is merging into. |
| * To ensure this does not block the PR, it should be configured to allow |
| maintainers to update it. |
| |
| * It has been open for review for at least one working day. This gives people |
| reasonable time to review. |
| * Trivial changes[^2] do not have to wait for one day and may be merged with |
| a single [Maintainer]'s approval. |
| |
| * All required GitHub workflows have succeeded. |
| * Urgent fix can take exception as long as it has been actively communicated |
| among [Maintainer]s. |
| |
| Any [Maintainer] can merge the PR once the above criteria have been met. |
| |
| [^1]: A qualified approval is a GitHub Pull Request review with "Approve" |
| status from an OpenTelemetry Go [Approver] or [Maintainer]. |
| [^2]: Trivial changes include: typo corrections, cosmetic non-substantive |
| changes, documentation corrections or updates, dependency updates, etc. |
| |
| ## Design Choices |
| |
| As with other OpenTelemetry clients, opentelemetry-go follows the |
| [OpenTelemetry Specification](https://opentelemetry.io/docs/specs/otel). |
| |
| It's especially valuable to read through the [library |
| guidelines](https://opentelemetry.io/docs/specs/otel/library-guidelines). |
| |
| ### Focus on Capabilities, Not Structure Compliance |
| |
| OpenTelemetry is an evolving specification, one where the desires and |
| use cases are clear, but the methods to satisfy those use cases are |
| not. |
| |
| As such, Contributions should provide functionality and behavior that |
| conforms to the specification, but the interface and structure are |
| flexible. |
| |
| It is preferable to have contributions follow the idioms of the |
| language rather than conform to specific API names or argument |
| patterns in the spec. |
| |
| For a deeper discussion, see |
| [this](https://github.com/open-telemetry/opentelemetry-specification/issues/165). |
| |
| ## Tests |
| |
| Each functionality should be covered by tests. |
| |
| Performance-critical functionality should also be covered by benchmarks. |
| |
| - Pull requests adding a performance-critical functionality |
| should have `go test -bench` output in their description. |
| - Pull requests changing a performance-critical functionality |
| should have [`benchstat`](https://pkg.go.dev/golang.org/x/perf/cmd/benchstat) |
| output in their description. |
| |
| ## Dependencies |
| |
| This project uses [Go Modules] for dependency management. All modules will use |
| `go.mod` to explicitly list all direct and indirect dependencies, ensuring a |
| clear dependency graph. The `go.sum` file for each module will be committed to |
| the repository and used to verify the integrity of downloaded modules, |
| preventing malicious tampering. |
| |
| This project uses automated dependency update tools (i.e. dependabot, |
| renovatebot) to manage updates to dependencies. This ensures that dependencies |
| are kept up-to-date with the latest security patches and features and are |
| reviewed before being merged. If you would like to propose a change to a |
| dependency it should be done through a pull request that updates the `go.mod` |
| file and includes a description of the change. |
| |
| See the [versioning and compatibility](./VERSIONING.md) policy for more details |
| about dependency compatibility. |
| |
| [Go Modules]: https://pkg.go.dev/cmd/go#hdr-Modules__module_versions__and_more |
| |
| ### Environment Dependencies |
| |
| This project does not partition dependencies based on the environment (i.e. |
| `development`, `staging`, `production`). |
| |
| Only the dependencies explicitly included in the released modules have been |
| tested and verified to work with the released code. No other guarantee is made |
| about the compatibility of other dependencies. |
| |
| ## Documentation |
| |
| Each (non-internal, non-test) package must be documented using |
| [Go Doc Comments](https://go.dev/doc/comment), |
| preferably in a `doc.go` file. |
| |
| Prefer using [Examples](https://pkg.go.dev/testing#hdr-Examples) |
| instead of putting code snippets in Go doc comments. |
| In some cases, you can even create [Testable Examples](https://go.dev/blog/examples). |
| |
| You can install and run a "local Go Doc site" in the following way: |
| |
| ```sh |
| go install golang.org/x/pkgsite/cmd/pkgsite@latest |
| pkgsite |
| ``` |
| |
| [`go.opentelemetry.io/otel/metric`](https://pkg.go.dev/go.opentelemetry.io/otel/metric) |
| is an example of a very well-documented package. |
| |
| ### README files |
| |
| Each (non-internal, non-test, non-documentation) package must contain a |
| `README.md` file containing at least a title, and a `pkg.go.dev` badge. |
| |
| The README should not be a repetition of Go doc comments. |
| |
| You can verify the presence of all README files with the `make verify-readmes` |
| command. |
| |
| ## Style Guide |
| |
| One of the primary goals of this project is that it is actually used by |
| developers. With this goal in mind the project strives to build |
| user-friendly and idiomatic Go code adhering to the Go community's best |
| practices. |
| |
| For a non-comprehensive but foundational overview of these best practices |
| the [Effective Go](https://golang.org/doc/effective_go.html) documentation |
| is an excellent starting place. |
| |
| We also recommend following the |
| [Go Code Review Comments](https://go.dev/wiki/CodeReviewComments) |
| that collects common comments made during reviews of Go code. |
| |
| As a convenience for developers building this project the `make precommit` |
| will format, lint, validate, and in some cases fix the changes you plan to |
| submit. This check will need to pass for your changes to be able to be |
| merged. |
| |
| In addition to idiomatic Go, the project has adopted certain standards for |
| implementations of common patterns. These standards should be followed as a |
| default, and if they are not followed documentation needs to be included as |
| to the reasons why. |
| |
| ### Configuration |
| |
| When creating an instantiation function for a complex `type T struct`, it is |
| useful to allow variable number of options to be applied. However, the strong |
| type system of Go restricts the function design options. There are a few ways |
| to solve this problem, but we have landed on the following design. |
| |
| #### `config` |
| |
| Configuration should be held in a `struct` named `config`, or prefixed with |
| specific type name this Configuration applies to if there are multiple |
| `config` in the package. This type must contain configuration options. |
| |
| ```go |
| // config contains configuration options for a thing. |
| type config struct { |
| // options ... |
| } |
| ``` |
| |
| In general the `config` type will not need to be used externally to the |
| package and should be unexported. If, however, it is expected that the user |
| will likely want to build custom options for the configuration, the `config` |
| should be exported. Please, include in the documentation for the `config` |
| how the user can extend the configuration. |
| |
| It is important that internal `config` are not shared across package boundaries. |
| Meaning a `config` from one package should not be directly used by another. The |
| one exception is the API packages. The configs from the base API, eg. |
| `go.opentelemetry.io/otel/trace.TracerConfig` and |
| `go.opentelemetry.io/otel/metric.InstrumentConfig`, are intended to be consumed |
| by the SDK therefore it is expected that these are exported. |
| |
| When a config is exported we want to maintain forward and backward |
| compatibility, to achieve this no fields should be exported but should |
| instead be accessed by methods. |
| |
| Optionally, it is common to include a `newConfig` function (with the same |
| naming scheme). This function wraps any defaults setting and looping over |
| all options to create a configured `config`. |
| |
| ```go |
| // newConfig returns an appropriately configured config. |
| func newConfig(options ...Option) config { |
| // Set default values for config. |
| config := config{/* […] */} |
| for _, option := range options { |
| config = option.apply(config) |
| } |
| // Perform any validation here. |
| return config |
| } |
| ``` |
| |
| If validation of the `config` options is also performed this can return an |
| error as well that is expected to be handled by the instantiation function |
| or propagated to the user. |
| |
| Given the design goal of not having the user need to work with the `config`, |
| the `newConfig` function should also be unexported. |
| |
| #### `Option` |
| |
| To set the value of the options a `config` contains, a corresponding |
| `Option` interface type should be used. |
| |
| ```go |
| type Option interface { |
| apply(config) config |
| } |
| ``` |
| |
| Having `apply` unexported makes sure that it will not be used externally. |
| Moreover, the interface becomes sealed so the user cannot easily implement |
| the interface on its own. |
| |
| The `apply` method should return a modified version of the passed config. |
| This approach, instead of passing a pointer, is used to prevent the config from being allocated to the heap. |
| |
| The name of the interface should be prefixed in the same way the |
| corresponding `config` is (if at all). |
| |
| #### Options |
| |
| All user configurable options for a `config` must have a related unexported |
| implementation of the `Option` interface and an exported configuration |
| function that wraps this implementation. |
| |
| The wrapping function name should be prefixed with `With*` (or in the |
| special case of a boolean options `Without*`) and should have the following |
| function signature. |
| |
| ```go |
| func With*(…) Option { … } |
| ``` |
| |
| ##### `bool` Options |
| |
| ```go |
| type defaultFalseOption bool |
| |
| func (o defaultFalseOption) apply(c config) config { |
| c.Bool = bool(o) |
| return c |
| } |
| |
| // WithOption sets a T to have an option included. |
| func WithOption() Option { |
| return defaultFalseOption(true) |
| } |
| ``` |
| |
| ```go |
| type defaultTrueOption bool |
| |
| func (o defaultTrueOption) apply(c config) config { |
| c.Bool = bool(o) |
| return c |
| } |
| |
| // WithoutOption sets a T to have Bool option excluded. |
| func WithoutOption() Option { |
| return defaultTrueOption(false) |
| } |
| ``` |
| |
| ##### Declared Type Options |
| |
| ```go |
| type myTypeOption struct { |
| MyType MyType |
| } |
| |
| func (o myTypeOption) apply(c config) config { |
| c.MyType = o.MyType |
| return c |
| } |
| |
| // WithMyType sets T to have include MyType. |
| func WithMyType(t MyType) Option { |
| return myTypeOption{t} |
| } |
| ``` |
| |
| ##### Functional Options |
| |
| ```go |
| type optionFunc func(config) config |
| |
| func (fn optionFunc) apply(c config) config { |
| return fn(c) |
| } |
| |
| // WithMyType sets t as MyType. |
| func WithMyType(t MyType) Option { |
| return optionFunc(func(c config) config { |
| c.MyType = t |
| return c |
| }) |
| } |
| ``` |
| |
| #### Instantiation |
| |
| Using this configuration pattern to configure instantiation with a `NewT` |
| function. |
| |
| ```go |
| func NewT(options ...Option) T {…} |
| ``` |
| |
| Any required parameters can be declared before the variadic `options`. |
| |
| #### Dealing with Overlap |
| |
| Sometimes there are multiple complex `struct` that share common |
| configuration and also have distinct configuration. To avoid repeated |
| portions of `config`s, a common `config` can be used with the union of |
| options being handled with the `Option` interface. |
| |
| For example. |
| |
| ```go |
| // config holds options for all animals. |
| type config struct { |
| Weight float64 |
| Color string |
| MaxAltitude float64 |
| } |
| |
| // DogOption apply Dog specific options. |
| type DogOption interface { |
| applyDog(config) config |
| } |
| |
| // BirdOption apply Bird specific options. |
| type BirdOption interface { |
| applyBird(config) config |
| } |
| |
| // Option apply options for all animals. |
| type Option interface { |
| BirdOption |
| DogOption |
| } |
| |
| type weightOption float64 |
| |
| func (o weightOption) applyDog(c config) config { |
| c.Weight = float64(o) |
| return c |
| } |
| |
| func (o weightOption) applyBird(c config) config { |
| c.Weight = float64(o) |
| return c |
| } |
| |
| func WithWeight(w float64) Option { return weightOption(w) } |
| |
| type furColorOption string |
| |
| func (o furColorOption) applyDog(c config) config { |
| c.Color = string(o) |
| return c |
| } |
| |
| func WithFurColor(c string) DogOption { return furColorOption(c) } |
| |
| type maxAltitudeOption float64 |
| |
| func (o maxAltitudeOption) applyBird(c config) config { |
| c.MaxAltitude = float64(o) |
| return c |
| } |
| |
| func WithMaxAltitude(a float64) BirdOption { return maxAltitudeOption(a) } |
| |
| func NewDog(name string, o ...DogOption) Dog {…} |
| func NewBird(name string, o ...BirdOption) Bird {…} |
| ``` |
| |
| ### Interfaces |
| |
| To allow other developers to better comprehend the code, it is important |
| to ensure it is sufficiently documented. One simple measure that contributes |
| to this aim is self-documenting by naming method parameters. Therefore, |
| where appropriate, methods of every exported interface type should have |
| their parameters appropriately named. |
| |
| #### Interface Stability |
| |
| All exported stable interfaces that include the following warning in their |
| documentation are allowed to be extended with additional methods. |
| |
| > Warning: methods may be added to this interface in minor releases. |
| |
| These interfaces are defined by the OpenTelemetry specification and will be |
| updated as the specification evolves. |
| |
| Otherwise, stable interfaces MUST NOT be modified. |
| |
| #### How to Change Specification Interfaces |
| |
| When an API change must be made, we will update the SDK with the new method one |
| release before the API change. This will allow the SDK one version before the |
| API change to work seamlessly with the new API. |
| |
| If an incompatible version of the SDK is used with the new API the application |
| will fail to compile. |
| |
| #### How Not to Change Specification Interfaces |
| |
| We have explored using a v2 of the API to change interfaces and found that there |
| was no way to introduce a v2 and have it work seamlessly with the v1 of the API. |
| Problems happened with libraries that upgraded to v2 when an application did not, |
| and would not produce any telemetry. |
| |
| More detail of the approaches considered and their limitations can be found in |
| the [Use a V2 API to evolve interfaces](https://github.com/open-telemetry/opentelemetry-go/issues/3920) |
| issue. |
| |
| #### How to Change Other Interfaces |
| |
| If new functionality is needed for an interface that cannot be changed it MUST |
| be added by including an additional interface. That added interface can be a |
| simple interface for the specific functionality that you want to add or it can |
| be a super-set of the original interface. For example, if you wanted to a |
| `Close` method to the `Exporter` interface: |
| |
| ```go |
| type Exporter interface { |
| Export() |
| } |
| ``` |
| |
| A new interface, `Closer`, can be added: |
| |
| ```go |
| type Closer interface { |
| Close() |
| } |
| ``` |
| |
| Code that is passed the `Exporter` interface can now check to see if the passed |
| value also satisfies the new interface. E.g. |
| |
| ```go |
| func caller(e Exporter) { |
| /* ... */ |
| if c, ok := e.(Closer); ok { |
| c.Close() |
| } |
| /* ... */ |
| } |
| ``` |
| |
| Alternatively, a new type that is the super-set of an `Exporter` can be created. |
| |
| ```go |
| type ClosingExporter struct { |
| Exporter |
| Close() |
| } |
| ``` |
| |
| This new type can be used similar to the simple interface above in that a |
| passed `Exporter` type can be asserted to satisfy the `ClosingExporter` type |
| and the `Close` method called. |
| |
| This super-set approach can be useful if there is explicit behavior that needs |
| to be coupled with the original type and passed as a unified type to a new |
| function, but, because of this coupling, it also limits the applicability of |
| the added functionality. If there exist other interfaces where this |
| functionality should be added, each one will need their own super-set |
| interfaces and will duplicate the pattern. For this reason, the simple targeted |
| interface that defines the specific functionality should be preferred. |
| |
| See also: |
| [Keeping Your Modules Compatible: Working with interfaces](https://go.dev/blog/module-compatibility#working-with-interfaces). |
| |
| ### Testing |
| |
| We allow using [`testify`](https://github.com/stretchr/testify) even though |
| it is seen as non-idiomatic according to |
| the [Go Test Comments](https://go.dev/wiki/TestComments#assert-libraries) page. |
| |
| The tests should never leak goroutines. |
| |
| Use the term `ConcurrentSafe` in the test name when it aims to verify the |
| absence of race conditions. The top-level tests with this term will be run |
| many times in the `test-concurrent-safe` CI job to increase the chance of |
| catching concurrency issues. This does not apply to subtests when this term |
| is not in their root name. |
| |
| ### Internal packages |
| |
| The use of internal packages should be scoped to a single module. A sub-module |
| should never import from a parent internal package. This creates a coupling |
| between the two modules where a user can upgrade the parent without the child, |
| and if the internal package API has changed, it will fail to upgrade[^3]. |
| |
| There are two known exceptions to this rule: |
| |
| - `go.opentelemetry.io/otel/internal/global` |
| - This package manages global state for all of opentelemetry-go. It needs to |
| be a single package in order to ensure the uniqueness of the global state. |
| - `go.opentelemetry.io/otel/internal/baggage` |
| - This package provides values in a `context.Context` that need to be |
| recognized by `go.opentelemetry.io/otel/baggage` and |
| `go.opentelemetry.io/otel/bridge/opentracing` but remain private. |
| |
| If you have duplicate code in multiple modules, make that code into a Go |
| template stored in `go.opentelemetry.io/otel/internal/shared` and use [gotmpl] |
| to render the templates in the desired locations. See [#4404] for an example of |
| this. |
| |
| [^3]: https://github.com/open-telemetry/opentelemetry-go/issues/3548 |
| |
| ### Ignoring context cancellation |
| |
| OpenTelemetry API implementations need to ignore the cancellation of the context that is |
| passed when recording a value (e.g. starting a span, recording a measurement, emitting a log). |
| Recording methods should not return an error describing the cancellation state of the context |
| when they complete, nor should they abort any work. |
| |
| This rule may not apply if the OpenTelemetry specification defines a timeout mechanism for |
| the method. In that case the context cancellation can be used for the timeout with the |
| restriction that this behavior is documented for the method. Otherwise, timeouts |
| are expected to be handled by the user calling the API, not the implementation. |
| |
| Stoppage of the telemetry pipeline is handled by calling the appropriate `Shutdown` method |
| of a provider. It is assumed the context passed from a user is not used for this purpose. |
| |
| Outside of the direct recording of telemetry from the API (e.g. exporting telemetry, |
| force flushing telemetry, shutting down a signal provider) the context cancellation |
| should be honored. This means all work done on behalf of the user provided context |
| should be canceled. |
| |
| ### Observability |
| |
| OpenTelemetry Go SDK components should be instrumented to enable users observability for the health and performance of the telemetry pipeline itself. |
| This allows operators to understand how well their observability infrastructure is functioning and to identify potential issues before they impact their applications. |
| |
| This section outlines the best practices for building instrumentation in OpenTelemetry Go SDK components. |
| |
| #### Environment Variable Activation |
| |
| Observability features are currently experimental. |
| They should be disabled by default and activated through the `OTEL_GO_X_OBSERVABILITY` environment variable. |
| This follows the established experimental feature pattern used throughout the SDK. |
| |
| Components should check for this environment variable using a consistent pattern: |
| |
| ```go |
| import "go.opentelemetry.io/otel/*/internal/x" |
| |
| if x.Observability.Enabled() { |
| // Initialize observability metrics |
| } |
| ``` |
| |
| **References**: |
| |
| - [stdouttrace exporter](./exporters/stdout/stdouttrace/internal/x/x.go) |
| - [sdk](./sdk/internal/x/x.go) |
| |
| #### Encapsulation |
| |
| Instrumentation should be encapsulated within a dedicated `struct` (e.g. `instrumentation`). |
| It should not be mixed into the instrumented component. |
| |
| Prefer this: |
| |
| ```go |
| type SDKComponent struct { |
| inst *instrumentation |
| } |
| |
| type instrumentation struct { |
| inflight otelconv.SDKComponentInflight |
| exported otelconv.SDKComponentExported |
| } |
| ``` |
| |
| To this: |
| |
| ```go |
| // ❌ Avoid this pattern. |
| type SDKComponent struct { |
| /* other SDKComponent fields... */ |
| |
| inflight otelconv.SDKComponentInflight |
| exported otelconv.SDKComponentExported |
| } |
| ``` |
| |
| The instrumentation code should not bloat the code being instrumented. |
| Likely, this means its own file, or its own package if it is complex or reused. |
| |
| #### Initialization |
| |
| Instrumentation setup should be explicit, side-effect free, and local to the relevant component. |
| Avoid relying on global or implicit [side effects][side-effect] for initialization. |
| |
| Encapsulate setup in constructor functions, ensuring clear ownership and scope: |
| |
| ```go |
| import ( |
| "errors" |
| |
| semconv "go.opentelemetry.io/otel/semconv/v1.37.0" |
| "go.opentelemetry.io/otel/semconv/v1.37.0/otelconv" |
| ) |
| |
| type SDKComponent struct { |
| inst *instrumentation |
| } |
| |
| func NewSDKComponent(config Config) (*SDKComponent, error) { |
| inst, err := newInstrumentation() |
| if err != nil { |
| return nil, err |
| } |
| return &SDKComponent{inst: inst}, nil |
| } |
| |
| type instrumentation struct { |
| inflight otelconv.SDKComponentInflight |
| exported otelconv.SDKComponentExported |
| } |
| |
| func newInstrumentation() (*instrumentation, error) { |
| if !x.Observability.Enabled() { |
| return nil, nil |
| } |
| |
| meter := otel.GetMeterProvider().Meter( |
| "<component-package-name>", |
| metric.WithInstrumentationVersion(sdk.Version()), |
| metric.WithSchemaURL(semconv.SchemaURL), |
| ) |
| |
| inst := &instrumentation{} |
| |
| var err, e error |
| inst.inflight, e = otelconv.NewSDKComponentInflight(meter) |
| err = errors.Join(err, e) |
| |
| inst.exported, e = otelconv.NewSDKComponentExported(meter) |
| err = errors.Join(err, e) |
| |
| return inst, err |
| } |
| ``` |
| |
| ```go |
| // ❌ Avoid this pattern. |
| func (c *Component) initObservability() { |
| // Initialize observability metrics |
| if !x.Observability.Enabled() { |
| return |
| } |
| |
| // Initialize observability metrics |
| c.inst = &instrumentation{/* ... */} |
| } |
| ``` |
| |
| [side-effect]: https://en.wikipedia.org/wiki/Side_effect_(computer_science) |
| |
| #### Performance |
| |
| When observability is disabled there should be little to no overhead. |
| |
| ```go |
| func (e *Exporter) ExportSpans(ctx context.Context, spans []trace.ReadOnlySpan) error { |
| if e.inst != nil { |
| attrs := expensiveOperation() |
| e.inst.recordSpanInflight(ctx, int64(len(spans)), attrs...) |
| } |
| // Export spans... |
| } |
| ``` |
| |
| ```go |
| // ❌ Avoid this pattern. |
| func (e *Exporter) ExportSpans(ctx context.Context, spans []trace.ReadOnlySpan) error { |
| attrs := expensiveOperation() |
| e.inst.recordSpanInflight(ctx, int64(len(spans)), attrs...) |
| // Export spans... |
| } |
| |
| func (i *instrumentation) recordSpanInflight(ctx context.Context, count int64, attrs ...attribute.KeyValue) { |
| if i == nil || i.inflight == nil { |
| return |
| } |
| i.inflight.Add(ctx, count, metric.WithAttributes(attrs...)) |
| } |
| ``` |
| |
| When observability is enabled, the instrumentation code paths should be optimized to reduce allocation and computation overhead. |
| |
| ##### Attribute and Option Allocation Management |
| |
| Pool attribute slices and options with [`sync.Pool`] to minimize allocations in measurement calls with dynamic attributes. |
| |
| ```go |
| var ( |
| attrPool = sync.Pool{ |
| New: func() any { |
| // Pre-allocate common capacity |
| knownCap := 8 // Adjust based on expected usage |
| s := make([]attribute.KeyValue, 0, knownCap) |
| // Return a pointer to avoid extra allocation on Put(). |
| return &s |
| }, |
| } |
| |
| addOptPool = &sync.Pool{ |
| New: func() any { |
| const n = 1 // WithAttributeSet |
| o := make([]metric.AddOption, 0, n) |
| // Return a pointer to avoid extra allocation on Put(). |
| return &o |
| }, |
| } |
| ) |
| |
| func (i *instrumentation) record(ctx context.Context, value int64, baseAttrs ...attribute.KeyValue) { |
| attrs := attrPool.Get().(*[]attribute.KeyValue) |
| defer func() { |
| *attrs = (*attrs)[:0] // Reset. |
| attrPool.Put(attrs) |
| }() |
| |
| *attrs = append(*attrs, baseAttrs...) |
| // Add any dynamic attributes. |
| *attrs = append(*attrs, semconv.OTelComponentName("exporter-1")) |
| |
| addOpt := addOptPool.Get().(*[]metric.AddOption) |
| defer func() { |
| *addOpt = (*addOpt)[:0] |
| addOptPool.Put(addOpt) |
| }() |
| |
| set := attribute.NewSet(*attrs...) |
| *addOpt = append(*addOpt, metric.WithAttributeSet(set)) |
| |
| i.counter.Add(ctx, value, *addOpt...) |
| } |
| ``` |
| |
| Pools are most effective when there are many pooled objects of the same sufficiently large size, and the objects are repeatedly used. |
| This amortizes the cost of allocation and synchronization. |
| Ideally, the pools should be scoped to be used as widely as possible within the component to maximize this efficiency while still ensuring correctness. |
| |
| [`sync.Pool`]: https://pkg.go.dev/sync#Pool |
| |
| ##### Cache common attribute sets for repeated measurements |
| |
| If a static set of attributes are used for measurements and they are known at compile time, pre-compute and cache these attributes. |
| |
| ```go |
| type spanLiveSetKey struct { |
| sampled bool |
| } |
| |
| var spanLiveSetCache = map[spanLiveSetKey]attribute.Set{ |
| {true}: attribute.NewSet( |
| otelconv.SDKSpanLive{}.AttrSpanSamplingResult( |
| otelconv.SpanSamplingResultRecordAndSample, |
| ), |
| ), |
| {false}: attribute.NewSet( |
| otelconv.SDKSpanLive{}.AttrSpanSamplingResult( |
| otelconv.SpanSamplingResultRecordOnly, |
| ), |
| ), |
| } |
| |
| func spanLiveSet(sampled bool) attribute.Set { |
| key := spanLiveSetKey{sampled: sampled} |
| return spanLiveSetCache[key] |
| } |
| ``` |
| |
| ##### Benchmarking |
| |
| Always provide benchmarks when introducing or refactoring instrumentation. |
| Demonstrate the impact (allocs/op, B/op, ns/op) in enabled/disabled scenarios: |
| |
| ```go |
| func BenchmarkExportSpans(b *testing.B) { |
| scenarios := []struct { |
| name string |
| obsEnabled bool |
| }{ |
| {"ObsDisabled", false}, |
| {"ObsEnabled", true}, |
| } |
| |
| for _, scenario := range scenarios { |
| b.Run(scenario.name, func(b *testing.B) { |
| b.Setenv( |
| "OTEL_GO_X_OBSERVABILITY", |
| strconv.FormatBool(scenario.obsEnabled), |
| ) |
| |
| exporter := NewExporter() |
| spans := generateTestSpans(100) |
| |
| b.ResetTimer() |
| b.ReportAllocs() |
| |
| for i := 0; i < b.N; i++ { |
| _ = exporter.ExportSpans(context.Background(), spans) |
| } |
| }) |
| } |
| } |
| ``` |
| |
| #### Error Handling and Robustness |
| |
| Errors should be reported back to the caller if possible, and partial failures should be handled as gracefully as possible. |
| |
| ```go |
| func newInstrumentation() (*instrumentation, error) { |
| if !x.Observability.Enabled() { |
| return nil, nil |
| } |
| |
| m := otel.GetMeterProvider().Meter(/* initialize meter */) |
| counter, err := otelconv.NewSDKComponentCounter(m) |
| // Use the partially initialized counter if available. |
| i := &instrumentation{counter: counter} |
| // Return any error to the caller. |
| return i, err |
| } |
| ``` |
| |
| ```go |
| // ❌ Avoid this pattern. |
| func newInstrumentation() *instrumentation { |
| if !x.Observability.Enabled() { |
| return nil, nil |
| } |
| |
| m := otel.GetMeterProvider().Meter(/* initialize meter */) |
| counter, err := otelconv.NewSDKComponentCounter(m) |
| if err != nil { |
| // ❌ Do not dump the error to the OTel Handler. Return it to the |
| // caller. |
| otel.Handle(err) |
| // ❌ Do not return nil if we can still use the partially initialized |
| // counter. |
| return nil |
| } |
| return &instrumentation{counter: counter} |
| } |
| ``` |
| |
| If the instrumented component cannot report the error to the user, let it report the error to `otel.Handle`. |
| |
| #### Context Propagation |
| |
| Ensure observability measurements receive the correct context, especially for trace exemplars and distributed context: |
| |
| ```go |
| func (e *Exporter) ExportSpans(ctx context.Context, spans []trace.ReadOnlySpan) error { |
| // Use the provided context for observability measurements |
| e.inst.recordSpanExportStarted(ctx, len(spans)) |
| |
| err := e.doExport(ctx, spans) |
| |
| if err != nil { |
| e.inst.recordSpanExportFailed(ctx, len(spans), err) |
| } else { |
| e.inst.recordSpanExportSucceeded(ctx, len(spans)) |
| } |
| |
| return err |
| } |
| ``` |
| |
| ```go |
| // ❌ Avoid this pattern. |
| func (e *Exporter) ExportSpans(ctx context.Context, spans []trace.ReadOnlySpan) error { |
| // ❌ Do not break the context propagation. |
| e.inst.recordSpanExportStarted(context.Background(), len(spans)) |
| |
| err := e.doExport(ctx, spans) |
| |
| /* ... */ |
| |
| return err |
| } |
| ``` |
| |
| #### Semantic Conventions Compliance |
| |
| All observability metrics should follow the [OpenTelemetry Semantic Conventions for SDK metrics](https://github.com/open-telemetry/semantic-conventions/blob/1cf2476ae5e518225a766990a28a6d5602bd5a30/docs/otel/sdk-metrics.md). |
| |
| Use the metric semantic conventions convenience package [otelconv](./semconv/v1.37.0/otelconv/metric.go). |
| |
| ##### Component Identification |
| |
| Component names and types should follow [semantic convention](https://github.com/open-telemetry/semantic-conventions/blob/1cf2476ae5e518225a766990a28a6d5602bd5a30/docs/registry/attributes/otel.md#otel-component-attributes). |
| |
| If a component is not a well-known type specified in the semantic conventions, use the package path scope type as a stable identifier. |
| |
| ```go |
| componentType := "go.opentelemetry.io/otel/sdk/trace.Span" |
| ``` |
| |
| ```go |
| // ❌ Do not do this. |
| componentType := "trace-span" |
| ``` |
| |
| The component name should be a stable unique identifier for the specific instance of the component. |
| |
| Use a global counter to ensure uniqueness if necessary. |
| |
| ```go |
| // Unique 0-based ID counter for component instances. |
| var componentIDCounter atomic.Int64 |
| |
| // nextID returns the next unique ID for a component. |
| func nextID() int64 { |
| return componentIDCounter.Add(1) - 1 |
| } |
| |
| // componentName returns a unique name for the component instance. |
| func componentName() attribute.KeyValue { |
| id := nextID() |
| name := fmt.Sprintf("%s/%d", componentType, id) |
| return semconv.OTelComponentName(name) |
| } |
| ``` |
| |
| The component ID will need to be resettable for deterministic testing. |
| If tests are in a different package than the component being tested (i.e. a `<component package>_test` package name), use a generated `counter` internal package to manage the counter. |
| See [stdouttrace exporter example](./exporters/stdout/stdouttrace/internal/gen.go) for reference. |
| |
| #### Testing |
| |
| Use deterministic testing with isolated state: |
| |
| ```go |
| func TestObservability(t *testing.T) { |
| // Restore state after test to ensure this does not affect other tests. |
| prev := otel.GetMeterProvider() |
| t.Cleanup(func() { otel.SetMeterProvider(prev) }) |
| |
| // Isolate the meter provider for deterministic testing |
| reader := metric.NewManualReader() |
| meterProvider := metric.NewMeterProvider(metric.WithReader(reader)) |
| otel.SetMeterProvider(meterProvider) |
| |
| // Use t.Setenv to ensure environment variable is restored after test. |
| t.Setenv("OTEL_GO_X_OBSERVABILITY", "true") |
| |
| // Reset component ID counter to ensure deterministic component names. |
| componentIDCounter.Store(0) |
| |
| /* ... test code ... */ |
| } |
| ``` |
| |
| Test order should not affect results. |
| Ensure that any global state (e.g. component ID counters) is reset between tests. |
| |
| ## Approvers and Maintainers |
| |
| ### Maintainers |
| |
| - [Damien Mathieu](https://github.com/dmathieu), Elastic ([GPG](https://keys.openpgp.org/search?q=5A126B972A81A6CE443E5E1B408B8E44F0873832)) |
| - [David Ashpole](https://github.com/dashpole), Google ([GPG](https://keys.openpgp.org/search?q=C0D1BDDCAAEAE573673085F176327DA4D864DC70)) |
| - [Robert Pająk](https://github.com/pellared), Splunk ([GPG](https://keys.openpgp.org/search?q=CDAD3A60476A3DE599AA5092E5F7C35A4DBE90C2)) |
| - [Sam Xie](https://github.com/XSAM), Splunk ([GPG](https://keys.openpgp.org/search?q=AEA033782371ABB18EE39188B8044925D6FEEBEA)) |
| - [Tyler Yahn](https://github.com/MrAlias), Splunk ([GPG](https://keys.openpgp.org/search?q=0x46B0F3E1A8B1BA5A)) |
| |
| For more information about the maintainer role, see the [community repository](https://github.com/open-telemetry/community/blob/main/guides/contributor/membership.md#maintainer). |
| |
| ### Approvers |
| |
| - [Flc](https://github.com/flc1125), Independent |
| |
| For more information about the approver role, see the [community repository](https://github.com/open-telemetry/community/blob/main/guides/contributor/membership.md#approver). |
| |
| ### Triagers |
| |
| - [Alex Kats](https://github.com/akats7), Capital One |
| |
| For more information about the triager role, see the [community repository](https://github.com/open-telemetry/community/blob/main/guides/contributor/membership.md#triager). |
| |
| ### Emeritus |
| |
| - [Aaron Clawson](https://github.com/MadVikingGod) |
| - [Anthony Mirabella](https://github.com/Aneurysm9) |
| - [Cheng-Zhen Yang](https://github.com/scorpionknifes) |
| - [Chester Cheung](https://github.com/hanyuancheung) |
| - [Evan Torrie](https://github.com/evantorrie) |
| - [Gustavo Silva Paiva](https://github.com/paivagustavo) |
| - [Josh MacDonald](https://github.com/jmacd) |
| - [Liz Fong-Jones](https://github.com/lizthegrey) |
| |
| For more information about the emeritus role, see the [community repository](https://github.com/open-telemetry/community/blob/main/guides/contributor/membership.md#emeritus-maintainerapprovertriager). |
| |
| ### Become an Approver or a Maintainer |
| |
| See the [community membership document in OpenTelemetry community |
| repo](https://github.com/open-telemetry/community/blob/main/guides/contributor/membership.md). |
| |
| [Approver]: #approvers |
| [Maintainer]: #maintainers |
| [gotmpl]: https://pkg.go.dev/go.opentelemetry.io/build-tools/gotmpl |
| [#4404]: https://github.com/open-telemetry/opentelemetry-go/pull/4404 |