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

fix: use api 1.1 in SDKs #2759

Merged
merged 1 commit into from
Feb 5, 2022
Merged

Conversation

Flarna
Copy link
Member

@Flarna Flarna commented Feb 3, 2022

Which problem is this PR solving?

Fixes #2755

Short description of the changes

Use API 1.1 in SDKs to avoid that SDKs register a TracerProvider,... using an older API then other OTel components.
This avoids the version check in API rejecting the use of API 1.1 but a peer may still use 1.0.x.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

CI was failing before

Checklist:

  • Followed the style guidelines of this project
  • Unit tests have been added
  • Documentation has been updated

Use API 1.1 in SDKs to avoid that SDKs register a TracerProvider,... using an older API
then other OTel components.
This avoids the version check in API rejecting the use of API 1.1 but a peer may still
use 1.0.x.
@Flarna Flarna requested a review from a team February 3, 2022 08:22
@codecov
Copy link

codecov bot commented Feb 3, 2022

Codecov Report

Merging #2759 (1e4f56a) into main (ee2342b) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #2759   +/-   ##
=======================================
  Coverage   93.27%   93.27%           
=======================================
  Files         159      159           
  Lines        5444     5444           
  Branches     1142     1142           
=======================================
  Hits         5078     5078           
  Misses        366      366           

@Flarna
Copy link
Member Author

Flarna commented Feb 3, 2022

This fixes the current issue in CI but honestly speaking I'm not sure if this is really the way to go. Maybe we should narrow more places or find some other way to ensure a correct mix of versions gets installed.

The version dependencies we have now (API, core-stable, core-experimental, contrib which releases packages independent) is getting more and more complex and hard to understand.

@vmarchaud
Copy link
Member

The version dependencies we have now (API, core-stable, core-experimental, contrib which releases packages independent) is getting more and more complex and hard to understand.

I agree that the matrix is complex but i don't see another way to do it right now :/

@legendecas
Copy link
Member

I'm landing this now as it is blocking our CI services.

@legendecas legendecas merged commit e9e4567 into open-telemetry:main Feb 5, 2022
@Flarna Flarna deleted the sdk-use-api-1.1 branch February 7, 2022 07:23
@Flarna Flarna mentioned this pull request Feb 7, 2022
2 tasks
@Flarna
Copy link
Member Author

Flarna commented Feb 7, 2022

I created a followup issue #2769

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.

Stable package tests CI failure on Node.js v16
3 participants