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

Removal Sinatra 1.x Appraisal #715

Merged

Conversation

zacheryph
Copy link
Contributor

Sinatra 1.x is not supported as it does not provide the Sinatra::Event API. It does not need to be appraised.

This should close #364.

This issue recommends documenting the last supported version for Sinatra 1.x, Given that the last release of Sinatra was January 2017, is this still something that is advised?

The gemspec references "sinatra", without a strict version requirement. I was going to update this but I noticed all the other instrumentation packages don't reference minimal versions for dependencies either.

Copy link

linux-foundation-easycla bot commented Nov 8, 2023

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: zacheryph / name: Zachery Hostens (e337e79)
  • ✅ login: arielvalentin / name: Ariel Valentin (2308826)

@simi
Copy link
Contributor

simi commented Nov 8, 2023

@zacheryph hello! Change looks good. Can you please update commit message according to https://github.com/open-telemetry/opentelemetry-ruby-contrib/blob/main/CONTRIBUTING.md#use-conventional-commit-messages?

@zacheryph zacheryph force-pushed the document-sinatra-1-support branch from 7761a7e to 6b874c6 Compare November 8, 2023 21:51
@arielvalentin
Copy link
Collaborator

I think all we need a matrix that contains the last supported version in a markdown file.

I also agree that the gemspec should also reflect the minimum version we support. We tend to encode that at runtime but making it explicit in the gemspec certainly helps IMO.

As far as this PR is concerned, removing the tests from the suite is the first step. If you would like to also include the min version in the gemspec as part of this PR I would be grateful.

@simi
Copy link
Contributor

simi commented Nov 8, 2023

If you would like to also include the min version in the gemspec as part of this PR I would be grateful.

@arielvalentin
Copy link
Collaborator

@simi that is bizarre. Does that mean the appraisal gem is overriding the sinatra version specified in the gemspec somehow?

@simi
Copy link
Contributor

simi commented Nov 8, 2023

@simi that is bizarre. Does that mean the appraisal gem is overriding the sinatra version specified in the gemspec somehow?

Yes, both ways. Up (for sinatra 3) and down (for sinatra 1). That is how gemspec included in Gemfile works (it is not Appraisals related) for development dependencies.

@arielvalentin
Copy link
Collaborator

That is how gemspec included in Gemfile works (it is not Appraisals related) for development dependencies.

This is going to sound like I have no idea how to Ruby. Is this because appraisals are overriding development dependencies with runtime installs?

@simi
Copy link
Contributor

simi commented Nov 9, 2023

@arielvalentin so as
@deivid-rodriguez pointed me out, it is actually "bug" (rubygems/rubygems#6014). Better to not rely on that IMHO.

Rack 1.x is not directly supported anymore. Sinatra 1.x in turn
"is not". Removal appraisal and add a compatibility note to both
Sinatra and Rack for proper instrumentation version usage.
@zacheryph zacheryph force-pushed the document-sinatra-1-support branch from bcfcfff to e337e79 Compare November 13, 2023 15:12
@zacheryph
Copy link
Contributor Author

@arielvalentin This is getting slightly messy at this point 🤣 (also rebased to capture latest main)

after looking at dependency requirements, the bigger issue i think here is Rack instrumentation and not Sinatra specifically. I went ahead and added an excerpt to the Rack README, as well as the Sinatra README. The latest Sinatra instrumentation should technically still work since it has a dependency version on otel-inst-rack ~> 0.21, and 0.22.1 is the latest version that will support Rack 1.x (does not introduce the Rack::Event API)

@arielvalentin arielvalentin merged commit 02799c8 into open-telemetry:main Nov 19, 2023
47 checks passed
jdehaan pushed a commit to tools-aoeur/opentelemetry-ruby-contrib that referenced this pull request Nov 21, 2023
fix: Removal Sinatra 1.x Appraisal

Rack 1.x is not directly supported anymore. Sinatra 1.x in turn
"is not". Removal appraisal and add a compatibility note to both
Sinatra and Rack for proper instrumentation version usage.

Co-authored-by: Ariel Valentin <[email protected]>
jdehaan added a commit to tools-aoeur/opentelemetry-ruby-contrib that referenced this pull request Nov 21, 2023
* added httpx opentelemetry adapter (open-telemetry#681)

* added httpx opentelemetry adapter

* Update instrumentation/httpx/CHANGELOG.md

Co-authored-by: Ariel Valentin <[email protected]>

* Update instrumentation/httpx/README.md

Co-authored-by: Ariel Valentin <[email protected]>

* Update instrumentation/httpx/README.md

Co-authored-by: Ariel Valentin <[email protected]>

---------

Co-authored-by: Ariel Valentin <[email protected]>
Co-authored-by: Josef Šimánek <[email protected]>

* chore: Change release restrictions

The toys gem checks that all actions are passing before allowing
a release request to be opened, however only the CI builds are actually required.

This change limits the release request requirements to look specifically for CI builds

* chore: Fix httpx version

* chore: bump toys

* release: Release opentelemetry-instrumentation-httpx 0.1.0 (initial release) (open-telemetry#713)

* release: Release opentelemetry-instrumentation-httpx 0.1.0 (initial release)

* Update instrumentation/httpx/CHANGELOG.md

* feat!: Drop Rails 6.0 EOL (open-telemetry#680)

* feat!: Drop Rails 6.0 EOL

6.0 is no longer receiving maintenance, security, or feature updates as of 01 Jun 2023

Users who want to continue instrumentating Rails applications should pin to earlier versions of the instrumentation.

* squash: Fix test

* squash: gem version object instead of string

* Update README.md

Co-authored-by: Kayla Reopelle (she/her) <[email protected]>

* Update instrumentation/README.md

Co-authored-by: Kayla Reopelle (she/her) <[email protected]>

---------

Co-authored-by: Kayla Reopelle (she/her) <[email protected]>

* Inline gemspec dev constraints with Appraisals. (open-telemetry#716)

* Removal Sinatra 1.x Appraisal (open-telemetry#715)

fix: Removal Sinatra 1.x Appraisal

Rack 1.x is not directly supported anymore. Sinatra 1.x in turn
"is not". Removal appraisal and add a compatibility note to both
Sinatra and Rack for proper instrumentation version usage.

Co-authored-by: Ariel Valentin <[email protected]>

---------

Co-authored-by: Tiago <[email protected]>
Co-authored-by: Ariel Valentin <[email protected]>
Co-authored-by: Josef Šimánek <[email protected]>
Co-authored-by: Ariel Valentin <[email protected]>
Co-authored-by: Kayla Reopelle (she/her) <[email protected]>
Co-authored-by: Zachery Hostens <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Drop Support for Rack and Sinatra 1.x
3 participants