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

[chore] [exporterhelper] Items based queue sizing with bounded channel #9164

Merged

Conversation

dmitryax
Copy link
Member

@dmitryax dmitryax commented Dec 20, 2023

Introduce an option to limit the queue size by the number of items instead of number of requests. This is preliminary step for having the exporter helper v2 with a batcher sender placed after the queue sender. Otherwise, it'll be hard for the users to estimate the queue size based on the number of requests without batch processor in front of it.

This change doesn't effect the existing functionality and the items based queue limiting cannot be utilized yet.

Updates #8122

Alternative to #9147

Copy link

codecov bot commented Dec 20, 2023

Codecov Report

Attention: 11 lines in your changes are missing coverage. Please review.

Comparison is base (1c84578) 91.39% compared to head (f72cb9b) 91.39%.

Files Patch % Lines
...porter/exporterhelper/internal/persistent_queue.go 87.05% 8 Missing and 3 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #9164      +/-   ##
==========================================
- Coverage   91.39%   91.39%   -0.01%     
==========================================
  Files         320      321       +1     
  Lines       17208    17297      +89     
==========================================
+ Hits        15728    15808      +80     
- Misses       1177     1183       +6     
- Partials      303      306       +3     

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

@dmitryax dmitryax force-pushed the itemized-queue-sizing-bound-channel branch from 2f30103 to 0ad2efc Compare December 20, 2023 20:06
@dmitryax dmitryax changed the title [chore] [DRAFT] Items based queue sizing with bound channel [chore] [exporterhelper] Items based queue sizing with bound channel Dec 20, 2023
@dmitryax dmitryax changed the title [chore] [exporterhelper] Items based queue sizing with bound channel [chore] [exporterhelper] Items based queue sizing with bounded channel Dec 20, 2023
@dmitryax dmitryax force-pushed the itemized-queue-sizing-bound-channel branch from 0ad2efc to 5d5ac37 Compare December 20, 2023 22:59
@dmitryax dmitryax marked this pull request as ready for review December 20, 2023 22:59
@dmitryax dmitryax requested review from a team and Aneurysm9 December 20, 2023 22:59
@dmitryax dmitryax force-pushed the itemized-queue-sizing-bound-channel branch from 5d5ac37 to 2d04229 Compare December 20, 2023 23:05
@@ -78,15 +84,17 @@ var (
)

// NewPersistentQueue creates a new queue backed by file storage; name and signal must be a unique combination that identifies the queue storage
func NewPersistentQueue[T any](capacity int, dataType component.DataType, storageID component.ID, marshaler func(req T) ([]byte, error), unmarshaler func([]byte) (T, error), set exporter.CreateSettings) Queue[T] {
func NewPersistentQueue[T any](cl QueueCapacityLimiter[T], dataType component.DataType, storageID component.ID,
Copy link
Member

Choose a reason for hiding this comment

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

Is that moment when you better have a Settings struct for all these params you pass. Separate PR.

exporter/exporterhelper/internal/queue_capacity.go Outdated Show resolved Hide resolved
@dmitryax dmitryax force-pushed the itemized-queue-sizing-bound-channel branch 3 times, most recently from 64301cb to b2f6084 Compare December 21, 2023 07:03
exporter/exporterhelper/internal/queue_capacity.go Outdated Show resolved Hide resolved
exporter/exporterhelper/internal/queue_capacity.go Outdated Show resolved Hide resolved
@dmitryax dmitryax force-pushed the itemized-queue-sizing-bound-channel branch from b2f6084 to b9a69cb Compare December 22, 2023 00:37
@dmitryax dmitryax force-pushed the itemized-queue-sizing-bound-channel branch from b9a69cb to f72cb9b Compare December 22, 2023 00:48
@dmitryax dmitryax merged commit fe2c989 into open-telemetry:main Dec 22, 2023
31 of 32 checks passed
@github-actions github-actions bot added this to the next release milestone Dec 22, 2023
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.

2 participants