Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

docs: Instrumentation Authors Guide #946

Merged
merged 21 commits into from
May 13, 2024
Merged
Changes from 9 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
94 changes: 80 additions & 14 deletions instrumentation/CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ The following steps are required to contribute a new instrumentation library:

This repository contains a script to generate a new instrumentation library.

The snippit below demonstrates how to generate a an instrumentation for the `werewolf` gem, starting from the repository root.
The snippet below demonstrates how to generate a an instrumentation for the `werewolf` gem, starting from the repository root.

```console

Expand Down Expand Up @@ -92,26 +92,69 @@ The output of the generator shows that it creates a new directory in the `instru

The original design and implementation of this project was heavily influenced by Datadog's `dd-trace-rb` project. You may refer to the [Datadog Porting Guide](datadog-porting-guide.md) as a reference for implementing instrumentations, however the following guidelines are specific to OpenTelemetry Ruby:
arielvalentin marked this conversation as resolved.
Show resolved Hide resolved

*. Use `OpenTelemetry::Instrumentation::Base`
*. Use the OpenTelemetry API
*. Use First Party Extension Points
*. Use Semantic Conventions
*. Write comprehensive automated tests
*. Understand Performance Characteristics
* Use `OpenTelemetry::Instrumentation::Base`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this group of bullets matches the headings for the sections below, what do you think about adding anchor links?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GitHub MD auto generates anchor links to a heading when you click it. Do you mean having sections use links to refer to each other?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's right, I forgot the links were automatically generated! So it would be linking whatever the auto-generated value is for the heading to the bullet with the same heading:

