Skip to content

Remove deprecated old batcher config#13003

Closed
bogdandrutu wants to merge 1 commit into
open-telemetry:mainfrom
bogdandrutu:rm-old-batcher
Closed

Remove deprecated old batcher config#13003
bogdandrutu wants to merge 1 commit into
open-telemetry:mainfrom
bogdandrutu:rm-old-batcher

Conversation

@bogdandrutu

Copy link
Copy Markdown
Member

No description provided.

Signed-off-by: Bogdan Drutu <bogdandrutu@gmail.com>
@bogdandrutu bogdandrutu changed the title [mdatagen] Add functions for processing structured events (#12961) Remove deprecated old batcher config May 8, 2025

@dmitryax dmitryax left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't think we can remove it until #12879 is resolved

@bogdandrutu

bogdandrutu commented May 8, 2025

Copy link
Copy Markdown
Member Author

I don't think we can remove it until #12879 is resolved

These are different issues. the deprecated config also does not support "request" batching.

See https://github.com/open-telemetry/opentelemetry-collector/pull/13003/files#diff-45d09df1b12e28cfc26a0bc617f391a559334fed9a5df242fd179ad5e85fe3e0L141

@dmitryax

dmitryax commented May 8, 2025

Copy link
Copy Markdown
Member

These are different issues. the deprecated config also does not support "request" batching.

See https://github.com/open-telemetry/opentelemetry-collector/pull/13003/files#diff-45d09df1b12e28cfc26a0bc617f391a559334fed9a5df242fd179ad5e85fe3e0L141

Right, but the problem is that persistent storage can only work with requests sizer. Having the old batcher config allows us to use items for batcher and requests for persistent queue. If the old batch interface is removed, users won't be able to use the persistent queue with the batcher at all because there only one sizer can be configured for both.

@iblancasa

Copy link
Copy Markdown
Contributor

If the old batch interface is removed, users won't be able to use the persistent queue with the batcher at all because there only one sizer can be configured for both.

But this is the current behavior, right? Maybe an error can be returned if the user tries to use that queue with a different sizer (until it is implemented).

@dmitryax

dmitryax commented May 15, 2025

Copy link
Copy Markdown
Member

But this is the current behavior, right? Maybe an error can be returned if the user tries to use that queue with a different sizer (until it is implemented).

No, currently users can configure persistent queue with the batcher using the old batcher config interface

@bogdandrutu

Copy link
Copy Markdown
Member Author

Since this is "experimental" feature, still not sure we should block this... but for the moment, if in this version we don't have a solution for your problem @dmitryax I suggest we move forward.

@dmitryax

Copy link
Copy Markdown
Member

Why do we need to remove this urgently? I think we should have a migration path without introducing regressions

@github-actions

github-actions Bot commented Jun 5, 2025

Copy link
Copy Markdown
Contributor

This PR was marked stale due to lack of activity. It will be closed in 14 days.

@github-actions github-actions Bot added the Stale label Jun 5, 2025
@github-actions

Copy link
Copy Markdown
Contributor

Closed as inactive. Feel free to reopen if this PR is still being worked on.

@github-actions github-actions Bot closed this Jun 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants