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

[exporter] Disable the API to pass in configurations using a callback that operates on batch sender #11448

Merged

Conversation

sfc-gh-sili
Copy link
Contributor

Description

As part of the effort to solve #10368, we no longer guarantee to initialize a batchSender when batcher is enabled. Therefore, we would like to remove the interface to set mergeFunc and mergeSplitFunc as a callback that operates on batchSender. Instead, users should use the alternative WithBatchFuncs that is a callback that operates baseExporter.

Context: #11414

Link to tracking issue

#8122
#10368

Testing

Documentation

@sfc-gh-sili sfc-gh-sili requested a review from a team as a code owner October 15, 2024 04:59
@sfc-gh-sili sfc-gh-sili requested a review from codeboten October 15, 2024 04:59
@sfc-gh-sili sfc-gh-sili changed the title [exporter] Clean up batch option that is applied to batch sender [exporter] Disable the API to pass in configurations using a callback that operates on batch sender Oct 15, 2024
Copy link

codecov bot commented Oct 15, 2024

Codecov Report

Attention: Patch coverage is 20.00000% with 4 lines in your changes missing coverage. Please review.

Project coverage is 92.15%. Comparing base (a7d019f) to head (918bd38).
Report is 6 commits behind head on main.

Files with missing lines Patch % Lines
exporter/exporterhelper/common.go 0.00% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #11448      +/-   ##
==========================================
- Coverage   92.15%   92.15%   -0.01%     
==========================================
  Files         432      431       -1     
  Lines       20253    20238      -15     
==========================================
- Hits        18664    18650      -14     
+ Misses       1228     1227       -1     
  Partials      361      361              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@dmitryax
Copy link
Member

Please add a breaking API changelog item

@@ -275,30 +275,14 @@ func WithCapabilities(capabilities consumer.Capabilities) Option {
}
}

// WithRequestBatchFuncs sets the functions for merging and splitting batches for an exporter built for custom request types.
func WithRequestBatchFuncs(mf exporterbatcher.BatchMergeFunc[internal.Request], msf exporterbatcher.BatchMergeSplitFunc[internal.Request]) BatcherOption {
Copy link
Member

Choose a reason for hiding this comment

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

Delete also BatcherOption

@dmitryax dmitryax merged commit 527df61 into open-telemetry:main Oct 16, 2024
48 of 49 checks passed
@github-actions github-actions bot added this to the next release milestone Oct 16, 2024
@sfc-gh-sili sfc-gh-sili deleted the sili-queue-disable-batch-option branch October 16, 2024 08:33
djaglowski pushed a commit to djaglowski/opentelemetry-collector that referenced this pull request Nov 21, 2024
… that operates on batch sender (open-telemetry#11448)

#### Description

As part of the effort to solve
open-telemetry#10368,
we no longer guarantee to initialize a `batchSender` when `batcher` is
enabled. Therefore, we would like to remove the interface to set
`mergeFunc` and `mergeSplitFunc` as a callback that operates on
`batchSender`. Instead, users should use the alternative
`WithBatchFuncs` that is a callback that operates `baseExporter`.

Context:
open-telemetry#11414

#### Link to tracking issue

open-telemetry#8122
open-telemetry#10368

---------

Co-authored-by: Bogdan Drutu <bogdandrutu@gmail.com>
HongChenTW pushed a commit to HongChenTW/opentelemetry-collector that referenced this pull request Dec 19, 2024
… that operates on batch sender (open-telemetry#11448)

#### Description

As part of the effort to solve
open-telemetry#10368,
we no longer guarantee to initialize a `batchSender` when `batcher` is
enabled. Therefore, we would like to remove the interface to set
`mergeFunc` and `mergeSplitFunc` as a callback that operates on
`batchSender`. Instead, users should use the alternative
`WithBatchFuncs` that is a callback that operates `baseExporter`.

Context:
open-telemetry#11414

#### Link to tracking issue

open-telemetry#8122
open-telemetry#10368

---------

Co-authored-by: Bogdan Drutu <bogdandrutu@gmail.com>
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.

3 participants