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

Minor clarification on BatchExportingProcessor behavior #4164

Merged
merged 10 commits into from
Aug 19, 2024

Conversation

cijothomas
Copy link
Member

@cijothomas cijothomas commented Jul 26, 2024

Changes

I am unsure of the meaning of the current wording, so trying to clarify here. It looks like the current wording was intended to convey that export() should not be triggered until previous export() finished, even if the triggers (batch-size met etc.) are ready. But the current wording is confusing. I hope I interpreted it correctly and this fix address the confusing aspect. If not, happy to correct myself.

For non-trivial changes, follow the change proposal process.

@cijothomas cijothomas requested review from a team July 26, 2024 05:53
Copy link
Member

@pellared pellared left a comment

Choose a reason for hiding this comment

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

That is a lot more readable and AFAIK it is indeed what the authors had in mind.

@pellared pellared added spec:trace Related to the specification/trace directory clarification clarify ambiguity in specification labels Jul 26, 2024
specification/trace/sdk.md Outdated Show resolved Hide resolved
cijothomas and others added 2 commits July 26, 2024 07:24
specification/trace/sdk.md Show resolved Hide resolved
@cijothomas
Copy link
Member Author

@tsloughter Could you take another look to see if the wording looks good?

@cijothomas
Copy link
Member Author

cijothomas commented Aug 5, 2024

Still need more reviews from @open-telemetry/technical-committee , could you help?

@cijothomas
Copy link
Member Author

@open-telemetry/technical-committee Please review/merge! Thanks.

@cijothomas
Copy link
Member Author

@open-telemetry/technical-committee @arminru Gentle reminder to look at this when you get a chance!

@arminru arminru merged commit 0b3328b into open-telemetry:main Aug 19, 2024
6 checks passed
@cijothomas cijothomas deleted the cijothomas/batch-span-fix branch August 19, 2024 14:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clarification clarify ambiguity in specification spec:trace Related to the specification/trace directory
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants