Add pessimistic version constraint to instrumentation and overnight Canary Build

[ericmustin]

spawning off some discussion here open-telemetry/opentelemetry-ruby#947, there are two areas that ought to be addressed in instrumentation

1. Be more defensive with version compatibility
2. Have a way to safely test against latest versions

Currently our instrumentation has the concept of a compatible block, which is just an arbitrary proc an instrumentation author can use to define logic for whether the instrumentation is compatible with the specific Gem version that has been installed. It could also be used to check arbitrary things like, for example, whether it's a jruby environment, or whether specific permissions are available, etc.

In practice, afaik, this is only used to do a MIN_VERSION comparison, like in action_pack, for example.

Over time, as new major versions come out, it becomes harder for us to say with confidence that our instrumentation is "compatible" with them, and so it might be worthwhile to make a pass through the instrumentation and add a MAX_VERSION comparison as well. In between the outcomes of "This instrumentation unexpectedly doesn't install and logs a warning to the user" and "This instrumentation causes an application crash during runtime", the former seems like a more reasonable posture for us to take.

With that posture we would also need to have some sort of mechanism for reliably testing latest major version releases. One solution here could be to include a latest or canary nightly build in CI, that pulls in the latest major+minor versions of everything , runs the test suite, and informs us of what fails. In this way we could establish some confidence when we want to bump the MAX_VERSION compatibility, or have some context to point to if contributors/users open issues asking for a major version bump.

lastly, as @arielvalentin suggests in this issue comment, open-telemetry/opentelemetry-ruby#947 (comment) , a DSL for declaring min/max version may make it easier to maintain this approach long term. Perhaps some semantic conventions around what constants to use for MIN_VERSION and MAX_VERSION combined with a modification to the base compatible() method could accomplish this (and also make it easier for a canary build to inform us when the latest gem version pulled down has a delta with the instrumentation's MAX version).

[arielvalentin]

@robertlaurin Following up on our conversation from last week re: open-telemetry/opentelemetry-ruby#1025.

Please correct me if I am misrepresenting your concerns and suggestions.

Some of concerns you brought up were:

1. Constraining max versions in gem specs or code make it difficult to test newer versions of instrumented gems
2. You are concerned about complexity of using multiple strategies for looking up the instrumented gems version
3. Giving developers the option to set an instrumented name adds complexity

You had suggested the following:

1. Have each instrumentation provide the version number of the instrumented gem using the constant instead of using Gem.loaded_specs or looking up the installed gem via the Gem API e.g. Kafka::VERSION

You had mentioned that you would run through this when you had time and would update the existing instrumentations. 👍🏼 that is totally fine

2. Require each instrumentation to declare an instrumented_library_name instead of making this field optional or trying to infer it from the instrumentation gem name.

Fine with me. I will put together a new PR that does that next week.

3. Keep the version constrains in code as constants instead of using the gemspec

This one I think is still up in the air. I pointed out during out chat that this would not alleviate the issue with ensuring we have a known/tested version as well as supporting an open ended version constraints you want to achieve with the appraisal gem.

I still find using RubyGems specifications to be the most idiomatic way to define compatibility constraints and compare the Requirement to the constant version of instrumented gem.

I recommend considering creating an open-ended appraisal and generating a special gemspec with an open-ended dependency for testing against the latest version of a library. I would also recommend that we make this check optional and potentially a nightly build for most cases that way if the "latest" version fails we do not block folks from releasing working versions.

We have a gemspec with the supported instrumented gems as a development dependency:

Gem::Specification.new do |spec|
  # lots of code...
  spec.add_development_dependency 'trilogy', '>= 2.0', '< 3.0'
end

We add an open-ended appraisal for latest

# frozen_string_literal: true

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

appraise 'trilogy-2.0' do
  gem 'trilogy', '~> 2.0'
end

appraise 'trilogy-latest' do
  gem 'trilogy' # maybe a git repo for edge versions
end

We either maintain multiple gemspecs, generate a new one removing constraints, or possibly add an environment variable that leaves off the constraints

Gem::Specification.new do |spec|
  # lots of code...
  spec.add_development_dependency 'trilogy'
end

Thoughts?

[github-actions]

👋 This issue has been marked as stale because it has been open with no activity. You can: comment on the issue or remove the stale label to hold stale off for a while, add the keep label to hold stale off permanently, or do nothing. If you do nothing this issue will be closed eventually by the stale bot.