* [Use `OpenTelemetry::Instrumentation::Base`](#anchor-link-to-Use-OpenTelemetry-Instrumentation-Base-heading)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. Treating that bulleted list more like a sub table of contents.

* Use the OpenTelemetry API
kaylareopelle marked this conversation as resolved.
Show resolved Hide resolved
* Use First Party Extension Points
arielvalentin marked this conversation as resolved.
Show resolved Hide resolved
* Use Semantic Conventions
* Write comprehensive automated tests
* Understand Performance Characteristics
arielvalentin marked this conversation as resolved.
Show resolved Hide resolved

### Use `OpenTelemetry::Instrumentation::Base`

The entry point of your instrumentation should be implemented as a subclass of `OpenTelemetry::Instrumentation::Base`:

* Implement `install` block, where all of the integration work happens
arielvalentin marked this conversation as resolved.
Show resolved Hide resolved
* Implement `present` block
* Implement `compatible` block and check for at least the minumum required gem version
* Implement `present` block, which is you check if the library you are instrumenting was loaded
arielvalentin marked this conversation as resolved.
Show resolved Hide resolved
* Implement `compatible` block and check for at least the minumum required library version
arielvalentin marked this conversation as resolved.
Show resolved Hide resolved
* Any custom `options` you want to support
arielvalentin marked this conversation as resolved.
Show resolved Hide resolved

The example below demonstrates how to implement the `Werewolf` instrumentation:

```ruby

# frozen_string_literal: true

# Copyright The OpenTelemetry Authors
#
# SPDX-License-Identifier: Apache-2.0

module OpenTelemetry
module Instrumentation
module Werewolf
class Instrumentation < OpenTelemetry::Instrumentation::Base
MINIMUM_VERSION = Gem::Version.new('0.1.0')

install do |_config|
require_relative 'handlers'
Handlers.subscribe
end

option :transformations, default: :omit, validate: %i[omit include]

present do
defined?(::Werewolf) && defined?(::ActiveSupport)
end

compatible do
Gem::Version.new(::Wereworlf::VERSION) >= MINIMUM_VERSION
end
arielvalentin marked this conversation as resolved.
Show resolved Hide resolved
end
end
end
end

```

* The `install` block lazily requires the instrumentation handlers, which subscribe to events published by the `Werewolf` event hooks.
* The `present` block checks if the `Werewolf` and `ActiveSupport` libraries are loaded, which it will use to subscribe to events and generate spans. It will skip the installation if those dependencies were not loaded before the instrumentation was being initialized.
arielvalentin marked this conversation as resolved.
Show resolved Hide resolved
* The `compatible` block checks if the `Werewolf` library version is at least `0.1.0` and will skip it if it is not.
arielvalentin marked this conversation as resolved.
Show resolved Hide resolved
* The `options` section allows you to define custom options that can be passed to the instrumentation. In this example, the `transformations` option is defined with a default value of `:omit` and a validation rule that only allows `:omit` or `:include` values.
arielvalentin marked this conversation as resolved.
Show resolved Hide resolved

### Use the OpenTelemetry API

Instrumentations are intended to be portable and usable with vendor distributions of the SDK. For this reason, you must use the OpenTelemetry API to create spans and add attributes, events, and links to spans and avoid using the OpenTelemetry SDK directly.
arielvalentin marked this conversation as resolved.
Show resolved Hide resolved

Each instrumentation _must_ use a named tracer. Instrumentations that inherit from `OpenTelemetry::Instrumentation::Base` will get a single helper method that will automatically provide your instrumentation with a named tracer under `OpenTelemetry::Instrumentation::${Gem Name>}::Instrumentation.instance.tracer`.
Each instrumentation _must_ use a named tracer. Instrumentations that inherit from `OpenTelemetry::Instrumentation::Base` will get a single helper method that will automatically provide your instrumentation with a named tracer under `OpenTelemetry::Instrumentation::${Gem Name}::Instrumentation.instance.tracer`.

For example, the `Werewolf` generated in the example above instrumentation is available `OpenTelemetry::Instrumentation::Werewolf::Instrumentation.instance.tracer`. You should reference this tracer in your code when creating spans like this:
arielvalentin marked this conversation as resolved.
Show resolved Hide resolved

Expand Down Expand Up @@ -151,11 +194,34 @@ When you do in fact run into cases where test doubles or API stubs are absolutel

### Understand Performance Characteristics
arielvalentin marked this conversation as resolved.
Show resolved Hide resolved

Instrumentation libraries should be as lightweight as possible
Instrumentation libraries should be as lightweight as possible and must:

* Avoid allocating too many objects and private methods
* Avoid allocating objects unless absolutely necessary
arielvalentin marked this conversation as resolved.
Show resolved Hide resolved
* Rely on `rubocop-performance` linters to catch performance issues
* Consider using [microbenchmarks](https://github.com/evanphx/benchmark-ips) and [profiling](https://ruby-prof.github.io/) to address any possible performance issues
* Provide minimal solutions and code paths

#### Minimal Solutions are Better

Instrumentations should have both the minimal amount of code necessary to provide useful insights to our users.
arielvalentin marked this conversation as resolved.
Show resolved Hide resolved

It may sound contrary to good engineering practices, but you should avoid adding lots of small methods, classes, and objects to handle your use cases.

Adding lots of small well factored code adds some overhead to the library we are instrumenting. It may result in unnecessary allocations, method dispatching, and other performance overhead. It will end up contributing to building large backtraces and making it harder to understand what is happening in the application, which will likely result in additional filtering logic.

In cases when code uses monkey patching, it runs the risk of _adding_ methods that conflict with the internal implementatation of the library and may result in unexpected behavior and bugs.
arielvalentin marked this conversation as resolved.
Show resolved Hide resolved

Avoid instrumenting every method in a library, instead focus on the methods the provide the _most_ insights into what typically causes performance problems for applications, e.g. I/O and network calls.

Hopefully in the near future, we will be able to provide [OTel Profiling](https://opentelemetry.io/blog/2024/profiling/) to help users gain an even deeper understanding of what is happening in their applications at a more granular level.
arielvalentin marked this conversation as resolved.
Show resolved Hide resolved

#### Avoid Adding Custom Extensions (E_TOOMANYOPTIONS)
arielvalentin marked this conversation as resolved.
Show resolved Hide resolved

Though your instrumentation may accept configurations options to customize the output, you should consider that the more options you add, the more complexity you will have to manage.

You should _avoid_ adding options that allow custom code blocks to be executed as part of the instrumentation. It is often difficult to predict error modes and the performance impact custom code will have on your instrumentation, which in turn will impact the service being instrumented.
arielvalentin marked this conversation as resolved.
Show resolved Hide resolved

You should steer users towards post processing as part of the OTel Collector, which has a richer and more powerful toolset, and execute out of the application critical code path.
arielvalentin marked this conversation as resolved.
Show resolved Hide resolved

## Enable CI

Expand Down Expand Up @@ -322,15 +388,15 @@ The `instrumentation_generator` will create a `README.md` file for your instrume
3. Any known limitations or caveats
4. The minimum supported gem version

In addition to that There should also be redundant `yardoc` comments in the entrypoint of your gem, i.e. the subclass `OpenTelemetry::Instrumentation::Base`.
In addition to that, there should also be redundant `yardoc` comments in the entrypoint of your gem, i.e. the subclass `OpenTelemetry::Instrumentation::Base`.

> :information_source: See the `ActiveJob` instrumentation [`README`](./active_job/README.md) for a comprehensive example.

### Examples

Executuable examples should be included in the `examples` directory that demonstrate how to use the instrumentation in a real-world scenario.

We recommend using Bundler's inline gemfile to run the examples. Here is an example from the `grape` instrumentation:
We recommend using [Bundler's inline gemfile](https://bundler.io/guides/bundler_in_a_single_file_ruby_script.html) to run the examples. Here is an example from the `grape` instrumentation:

```ruby

Expand Down