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.

Sign up for GitHub

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

Jump to bottom

Move TraceState operations description from SDK to API #922

Merged
merged 2 commits into from
Sep 9, 2020
Merged

Move TraceState operations description from SDK to API #922

merged 2 commits into from
Sep 9, 2020

Conversation

iNikem
Copy link
Contributor

@iNikem iNikem commented Sep 4, 2020

After #905 was merged I was approached by JavaScript and Java SIGs, who brought to my attention that in these languages Propagators currently depend only on API, not SDK. As Propagators are the most common users of the TraceState API, I hereby try to remedy the situation and propose to move it from SDK specification to the API.

@iNikem iNikem requested review from a team as code owners September 4, 2020 17:29
@iNikem
Copy link
Contributor Author

iNikem commented Sep 4, 2020

@dyladan

dyladan
dyladan approved these changes Sep 4, 2020
View reviewed changes
Copy link
Member

@dyladan dyladan left a comment

Choose a reason for hiding this comment

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

Thanks :)

@jtescher jtescher mentioned this pull request Sep 4, 2020
Closed
@carlosalberto carlosalberto added area:api Cross language API specification issue area:sdk Related to the SDK spec:trace Related to the specification/trace directory release:required-for-ga Must be resolved before GA release, or nice to have before GA priority:p1 Highest priority level labels Sep 8, 2020
@carlosalberto
Copy link
Contributor

Marking it as Required for GA, as this breaks the API (for good, hopefully :) )

Oberon00
Oberon00 approved these changes Sep 9, 2020
View reviewed changes
Copy link
Member

@Oberon00 Oberon00 left a comment

Choose a reason for hiding this comment

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

This should mostly document the status quo and is sensible enough 👍

@carlosalberto
Copy link
Contributor

@open-telemetry/specs-approvers Please review this change.

@bogdandrutu bogdandrutu merged commit c7055b1 into open-telemetry:master Sep 9, 2020
@fbogsany fbogsany mentioned this pull request Sep 10, 2020
Closed
@iNikem iNikem deleted the tracestate branch September 10, 2020 05:57
@matej-g matej-g mentioned this pull request Nov 6, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tsloughter tsloughter tsloughter approved these changes

@anuraaga anuraaga anuraaga approved these changes

@carlosalberto carlosalberto carlosalberto approved these changes

@Oberon00 Oberon00 Oberon00 approved these changes

@bogdandrutu bogdandrutu bogdandrutu approved these changes

@dyladan dyladan dyladan approved these changes

@arminru arminru arminru approved these changes

Assignees

@bogdandrutu bogdandrutu

Labels
area:api Cross language API specification issue area:sdk Related to the SDK priority:p1 Highest priority level release:required-for-ga Must be resolved before GA release, or nice to have before GA spec:trace Related to the specification/trace directory
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
8 participants
@iNikem @carlosalberto @tsloughter @anuraaga @Oberon00 @bogdandrutu @dyladan @arminru