[exporterqueue] Bare minimum frame of queue batcher + unit test.

[sfc-gh-sili]

Description

This PR is a bare minimum implementation of a component called queue batcher. On completion, this component will replace consumers in queue_sender, and thus moving queue-batch from a pulling model instead of pushing model.

Limitations of the current code

• This implements only the case where batching is disabled, which means no merge of splitting of requests + no timeout flushing.
• This implementation does not enforce an upper bound on concurrency

All these code paths are marked as panic currently, and they will be replaced with actual implementation in coming PRs. This PR is split from #11507 for easier review.

Design doc:
https://docs.google.com/document/d/1y5jt7bQ6HWt04MntF8CjUwMBBeNiJs2gV4uUZfJjAsE/edit?usp=sharing

Link to tracking issue

#8122
#10368

Testing

Documentation

[codecov]

Codecov Report

Attention: Patch coverage is 81.94444% with 13 lines in your changes missing coverage. Please review.

Project coverage is 91.43%. Comparing base (ef79a0e) to head (1d5a4f3).

Files with missing lines	Patch %	Lines
exporter/internal/queue/fake_request.go	66.66%	8 Missing ⚠️
exporter/internal/queue/batcher.go	83.87%	3 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #11532      +/-   ##
==========================================
- Coverage   91.46%   91.43%   -0.03%     
==========================================
  Files         435      438       +3     
  Lines       23757    23829      +72     
==========================================
+ Hits        21729    21788      +59     
- Misses       1650     1661      +11     
- Partials      378      380       +2     

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

[bogdandrutu]

Is this not Request.export?

[sfc-gh-sili]

Good point.
This was modeled after consumeFunc used in consumer but it seems having req.Export() here is enough for the use case.

[bogdandrutu]

Comment on the struct please.

[bogdandrutu]

In golang, comments start with func name. flush

[bogdandrutu]

Suggested change

	// flushAsync() starts a goroutine that calls flushIfNecessary(). It blocks until a worker is available.
	// flushAsync starts a goroutine that calls flushIfNecessary(). It blocks until a worker is available.

[bogdandrutu]

This is a best practice to ensure function gets called on all possible "return paths". Even though now is a simple func may get more complicated.

Suggested change

	go func() {
	qb.flush(batchToFlush)
	qb.stopWG.Done()
	}()
	go func() {
	defer qb.stopWG.Done()
	qb.flush(batchToFlush)
	}()

[sfc-gh-sili]

Thanks for the advice!

[bogdandrutu]

Is the batcher responsibility to start/shutdown the queue?

[sfc-gh-sili]

That's a good point. Let me keep them here for now because this is consistent with queue.consumer. I wanted to make queue.batcher work like an in-place swap for queue.consumer

I'll leave a note to move this up to queue sender once queue.consumer is cleaned up.

[sfc-gh-sili]

#11534

[dmitryax]

PR is merged. We can remove this I guess

[bogdandrutu]

I think would be simpler if Batcher is an interface, and the "not enabled" and "enabled" are 2 different implementations.

[sfc-gh-sili]

That's a good idea.
Implemented a special-case batcher called DisabledBatcher to handle the "not enabled" case. Let me know what you think of it.