From f5f3e68c943134a341d6f5c8ef55674e13d3af06 Mon Sep 17 00:00:00 2001 From: Ariel Valentin Date: Sat, 27 Apr 2024 10:14:23 -0500 Subject: [PATCH 01/17] docs: Instrumentation Authors Guide Provides explicit guidance for instrumentation authors to reduce friction when submitting contributions --- CONTRIBUTING.md | 95 ++-- instrumentation/CONTRIBUTING.md | 407 ++++++++++++++++++ instrumentation/README.md | 9 +- instrumentation/datadog-porting-guide.md | 8 +- .../grape/example/trace_demonstration.rb | 19 +- 5 files changed, 478 insertions(+), 60 deletions(-) create mode 100644 instrumentation/CONTRIBUTING.md diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index a8a5fe37b..05ad92d5b 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -24,7 +24,7 @@ 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: https://github.com/open-telemetry/opentelemetry-specification/issues/165 +For a deeper discussion, see: ## Getting started @@ -40,6 +40,7 @@ git clone git@github.com:YOUR_GITHUB_NAME/opentelemetry-ruby-contrib.git ``` or + ```sh git clone https://github.com/YOUR_GITHUB_NAME/opentelemetry-ruby-contrib.git ``` @@ -66,10 +67,10 @@ _Setting up a running Ruby environment is outside the scope of this document._ This repository contains multiple Ruby gems: - * Various instrumentation gems located in subdirectories of `instrumentation` - * Various resource detector gems located in subdirectories of `resources` - * `opentelemetry-propagator-xray` located in the `propagator/xray` directory - * `opentelemetry-propagator-ottrace` located in the `propagator/ottrace` directory +* Various instrumentation gems located in subdirectories of `instrumentation` +* Various resource detector gems located in subdirectories of `resources` +* `opentelemetry-propagator-xray` located in the `propagator/xray` directory +* `opentelemetry-propagator-ottrace` located in the `propagator/ottrace` directory Each of these gems has its configuration and tests. @@ -88,9 +89,9 @@ configuration details. The services provided include: - * `app` - main container environment scoped to the `/app` directory. Used +* `app` - main container environment scoped to the `/app` directory. Used primarily to build and tag the `opentelemetry/opentelemetry-ruby-contrib:latest` image. - * `x-instrumentation-` - container environment scoped to a specific instrumentation library. See `docker-compose.yml` for available services. +* `x-instrumentation-` - container environment scoped to a specific instrumentation library. See `docker-compose.yml` for available services. To test using Docker: @@ -100,9 +101,9 @@ To test using Docker: * `docker-compose build` * This makes the image available locally 4. Install dependencies for the service you want to interact with - * `docker-compose run bundle install` + * `docker-compose run bundle install` 5. Run the tests - * `docker-compose run bundle exec rake test` + * `docker-compose run bundle exec rake test` ## Processing and visualizing traces locally @@ -155,8 +156,8 @@ to ensure that your code complies before opening a pull request. We also use Yard to generate class documentation automatically. Among other things, this means: - * Methods and arguments should include the appropriate type annotations - * You can use markdown formatting in your documentation comments +* Methods and arguments should include the appropriate type annotations +* You can use markdown formatting in your documentation comments You can generate the docs locally to see the results, by running: @@ -165,6 +166,10 @@ bundle install bundle exec rake yard ``` +## Instrumentation Authors Guide + +Please make sure that you review the [Instrumentation Authors Guide](.instrumentation/CONTRIBUTING.md) before submitting a new instrumentation. + ## Create a Pull Request You'll need to create a Pull Request once you've finished your work. @@ -223,8 +228,8 @@ Releases are normally performed using GitHub Actions. 2. In the GitHub UI, go to the `Actions` tab, select the `Open release request` workflow, and run the workflow manually using the dropdown in the upper right. - * Releases must be run from the main branch. - * If you leave the `Gems to release` field, blank, and the script will + * Releases must be run from the main branch. + * If you leave the `Gems to release` field, blank, and the script will find all the gems that have had conventional-commit-tagged changes since their last release. Alternately, you can specify which gems to release by including their names, space-delimited, in this this field. You can @@ -247,9 +252,9 @@ Releases are normally performed using GitHub Actions. was opened; make sure the label is still there when you merge it. 6. The automated release script will run automatically, and will release the gem(s) once CI has completed. This includes: - * For each gem, it will create a release tag and a GitHub release. - * It will build and push the gems to rubygems. - * If the releases succeed, the script will update the release pull + * For each gem, it will create a release tag and a GitHub release. + * It will build and push the gems to rubygems. + * If the releases succeed, the script will update the release pull request with the results and change its label to `release: complete`. If something went wrong, the script will, if possible, report the error on the release pull request and change its label to `release: error`. @@ -268,20 +273,20 @@ review the release logs for the GitHub Actions workflows. There are four GitHub actions workflows related to releases. - * `Open release request` is the main release entrypoint, and is used to open - a release pull request. If something goes wrong with this process, the logs - will appear in the workflow run. - * `Force release` is generally used only to restart a failed release. - * `[release hook] Update open releases` is run on pushes to the main branch, - and pushes warnings to open release pull requests if you make modifications - before triggering the release (i.e. because you might need to update the - changelogs.) - * `[release hook] Process release` is the main release automation script and - is run when a pull request is closed. If it determines that a release pull - request was merged, it kicks off the release process for the affected gems. - It also updates the label on a closed release pull request. Finally, it - deletes release branches when they are no longer being used. If something - goes wrong with any of these processes, the logs will appear here. +* `Open release request` is the main release entrypoint, and is used to open + a release pull request. If something goes wrong with this process, the logs + will appear in the workflow run. +* `Force release` is generally used only to restart a failed release. +* `[release hook] Update open releases` is run on pushes to the main branch, + and pushes warnings to open release pull requests if you make modifications + before triggering the release (i.e. because you might need to update the + changelogs.) +* `[release hook] Process release` is the main release automation script and + is run when a pull request is closed. If it determines that a release pull + request was merged, it kicks off the release process for the affected gems. + It also updates the label on a closed release pull request. Finally, it + deletes release branches when they are no longer being used. If something + goes wrong with any of these processes, the logs will appear here. #### Restarting a release @@ -292,14 +297,14 @@ release, you can use the `Force release` workflow. GitHub UI. 2. In the GitHub UI, go to the `Actions` tab, select the `Force release` workflow, and run it manually. - * You must provide the gem name and version explicitly in the fields. - * The `Extra flags` field is useful for advanced cases. For example, if - the GitHub release tag is already created and the gem already pushed to - Rubygems, but the docs still need to be built, you can pass - `--only=docs` to perform only that one step. You can also force a - release even if the build is not green or the version/changelog checks - are failing, by passing `--skip-checks`. For more details, install the - `toys` gem and run `toys release perform --help` locally. + * You must provide the gem name and version explicitly in the fields. + * The `Extra flags` field is useful for advanced cases. For example, if + the GitHub release tag is already created and the gem already pushed to + Rubygems, but the docs still need to be built, you can pass + `--only=docs` to perform only that one step. You can also force a + release even if the build is not green or the version/changelog checks + are failing, by passing `--skip-checks`. For more details, install the + `toys` gem and run `toys release perform --help` locally. #### Running releases locally @@ -319,7 +324,7 @@ changed gems. To force-release, assuming the version and changelog are already modified: -``` +```console toys release perform --rubygems-api-key=$API_KEY $GEM_NAME $GEM_VERSION ``` @@ -337,12 +342,12 @@ not correspond exactly to the gem name. For releases to succeed, new gems MUST include the following: - * The above configuration entry. - * The `*.gemspec` file, with the name matching the gem name. - * A `version.rb` file in the standard location, or in a location listed in - the configuration. - * A `CHANGELOG.md` file. - * A `yard` rake task. +* The above configuration entry. +* The `*.gemspec` file, with the name matching the gem name. +* A `version.rb` file in the standard location, or in a location listed in + the configuration. +* A `CHANGELOG.md` file. +* A `yard` rake task. ## Dependabot Updates diff --git a/instrumentation/CONTRIBUTING.md b/instrumentation/CONTRIBUTING.md new file mode 100644 index 000000000..a3fe3df27 --- /dev/null +++ b/instrumentation/CONTRIBUTING.md @@ -0,0 +1,407 @@ +# Instrumentation Authors Guide + +This guide is for authors of OpenTelemetry Ruby instrumentation libraries. It provides guidance on how to contribute an instrumentation library. + +Please make sure to read and understand the [CONTRIBUTING](../CONTRIBUTING.md) guide before submitting any changes to instrumentation. + +## What we expect from you + +We are a community of volunteers with a shared goal of improving observability in Ruby applications. + +We welcome contributions from everyone, and we want to make sure that you have a great experience contributing to this project, as well as a positive impact on the Ruby community. + +We have limited capacity to maintain instrumentation libraries, so we ask that you commit to maintaining the instrumentation library you contribute. + +In addition to the requirements to maintain at least community member status, contributing an instrumentation to this project requires the following: + +1. Responding to issues and pull requests +2. Performing timely code reviews and responding to issues +3. Addressing security vulnerabilities +4. Keeping the instrumentation library up to date with the latest: + * OpenTelemetry Ruby API and SDK changes + * Ruby language changes + * Instrumented library changes + +If you do not have the capacity to maintain the instrumentation library, please consider contributing to the OpenTelemetry Ruby project in other ways, or consider creating a separate project for the instrumentation library. + +> :warning: Libraries that do not meet these requirements may be removed from the project at any time at the discretion of OpenTelemetry Ruby Contrib Maintainers. + +## Contributing a New Instrumentation Library + +Our long-term goal is to provide instrumentation for all popular Ruby libraries. Ideally we would like to have first-party instrumentation for all libraries maintained by the Gem authors to ensure compatability with upstream gems, however in many cases this is not possible. + +For this reason we welcome contributions of new instrumentation libraries that cannot be maintained by the original gem authors as first party instrumentations. + +The following steps are required to contribute a new instrumentation library: + +1. Generate an instrumentation gem skeleton +2. Implement the instrumentation library including comprehensive automated tests +3. Add the instrumentation library to the appropriate CI workflow +4. Include documentation for your instrumentation + * Document all instrumentation-specific configuration options in the `README.md` and `yardoc` comments + * Document all semantic conventions used by the instrumentation in the `README.md` + * Provide executable examples in an `examples` directory +5. Submit a pull request + +## Generate the Gem + +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. + +```console + +$bash opentelemetry-ruby-contrib> bin/instrumentation_generator werewolf + +``` + +The output of the generator shows that it creates a new directory in the `instrumentation` directory using the name of the gem: + +``` console + +🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨 + + WARNING: Your gem will *NOT* be tested until you add it to the CI workflows in `.github/workflows/ci.yml`!! + +🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨 + + create instrumentation/werewolf/.rubocop.yml + create instrumentation/werewolf/.yardopts + create instrumentation/werewolf/Appraisals + create instrumentation/werewolf/CHANGELOG.md + create instrumentation/werewolf/Gemfile + create instrumentation/werewolf/LICENSE + create instrumentation/werewolf/opentelemetry-instrumentation-werewolf.gemspec + create instrumentation/werewolf/Rakefile + create instrumentation/werewolf/README.md + create instrumentation/werewolf/lib/opentelemetry-instrumentation-werewolf.rb + create instrumentation/werewolf/lib/opentelemetry/instrumentation.rb + create instrumentation/werewolf/lib/opentelemetry/instrumentation/werewolf.rb + create instrumentation/werewolf/lib/opentelemetry/instrumentation/werewolf/instrumentation.rb + create instrumentation/werewolf/lib/opentelemetry/instrumentation/werewolf/version.rb + create instrumentation/werewolf/test/test_helper.rb + create instrumentation/werewolf/test/opentelemetry/instrumentation/werewolf/instrumentation_test.rb + insert .toys/.data/releases.yml + insert instrumentation/all/Gemfile + insert instrumentation/all/opentelemetry-instrumentation-all.gemspec + insert instrumentation/all/lib/opentelemetry/instrumentation/all.rb + +``` + +## Implementation guidelines + +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: + +*. 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` + +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 +* Implement `present` block +* Implement `compatible` block and check for at least the minumum required gem version + +### 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. + +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: + +```ruby + + OpenTelemetry::Instrumentation::Werewolf::Instrumentation.instance.tracer.start_span('transform') do + # code to be traced + end + +``` + +> :warning: This tracer is not _upgradable_ before the SDK is initialized, therefore it is important that your instrumentation _always_ use stack local references of the tracer. + +### Use First Party Extension Points + +Whenever possible, use first party extension points (hooks) to instrument libraries. This ensures that the instrumentation is compatible with the latest versions of the library and that the instrumentation is maintained by the library authors. `ActiveSupport::Notifications` and `Middleware` are good examples of a first party extension points that are used by our instrumentation libraries. + +Monkey patching is discouraged in OpenTelemetry Ruby because it is the most common source of bugs and incompatability with the libraries we are instrumenting. If you must monkey patch, please ensure that the monkey patch is as isolated as possible and that it is clearly documented. + +### Use Semantic Conventions + +Use the OpenTelemetry Semantic Conventions to ensure that the instrumentation is compatible with other OpenTelemetry libraries and that the data is useful in a distributed context. + +> :information_source: Privacy and security are important considerations when adding attributes to spans. Please ensure that you are not adding sensitive information to spans. If you are unsure, please ask for a review. + +When semantic conventions do not exist, use the [Elastic Common Schema](https://www.elastic.co/guide/en/ecs/current/index.html) and submit a Issue/PR with your attributes to the [Semantic Conventions repo](https://github.com/open-telemetry/semantic-conventions) to propose a new set of standard attributes. + +If the attribute is specific to your instrumentation, then consider namespacing it using the `instrumentation` prefix e.g. `werewolf.bite.damage`. + +### Write Comprehensive Automated Tests + +Code that is not tested will not be accepted by maintainers. We understand that providing 100% test coverage is not always possible but we still ask that you provide your best effort when writing automated tests. + +Most of the libraries we are instrumenting introduce changes that are outside of our control. For this reason integration or state based tests are preferred over interaction (mock) tests. + +When you do in fact run into cases where test doubles or API stubs are absolutely necessary then we recommend using the `rspec-mocks` and `webmocks` gems. + +### Understand Performance Characteristics + +Instrumentation libraries should be as lightweight as possible + +* Avoid allocating too many objects and private methods +* 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 + +## Enable CI + +This project contains multiple CI workflows that execute tests and ensures the gems are installable. + +### Standalone Instrumentations + +For standalone instrumentations that do not have any external service dependencies, add the gem to the `/.github/workflows/ci-instrumentation.yml` file under `jobs/instrumentation/strategy/matrix/gem`: + +``` yaml + +jobs: + instrumentation: + strategy: + fail-fast: false + matrix: + gem: + - action_pack + - action_view + - active_job + # ... + - werewolf + os: + - ubuntu-latest + +``` + +#### JRuby Compatibility + +If your gem is incompatible with `JRuby`you can exclude them from the matrix by adding an entry to the `/.github/workflows/ci-instrumentation.yml` file under `jobs/instrumentation/steps/[name="JRuby Filter"]`: + +``` yaml + - name: "JRuby Filter" + id: jruby_skip + shell: bash + run: | + echo "skip=false" >> $GITHUB_OUTPUT + [[ "${{ matrix.gem }}" == "action_pack" ]] && echo "skip=true" >> $GITHUB_OUTPUT + # ... + [[ "${{ matrix.gem }}" == "werewolf" ]] && echo "skip=true" >> $GITHUB_OUTPUT + # This is essentially a bash script getting evaluated, so we need to return true or the whole job fails. + true +``` + +### External Service Instrumentations + +Adding jobs for instrumentations that have external service dependencies may be a bit more difficult if the job does not already have a similar service configured. + +#### Using Existing Services + +CI is currently configured to support the following services: + +* kafka +* memcached +* mongodb +* mysql +* postgresql +* rabbitmq +* redis + +If your gem depends on one of those services then great! The next step is to at the gem to matrix in the `/.github/workflows/ci-service-instrumentation.yml` file under `jobs/instrumentation_*/strategy/matrix/gem`: + +```yaml + + instrumentation_kafka: + strategy: + fail-fast: false + matrix: + gem: + - racecar + - rdkafka + - ruby_kafka + - werewolf + os: + - ubuntu-latest +``` + +#### Adding a New Service + +Assuming your external service is not supported, you may consider adding it as a new job in the `/.github/workflows/ci-service-instrumentation.yml` file, however we will accept new services on a case-by-case basis. + +Add the service container to `jobs/instrumentation_with_services/services` and add the gem to the matrix in `jobs/instrumentation_with_services/strategy/matrix/gem`: + +```yaml + + instrumentation_with_services: + strategy: + fail-fast: false + matrix: + gem: + - dalli + - mongo + - werewolf + os: + - ubuntu-latest + services: + # ... + my_service: + image: my_service:latest + # ... +``` + +> :information_source: Please refer to the official [GitHub Actions Documentation](https://docs.github.com/en/actions/using-containerized-services/about-service-containers) for more information on how to add a service container. + +If we determine the service container slows down the test suite significantly, it may make sense to copy the marix and steps stanzas from an existing instrumentation and update it to use the new service container as a dependency: + +```yaml + + instrumentation_silver: + strategy: + fail-fast: false + matrix: + gem: + - werewolf + os: + - ubuntu-latest + name: other / ${{ matrix.gem }} / ${{ matrix.os }} + runs-on: ${{ matrix.os }} + steps: + - uses: actions/checkout@v4 + - name: "Test Ruby 3.3" + uses: ./.github/actions/test_gem + with: + gem: "opentelemetry-instrumentation-${{ matrix.gem }}" + ruby: "3.3" + - name: "Test Ruby 3.2" + uses: ./.github/actions/test_gem + with: + gem: "opentelemetry-instrumentation-${{ matrix.gem }}" + ruby: "3.2" + - name: "Test Ruby 3.1" + uses: ./.github/actions/test_gem + with: + gem: "opentelemetry-instrumentation-${{ matrix.gem }}" + ruby: "3.1" + - name: "Test Ruby 3.0" + uses: ./.github/actions/test_gem + with: + gem: "opentelemetry-instrumentation-${{ matrix.gem }}" + ruby: "3.0" + yard: true + rubocop: true + build: true + - name: "Test JRuby" + uses: ./.github/actions/test_gem + with: + gem: "opentelemetry-instrumentation-${{ matrix.gem }}" + ruby: "jruby-9.4.2.0" + services: + # ... + my_service: + image: my_service:latest + # ... +``` + +## Documentation + +### README and Yardoc + +The `instrumentation_generator` will create a `README.md` file for your instrumentation. Please ensure that the `README` is up-to-date and contains the following: + +1. The span names, events, and semantic attributes that are emitted by the instrumentation +2. The configuration options that are available +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`. + +> :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: + +```ruby + +# frozen_string_literal: true + +require 'bundler/inline' + +gemfile(true) do + source 'https://rubygems.org' + gem 'grape', '~> 1.2' + gem 'opentelemetry-sdk' + gem 'opentelemetry-instrumentation-rack' + gem 'opentelemetry-instrumentation-grape' +end + +# Export traces to console +ENV['OTEL_TRACES_EXPORTER'] ||= 'console' + +OpenTelemetry::SDK.configure do |c| + c.service_name = 'trace_demonstration' + c.use_all # this will only require instrumentation gems it finds that are installed by bundler. +end + +at_exit do + OpenTelemetry.tracer_provider.shutdown +end + +# A basic Grape API example +class ExampleAPI < Grape::API + format :json + + desc 'Return a greeting message' + get :hello do + { message: 'Hello, world!' } + end + + desc 'Return information about a user' + params do + requires :id, type: Integer, desc: 'User ID' + end + get 'users/:id' do + { id: params[:id], name: 'John Doe', email: 'johndoe@example.com' } + end +end + +# Set up fake Rack application +builder = Rack::Builder.app do + # Integration is automatic in web frameworks but plain Rack applications require this line. + # Enable it in your config.ru. + use *OpenTelemetry::Instrumentation::Rack::Instrumentation.instance.middleware_args + run ExampleAPI +end +app = Rack::MockRequest.new(builder) + +app.get('/hello') +app.get('/users/1') + +``` + +## Submit a pull request + +You are encouraged to submit a `draft` pull request early in the development process to get feedback from the maintainers, run your tests via CI, and ensure that your changes are in line with the project's goals. + +The `CODEOWNERS` is used to notify the instrumentation authors a pull request is opened. Please add yourself to the `instrumentation` section of the `CODEOWNERS` file, e.g. + +```plaintext + +instrumentation/werewolf/ @lycanthrope @open-telemetry/ruby-contrib-maintainers @open-telemetry/ruby-contrib-approvers + +``` + +> :information_source: In order for you to receive a request to review PRs, you must be a member of the [`open-telemetry/community`](https://github.com/open-telemetry/community/blob/5db097b38ce930fb1ff3eb79a1625bae46136894/community-membership.md#community-membership). Please consider applying for membership if you are not already a member. + +The [CONTRIBUTING.md](../CONTRIBUTING.md) guide has the remaining steps to get your contribution reviewed, merged, and released. diff --git a/instrumentation/README.md b/instrumentation/README.md index a20b8b1f9..770e1a70d 100644 --- a/instrumentation/README.md +++ b/instrumentation/README.md @@ -10,7 +10,6 @@ Released instrumentations can be found at the [OpenTelemetry registry](https://o In-progress instrumentations can be found [here](https://github.com/open-telemetry/opentelemetry-ruby-contrib/issues?q=is%3Aopen+label%3Ainstrumentation+-label%3A%22help+wanted%22+). - ## How do I get started? ### Individual instrumentation libraries @@ -19,16 +18,20 @@ To get started with a single instrumentation library, for example `opentelemetry ### 1. Install the gem -``` +```console + gem install opentelemetry-instrumentation-rack + ``` ### 2. Configure OpenTelemetry to use the instrumentation -``` +```console + OpenTelemetry::SDK.configure do |c| c.use 'OpenTelemetry::Instrumentation::Rack' end + ``` Instrumentation-specific documentation can be found in each subdirectory's `README.md`. diff --git a/instrumentation/datadog-porting-guide.md b/instrumentation/datadog-porting-guide.md index dc9c63c75..1ccadefd5 100644 --- a/instrumentation/datadog-porting-guide.md +++ b/instrumentation/datadog-porting-guide.md @@ -16,11 +16,15 @@ Aid developers who wish to port existing datadog (dd-trace-rb) instrumentation t * Add Gemfile, opentelemetry-instrumentation-#{name}.gemspec * Add runnable (docker) example (using its own Gemfile) -``` + +```console + $ docker-compose run ex-instrumentation-myinstrumentation bundle install $ docker-compose run ex-instrumentation-myinstrumentation bash-5.0$ ruby trace_demonstration.rb + ``` + * Rakefile * `tests/test_helper.rb` * Integrate rubocop (see https://github.com/open-telemetry/opentelemetry-ruby/pull/172#pullrequestreview-349183775) @@ -66,7 +70,7 @@ bash-5.0$ ruby trace_demonstration.rb ### Runtime performance considerations Watch for "low hanging fruit" performance improvements including: -* reduce object allocations - move to a constant, or cache values that would be generated repeatedly +* reduce object allocations - move to a constant, or cache values that would be generated repeatedly * look for "easy wins" vs. "complete redesigns" ## Testing diff --git a/instrumentation/grape/example/trace_demonstration.rb b/instrumentation/grape/example/trace_demonstration.rb index ca93f5eea..7f67a93d1 100644 --- a/instrumentation/grape/example/trace_demonstration.rb +++ b/instrumentation/grape/example/trace_demonstration.rb @@ -4,20 +4,22 @@ gemfile(true) do source 'https://rubygems.org' - gem 'opentelemetry-api' - gem 'opentelemetry-instrumentation-grape' + gem 'grape', '~> 1.2' gem 'opentelemetry-sdk' - gem 'grape' + gem 'opentelemetry-instrumentation-rack' + gem 'opentelemetry-instrumentation-grape' end -require 'opentelemetry-instrumentation-rack' - # Export traces to console ENV['OTEL_TRACES_EXPORTER'] ||= 'console' OpenTelemetry::SDK.configure do |c| c.service_name = 'trace_demonstration' - c.use_all # this will only require instrumentation gems it finds that are installed by bundler. + c.use_all # this will only require instrumentation gems it finds that are installed by bundler. +end + +at_exit do + OpenTelemetry.tracer_provider.shutdown end # A basic Grape API example @@ -30,9 +32,6 @@ class ExampleAPI < Grape::API end desc 'Return information about a user' - # Filters - before { sleep(0.01) } - after { sleep(0.01) } params do requires :id, type: Integer, desc: 'User ID' end @@ -45,7 +44,7 @@ class ExampleAPI < Grape::API builder = Rack::Builder.app do # Integration is automatic in web frameworks but plain Rack applications require this line. # Enable it in your config.ru. - use OpenTelemetry::Instrumentation::Rack::Middlewares::TracerMiddleware + use *OpenTelemetry::Instrumentation::Rack::Instrumentation.instance.middleware_args run ExampleAPI end app = Rack::MockRequest.new(builder) From 60aba5ef67c1cb7b40a4c3e2981888cb7676ef6c Mon Sep 17 00:00:00 2001 From: Ariel Valentin Date: Mon, 29 Apr 2024 13:00:12 -0500 Subject: [PATCH 02/17] Update CONTRIBUTING.md Co-authored-by: Marco Costa --- instrumentation/CONTRIBUTING.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/CONTRIBUTING.md b/instrumentation/CONTRIBUTING.md index a3fe3df27..b7abe54e3 100644 --- a/instrumentation/CONTRIBUTING.md +++ b/instrumentation/CONTRIBUTING.md @@ -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 From 827aacb2e5088aaa8a93ce01a0cd58e9fa537ee1 Mon Sep 17 00:00:00 2001 From: Ariel Valentin Date: Mon, 29 Apr 2024 13:00:23 -0500 Subject: [PATCH 03/17] Update CONTRIBUTING.md Co-authored-by: Marco Costa --- instrumentation/CONTRIBUTING.md | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/instrumentation/CONTRIBUTING.md b/instrumentation/CONTRIBUTING.md index b7abe54e3..ac56aa9ce 100644 --- a/instrumentation/CONTRIBUTING.md +++ b/instrumentation/CONTRIBUTING.md @@ -92,12 +92,12 @@ 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: -*. 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` +* Use the OpenTelemetry API +* Use First Party Extension Points +* Use Semantic Conventions +* Write comprehensive automated tests +* Understand Performance Characteristics ### Use `OpenTelemetry::Instrumentation::Base` From ec33c6e165af386db0379c8d51542e08f5eae455 Mon Sep 17 00:00:00 2001 From: Ariel Valentin Date: Mon, 29 Apr 2024 13:00:46 -0500 Subject: [PATCH 04/17] Update CONTRIBUTING.md Co-authored-by: Marco Costa --- instrumentation/CONTRIBUTING.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/CONTRIBUTING.md b/instrumentation/CONTRIBUTING.md index ac56aa9ce..b9f661d96 100644 --- a/instrumentation/CONTRIBUTING.md +++ b/instrumentation/CONTRIBUTING.md @@ -111,7 +111,7 @@ The entry point of your instrumentation should be implemented as a subclass of ` 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. -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: From 1a9f754556b7cfaea209470286ef8d8750fff6a6 Mon Sep 17 00:00:00 2001 From: Ariel Valentin Date: Mon, 29 Apr 2024 13:03:44 -0500 Subject: [PATCH 05/17] Update CONTRIBUTING.md Co-authored-by: Marco Costa --- instrumentation/CONTRIBUTING.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/CONTRIBUTING.md b/instrumentation/CONTRIBUTING.md index b9f661d96..7ce8ccf0f 100644 --- a/instrumentation/CONTRIBUTING.md +++ b/instrumentation/CONTRIBUTING.md @@ -322,7 +322,7 @@ 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. From 5a0633307a763fab507208466f82e8597bc3d90b Mon Sep 17 00:00:00 2001 From: Ariel Valentin Date: Mon, 29 Apr 2024 13:03:53 -0500 Subject: [PATCH 06/17] Update CONTRIBUTING.md Co-authored-by: Marco Costa --- instrumentation/CONTRIBUTING.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/CONTRIBUTING.md b/instrumentation/CONTRIBUTING.md index 7ce8ccf0f..d12b46da1 100644 --- a/instrumentation/CONTRIBUTING.md +++ b/instrumentation/CONTRIBUTING.md @@ -330,7 +330,7 @@ In addition to that, there should also be redundant `yardoc` comments in the ent 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 From 7285a7d8226086503e23f1f6b0d14ed79b2d5628 Mon Sep 17 00:00:00 2001 From: Ariel Valentin Date: Mon, 29 Apr 2024 16:57:38 -0500 Subject: [PATCH 07/17] squash: Feedback about using private methods --- instrumentation/CONTRIBUTING.md | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/instrumentation/CONTRIBUTING.md b/instrumentation/CONTRIBUTING.md index d12b46da1..15f7b3724 100644 --- a/instrumentation/CONTRIBUTING.md +++ b/instrumentation/CONTRIBUTING.md @@ -153,10 +153,24 @@ When you do in fact run into cases where test doubles or API stubs are absolutel Instrumentation libraries should be as lightweight as possible -* Avoid allocating too many objects and private methods +* Avoid allocating objects unless absolutely necessary * 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 +#### Minimal Solutions are Better + +Instrumentations should have both the minimal amount of code necessary to provide useful insights to our users. + +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 that may result in unexpected behavior and bugs. + +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. + ## Enable CI This project contains multiple CI workflows that execute tests and ensures the gems are installable. From c344b9d5beda87a5a1725bf967f9749f4052f852 Mon Sep 17 00:00:00 2001 From: Ariel Valentin Date: Mon, 29 Apr 2024 17:15:06 -0500 Subject: [PATCH 08/17] squash: add example for base DSL --- instrumentation/CONTRIBUTING.md | 47 +++++++++++++++++++++++++++++++-- 1 file changed, 45 insertions(+), 2 deletions(-) diff --git a/instrumentation/CONTRIBUTING.md b/instrumentation/CONTRIBUTING.md index 15f7b3724..35d03d8ed 100644 --- a/instrumentation/CONTRIBUTING.md +++ b/instrumentation/CONTRIBUTING.md @@ -104,8 +104,51 @@ The original design and implementation of this project was heavily influenced by 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 -* 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 +* Implement `compatible` block and check for at least the minumum required library version +* Any custom `options` you want to support + +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 + 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. +* The `compatible` block checks if the `Werewolf` library version is at least `0.1.0` and will skip it if it is not. +* 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. ### Use the OpenTelemetry API From eca1f617170b8d2fd40494068d000be04adb0bbb Mon Sep 17 00:00:00 2001 From: Ariel Valentin Date: Mon, 29 Apr 2024 17:27:43 -0500 Subject: [PATCH 09/17] squash: more thoughts on options and custom code --- instrumentation/CONTRIBUTING.md | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/instrumentation/CONTRIBUTING.md b/instrumentation/CONTRIBUTING.md index 35d03d8ed..aca1112d9 100644 --- a/instrumentation/CONTRIBUTING.md +++ b/instrumentation/CONTRIBUTING.md @@ -194,11 +194,12 @@ When you do in fact run into cases where test doubles or API stubs are absolutel ### Understand Performance Characteristics -Instrumentation libraries should be as lightweight as possible +Instrumentation libraries should be as lightweight as possible and must: * Avoid allocating objects unless absolutely necessary * 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 @@ -208,12 +209,20 @@ It may sound contrary to good engineering practices, but you should avoid adding 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 that may result in unexpected behavior and bugs. +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. 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. +#### Avoid Adding Custom Extensions (E_TOOMANYOPTIONS) + +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. + +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. + ## Enable CI This project contains multiple CI workflows that execute tests and ensures the gems are installable. From f688bb063bed68ec81b00b875c04e0b707bc4982 Mon Sep 17 00:00:00 2001 From: Ariel Valentin Date: Tue, 30 Apr 2024 12:07:57 -0500 Subject: [PATCH 10/17] squash: Update instrumentation/CONTRIBUTING.md Co-authored-by: Marco Costa --- instrumentation/CONTRIBUTING.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/CONTRIBUTING.md b/instrumentation/CONTRIBUTING.md index aca1112d9..28c61b4cd 100644 --- a/instrumentation/CONTRIBUTING.md +++ b/instrumentation/CONTRIBUTING.md @@ -213,7 +213,7 @@ In cases when code uses monkey patching, it runs the risk of _adding_ methods th 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. +In the near future, [OTel Profiling](https://opentelemetry.io/blog/2024/profiling/) will provide users an even deeper understanding of what is happening in their applications at a more granular level. #### Avoid Adding Custom Extensions (E_TOOMANYOPTIONS) From 898279f10f1c136efe210584986fe740a0981c8b Mon Sep 17 00:00:00 2001 From: Ariel Valentin Date: Tue, 30 Apr 2024 12:17:11 -0500 Subject: [PATCH 11/17] squash: Update instrumentation/CONTRIBUTING.md --- instrumentation/CONTRIBUTING.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/CONTRIBUTING.md b/instrumentation/CONTRIBUTING.md index 28c61b4cd..117f8354d 100644 --- a/instrumentation/CONTRIBUTING.md +++ b/instrumentation/CONTRIBUTING.md @@ -215,7 +215,7 @@ Avoid instrumenting every method in a library, instead focus on the methods the In the near future, [OTel Profiling](https://opentelemetry.io/blog/2024/profiling/) will provide users an even deeper understanding of what is happening in their applications at a more granular level. -#### Avoid Adding Custom Extensions (E_TOOMANYOPTIONS) +#### Avoid Adding Custom Extensions 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. From eb26f9c9f5cc582c73cfa69d922756797c5269cb Mon Sep 17 00:00:00 2001 From: Ariel Valentin Date: Tue, 30 Apr 2024 12:18:13 -0500 Subject: [PATCH 12/17] squash: Update instrumentation/CONTRIBUTING.md --- instrumentation/CONTRIBUTING.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/CONTRIBUTING.md b/instrumentation/CONTRIBUTING.md index 117f8354d..a42a01996 100644 --- a/instrumentation/CONTRIBUTING.md +++ b/instrumentation/CONTRIBUTING.md @@ -203,7 +203,7 @@ Instrumentation libraries should be as lightweight as possible and must: #### Minimal Solutions are Better -Instrumentations should have both the minimal amount of code necessary to provide useful insights to our users. +Instrumentations should have the minimal amount of code necessary to provide useful insights to our users. 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. From 9e591020871abba0ede8dcbe500d82f3ac1ef56b Mon Sep 17 00:00:00 2001 From: Ariel Valentin Date: Wed, 1 May 2024 10:06:58 -0500 Subject: [PATCH 13/17] squash: add perf examples --- instrumentation/CONTRIBUTING.md | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/instrumentation/CONTRIBUTING.md b/instrumentation/CONTRIBUTING.md index a42a01996..59c952a09 100644 --- a/instrumentation/CONTRIBUTING.md +++ b/instrumentation/CONTRIBUTING.md @@ -194,27 +194,35 @@ When you do in fact run into cases where test doubles or API stubs are absolutel ### Understand Performance Characteristics +The OTel Specification describes expectations around the performance of SDKs, which you must review and apply to instrumentations: + Instrumentation libraries should be as lightweight as possible and must: -* Avoid allocating objects unless absolutely necessary * 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 +#### Provide minimal solutions and code paths -Instrumentations should have the minimal amount of code necessary to provide useful insights to our users. +Instrumentations should have the minimal amount of code necessary to provide useful insights to our users. It may sound contrary to good engineering practices, but you must avoid adding lots of small methods, classes, and objects when instrumenting a library. -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. +Though often easier to maintain and reason about; small and well factored code adds overhead to the library you are instrumenting resulting in performance degradation due to unnecessary object allocations, method dispatching, and other performance overhead. -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. +It will also contribute to building large backtraces that makes it more difficult for our end users to understand the essential parts of exception reports application. That will in turn likely result in additional filtering logic in the their application to avoid reporting unnecessary stack frames. 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. -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. +Avoid instrumenting _every_ method in a library and 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. The use case for this type of low level granularity fails under the purview of profiling. In the near future, [OTel Profiling](https://opentelemetry.io/blog/2024/profiling/) will provide users an even deeper understanding of what is happening in their applications at a more granular level. +Here are some examples of performance fixes that reduced object allocations and method dispatching: + +* +* +* +* + #### Avoid Adding Custom Extensions 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. From 047059e9ef8b81e58b1ca86c88f1f8183fa91027 Mon Sep 17 00:00:00 2001 From: Ariel Valentin Date: Wed, 1 May 2024 10:15:35 -0500 Subject: [PATCH 14/17] squash: another example --- instrumentation/CONTRIBUTING.md | 1 + 1 file changed, 1 insertion(+) diff --git a/instrumentation/CONTRIBUTING.md b/instrumentation/CONTRIBUTING.md index 59c952a09..8f43b7e0a 100644 --- a/instrumentation/CONTRIBUTING.md +++ b/instrumentation/CONTRIBUTING.md @@ -222,6 +222,7 @@ Here are some examples of performance fixes that reduced object allocations and * * * +* #### Avoid Adding Custom Extensions From 557193c6e84483f4f4c20469b359a600a738302d Mon Sep 17 00:00:00 2001 From: Ariel Valentin Date: Wed, 1 May 2024 10:16:08 -0500 Subject: [PATCH 15/17] squash: event more --- instrumentation/CONTRIBUTING.md | 1 + 1 file changed, 1 insertion(+) diff --git a/instrumentation/CONTRIBUTING.md b/instrumentation/CONTRIBUTING.md index 8f43b7e0a..f7dd23fa9 100644 --- a/instrumentation/CONTRIBUTING.md +++ b/instrumentation/CONTRIBUTING.md @@ -221,6 +221,7 @@ Here are some examples of performance fixes that reduced object allocations and * * * +* * * From faf117798bfb049e446a7d588d1531e4f3d47b02 Mon Sep 17 00:00:00 2001 From: Ariel Valentin Date: Sat, 11 May 2024 10:14:14 -0500 Subject: [PATCH 16/17] docs: Apply suggestions from code review Co-authored-by: Kayla Reopelle (she/her) <87386821+kaylareopelle@users.noreply.github.com> --- CONTRIBUTING.md | 8 +-- instrumentation/CONTRIBUTING.md | 109 ++++++++++++++++---------------- 2 files changed, 60 insertions(+), 57 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 05ad92d5b..b885db0ae 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -166,11 +166,11 @@ bundle install bundle exec rake yard ``` -## Instrumentation Authors Guide +## Instrumentation author's guide -Please make sure that you review the [Instrumentation Authors Guide](.instrumentation/CONTRIBUTING.md) before submitting a new instrumentation. +Please make sure that you review the [Instrumentation author's guide](.instrumentation/CONTRIBUTING.md) before submitting a new instrumentation. -## Create a Pull Request +## Create a pull request You'll need to create a Pull Request once you've finished your work. The [Kubernetes GitHub Workflow][kube-github-workflow-pr] document has @@ -349,7 +349,7 @@ For releases to succeed, new gems MUST include the following: * A `CHANGELOG.md` file. * A `yard` rake task. -## Dependabot Updates +## Dependabot updates This repository uses [Dependabot](https://dependabot.com/) to keep dependencies up to date, however there shared development dependencies are often scattered across multiple gems. Dependabot does not currently support the ability to group dependencies for gems in multiple subdirectories, so we use a custom script to bulk update dependencies across all gems. diff --git a/instrumentation/CONTRIBUTING.md b/instrumentation/CONTRIBUTING.md index f7dd23fa9..1f63ac94f 100644 --- a/instrumentation/CONTRIBUTING.md +++ b/instrumentation/CONTRIBUTING.md @@ -1,4 +1,4 @@ -# Instrumentation Authors Guide +# Instrumentation author's guide This guide is for authors of OpenTelemetry Ruby instrumentation libraries. It provides guidance on how to contribute an instrumentation library. @@ -8,11 +8,11 @@ Please make sure to read and understand the [CONTRIBUTING](../CONTRIBUTING.md) g We are a community of volunteers with a shared goal of improving observability in Ruby applications. -We welcome contributions from everyone, and we want to make sure that you have a great experience contributing to this project, as well as a positive impact on the Ruby community. +We welcome contributions from everyone. We want to make sure that you have a great experience contributing to this project, as well as a positive impact on the Ruby community. We have limited capacity to maintain instrumentation libraries, so we ask that you commit to maintaining the instrumentation library you contribute. -In addition to the requirements to maintain at least community member status, contributing an instrumentation to this project requires the following: +In addition to the requirements to maintain at least [community member status](https://github.com/open-telemetry/community/blob/main/community-membership.md), contributing an instrumentation to this project requires the following: 1. Responding to issues and pull requests 2. Performing timely code reviews and responding to issues @@ -22,28 +22,28 @@ In addition to the requirements to maintain at least community member status, co * Ruby language changes * Instrumented library changes -If you do not have the capacity to maintain the instrumentation library, please consider contributing to the OpenTelemetry Ruby project in other ways, or consider creating a separate project for the instrumentation library. +If you do not have the capacity to maintain the instrumentation library, please consider contributing to the OpenTelemetry Ruby project in other ways or consider creating a separate project for the instrumentation library. > :warning: Libraries that do not meet these requirements may be removed from the project at any time at the discretion of OpenTelemetry Ruby Contrib Maintainers. -## Contributing a New Instrumentation Library +## Contributing a new instrumentation library -Our long-term goal is to provide instrumentation for all popular Ruby libraries. Ideally we would like to have first-party instrumentation for all libraries maintained by the Gem authors to ensure compatability with upstream gems, however in many cases this is not possible. +Our long-term goal is to provide instrumentation for all popular Ruby libraries. Ideally, we would like to have first-party instrumentation for all libraries maintained by the gem's authors to ensure compatibility with upstream gems. However, in many cases this is not possible. -For this reason we welcome contributions of new instrumentation libraries that cannot be maintained by the original gem authors as first party instrumentations. +For this reason, we welcome contributions of new instrumentation libraries that cannot be maintained by the original gem authors as first-party instrumentation. The following steps are required to contribute a new instrumentation library: 1. Generate an instrumentation gem skeleton -2. Implement the instrumentation library including comprehensive automated tests -3. Add the instrumentation library to the appropriate CI workflow +2. Implement the instrumentation library, including comprehensive automated tests +3. Add the instrumentation library to the appropriate CI workflows 4. Include documentation for your instrumentation - * Document all instrumentation-specific configuration options in the `README.md` and `yardoc` comments + * Document all instrumentation-specific configuration options in the `README.md` and `yardoc` class comments * Document all semantic conventions used by the instrumentation in the `README.md` * Provide executable examples in an `examples` directory 5. Submit a pull request -## Generate the Gem +## Generate the gem This repository contains a script to generate a new instrumentation library. @@ -90,23 +90,24 @@ The output of the generator shows that it creates a new directory in the `instru ## Implementation guidelines -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: +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: * Use `OpenTelemetry::Instrumentation::Base` * Use the OpenTelemetry API -* Use First Party Extension Points +* Use first-party extension points * Use Semantic Conventions * Write comprehensive automated tests -* Understand Performance Characteristics +* Understand performance characteristics ### 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 -* Implement `present` block, which is you check if the library you are instrumenting was loaded -* Implement `compatible` block and check for at least the minumum required library version -* Any custom `options` you want to support +* Implement an `install` block, where all of the integration work happens +* Implement a `present` block, which checks whether the library you are instrumenting was loaded +* Implement a `compatible` block and check for at least the minimum required library version + * OpenTelemetry Ruby Contrib generally supports only versions of gems that are within the maintenance window +* Add any custom configuration `options` you want to support The example below demonstrates how to implement the `Werewolf` instrumentation: @@ -146,17 +147,17 @@ 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. -* The `compatible` block checks if the `Werewolf` library version is at least `0.1.0` and will skip it if it is not. -* 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. +* 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 initialized. +* The `compatible` block checks if the `Werewolf` library version is at least `0.1.0` and will skip installation if it is not. +* The `options` section allows you to define custom configuration 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. ### 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. +Instrumentations are intended to be portable and usable with vendor distributions of the SDK. For this reason, you must use the [OpenTelemetry API](https://github.com/open-telemetry/opentelemetry-ruby/tree/main/api) to create spans and add attributes, events, and links to spans and avoid using the [OpenTelemetry SDK](https://github.com/open-telemetry/opentelemetry-ruby/tree/main/sdk) directly. 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: +For example, the `Werewolf` module generated in the example above is available via `OpenTelemetry::Instrumentation::Werewolf::Instrumentation.instance.tracer`. You should reference this tracer in your code when creating spans like this: ```ruby @@ -168,33 +169,33 @@ For example, the `Werewolf` generated in the example above instrumentation is av > :warning: This tracer is not _upgradable_ before the SDK is initialized, therefore it is important that your instrumentation _always_ use stack local references of the tracer. -### Use First Party Extension Points +### Use first-party extension points Whenever possible, use first party extension points (hooks) to instrument libraries. This ensures that the instrumentation is compatible with the latest versions of the library and that the instrumentation is maintained by the library authors. `ActiveSupport::Notifications` and `Middleware` are good examples of a first party extension points that are used by our instrumentation libraries. -Monkey patching is discouraged in OpenTelemetry Ruby because it is the most common source of bugs and incompatability with the libraries we are instrumenting. If you must monkey patch, please ensure that the monkey patch is as isolated as possible and that it is clearly documented. +Monkey patching is discouraged in OpenTelemetry Ruby because it is the most common source of bugs and incompatability with the libraries we instrument. If you must monkey patch, please ensure that the monkey patch is as isolated as possible and that it is clearly documented. ### Use Semantic Conventions -Use the OpenTelemetry Semantic Conventions to ensure that the instrumentation is compatible with other OpenTelemetry libraries and that the data is useful in a distributed context. +Use the [OpenTelemetry Semantic Conventions](https://opentelemetry.io/docs/concepts/semantic-conventions/) to ensure the instrumentation is compatible with other OpenTelemetry libraries and that the data is useful in a distributed context. > :information_source: Privacy and security are important considerations when adding attributes to spans. Please ensure that you are not adding sensitive information to spans. If you are unsure, please ask for a review. -When semantic conventions do not exist, use the [Elastic Common Schema](https://www.elastic.co/guide/en/ecs/current/index.html) and submit a Issue/PR with your attributes to the [Semantic Conventions repo](https://github.com/open-telemetry/semantic-conventions) to propose a new set of standard attributes. +When semantic conventions do not exist, use the [Elastic Common Schema](https://www.elastic.co/guide/en/ecs/current/index.html) and submit an Issue/PR with your attributes to the [Semantic Conventions repo](https://github.com/open-telemetry/semantic-conventions) to propose a new set of standard attributes. -If the attribute is specific to your instrumentation, then consider namespacing it using the `instrumentation` prefix e.g. `werewolf.bite.damage`. +If the attribute is specific to your instrumentation, then consider namespacing it using the `instrumentation` prefix e.g. `werewolf.bite.damage` and calling it out in the instrumentation README. -### Write Comprehensive Automated Tests +### Write comprehensive automated tests Code that is not tested will not be accepted by maintainers. We understand that providing 100% test coverage is not always possible but we still ask that you provide your best effort when writing automated tests. -Most of the libraries we are instrumenting introduce changes that are outside of our control. For this reason integration or state based tests are preferred over interaction (mock) tests. +Most of the libraries instrument introduce changes outside of our control. For this reason, integration or state-based tests are preferred over interaction (mock) tests. -When you do in fact run into cases where test doubles or API stubs are absolutely necessary then we recommend using the `rspec-mocks` and `webmocks` gems. +When you do in fact run into cases where test doubles or API stubs are absolutely necessary, we recommend using the [`rspec-mocks`](https://github.com/rspec/rspec-mocks) and [`webmocks`](https://github.com/bblimke/webmock) gems. -### Understand Performance Characteristics +### Understand performance characteristics -The OTel Specification describes expectations around the performance of SDKs, which you must review and apply to instrumentations: +The OTel Specification describes expectations around the performance of SDKs, which you must review and apply to instrumentation: Instrumentation libraries should be as lightweight as possible and must: @@ -204,15 +205,15 @@ Instrumentation libraries should be as lightweight as possible and must: #### Provide minimal solutions and code paths -Instrumentations should have the minimal amount of code necessary to provide useful insights to our users. It may sound contrary to good engineering practices, but you must avoid adding lots of small methods, classes, and objects when instrumenting a library. +Instrumentation should have the minimal amount of code necessary to provide useful insights to our users. It may sound contrary to good engineering practices, but you must avoid adding lots of small methods, classes, and objects when instrumenting a library. -Though often easier to maintain and reason about; small and well factored code adds overhead to the library you are instrumenting resulting in performance degradation due to unnecessary object allocations, method dispatching, and other performance overhead. +Though often easier to maintain and reason about; small and well-factored code adds overhead to the library you're instrumenting, resulting in performance degradation due to unnecessary object allocations, method dispatching, and other performance overhead. -It will also contribute to building large backtraces that makes it more difficult for our end users to understand the essential parts of exception reports application. That will in turn likely result in additional filtering logic in the their application to avoid reporting unnecessary stack frames. +It also contributes to building large backtraces, making it more difficult for our end users to understand the essential parts of exception reports. That will likely result in additional filtering logic in their application to avoid reporting unnecessary stack frames. -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. +In cases when code uses monkey patching, it runs the risk of _adding_ methods that conflict with the internal implementation of the library and may result in unexpected behavior and bugs. -Avoid instrumenting _every_ method in a library and 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. The use case for this type of low level granularity fails under the purview of profiling. +Avoid instrumenting _every_ method in a library and instead focus on the methods that provide the _most_ insights into what typically causes performance problems for applications, e.g. I/O and network calls. The use case for this type of low-level granularity falls under the purview of profiling. In the near future, [OTel Profiling](https://opentelemetry.io/blog/2024/profiling/) will provide users an even deeper understanding of what is happening in their applications at a more granular level. @@ -225,21 +226,21 @@ Here are some examples of performance fixes that reduced object allocations and * * -#### Avoid Adding Custom Extensions +#### Avoid adding custom extensions 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. +You should _avoid_ adding options that allow custom code blocks (`type: :callable`) 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. -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. +You should steer users towards post-processing as part of the [OTel Collector](https://opentelemetry.io/docs/collector/), which has a richer and more powerful toolset, and executes out of the application's critical code path. ## Enable CI -This project contains multiple CI workflows that execute tests and ensures the gems are installable. +This project contains multiple CI workflows that execute tests and ensure the gems are installable. -### Standalone Instrumentations +### Standalone instrumentation -For standalone instrumentations that do not have any external service dependencies, add the gem to the `/.github/workflows/ci-instrumentation.yml` file under `jobs/instrumentation/strategy/matrix/gem`: +For standalone instrumentation that does not have any external service dependencies, add the gem to the `/.github/workflows/ci-instrumentation.yml` file under `jobs/instrumentation/strategy/matrix/gem`: ``` yaml @@ -261,7 +262,7 @@ jobs: #### JRuby Compatibility -If your gem is incompatible with `JRuby`you can exclude them from the matrix by adding an entry to the `/.github/workflows/ci-instrumentation.yml` file under `jobs/instrumentation/steps/[name="JRuby Filter"]`: +If your gem is incompatible with `JRuby`, you can exclude it from the matrix by adding an entry to the `/.github/workflows/ci-instrumentation.yml` file under `jobs/instrumentation/steps/[name="JRuby Filter"]`: ``` yaml - name: "JRuby Filter" @@ -276,9 +277,9 @@ If your gem is incompatible with `JRuby`you can exclude them from the matrix by true ``` -### External Service Instrumentations +### External service instrumentations -Adding jobs for instrumentations that have external service dependencies may be a bit more difficult if the job does not already have a similar service configured. +Adding jobs for instrumentation with external service dependencies may be a bit more difficult if the job does not already have a similar service configured. #### Using Existing Services @@ -292,7 +293,7 @@ CI is currently configured to support the following services: * rabbitmq * redis -If your gem depends on one of those services then great! The next step is to at the gem to matrix in the `/.github/workflows/ci-service-instrumentation.yml` file under `jobs/instrumentation_*/strategy/matrix/gem`: +If your gem depends on one of those services, then great! The next step is to add the gem to matrix in the `/.github/workflows/ci-service-instrumentation.yml` file under `jobs/instrumentation_*/strategy/matrix/gem`: ```yaml @@ -391,16 +392,18 @@ If we determine the service container slows down the test suite significantly, i ### README and Yardoc -The `instrumentation_generator` will create a `README.md` file for your instrumentation. Please ensure that the `README` is up-to-date and contains the following: +The `instrumentation_generator` creates a `README.md` file for your instrumentation. Please ensure that the `README` is up-to-date and contains the following: -1. The span names, events, and semantic attributes that are emitted by the instrumentation -2. The configuration options that are available +1. The span names, events, and semantic attributes emitted by the instrumentation +2. The configuration options available 3. Any known limitations or caveats 4. The minimum supported gem version +> :information_source: See the `ActiveJob` instrumentation [`README`](./active_job/README.md) for a comprehensive example. + 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. +> :information_source: See the `Sidekiq::Instrumentation` [class description](./sidekiq/lib/opentelemetry/instrumentation/sidekiq/instrumentation.rb) for a comprehensive example. ### Examples @@ -470,7 +473,7 @@ app.get('/users/1') You are encouraged to submit a `draft` pull request early in the development process to get feedback from the maintainers, run your tests via CI, and ensure that your changes are in line with the project's goals. -The `CODEOWNERS` is used to notify the instrumentation authors a pull request is opened. Please add yourself to the `instrumentation` section of the `CODEOWNERS` file, e.g. +The `CODEOWNERS` is used to notify instrumentation authors when a pull request is opened. Please add yourself to the `instrumentation` section of the `CODEOWNERS` file, e.g. ```plaintext From 756353dbb94e9f70a668c224f88fff269a89765e Mon Sep 17 00:00:00 2001 From: Ariel Valentin Date: Mon, 13 May 2024 15:05:42 -0500 Subject: [PATCH 17/17] squash: Update instrumentation/CONTRIBUTING.md Co-authored-by: Kayla Reopelle (she/her) <87386821+kaylareopelle@users.noreply.github.com> --- instrumentation/CONTRIBUTING.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/CONTRIBUTING.md b/instrumentation/CONTRIBUTING.md index 1f63ac94f..ff45dcbbb 100644 --- a/instrumentation/CONTRIBUTING.md +++ b/instrumentation/CONTRIBUTING.md @@ -171,7 +171,7 @@ For example, the `Werewolf` module generated in the example above is available v ### Use first-party extension points -Whenever possible, use first party extension points (hooks) to instrument libraries. This ensures that the instrumentation is compatible with the latest versions of the library and that the instrumentation is maintained by the library authors. `ActiveSupport::Notifications` and `Middleware` are good examples of a first party extension points that are used by our instrumentation libraries. +Whenever possible, use first-party extension points (hooks) to instrument libraries. This ensures that the instrumentation is compatible with the latest versions of the library and that the instrumentation is maintained by the library authors. [`ActiveSupport::Notifications`](https://guides.rubyonrails.org/active_support_instrumentation.html) and `Middleware` are good examples of first-party extension points used by our instrumentation libraries. Monkey patching is discouraged in OpenTelemetry Ruby because it is the most common source of bugs and incompatability with the libraries we instrument. If you must monkey patch, please ensure that the monkey patch is as isolated as possible and that it is clearly documented.