Skip to content

Commit 20a49b7

Browse files
VihasMakwanacarsonipmx-psi
authored
[exporter/elasticsearch] use validation for batcher (#38072)
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a feature - Explain what this achieves.--> #### Description This PR fixes the validation for batcher. - We create our own batcher struct and embed it, instead of using `exporterbatcher.Config` (for backward compatibility). We ignore key validations done in [exporterbatcher's validate](https://github.com/open-telemetry/opentelemetry-collector/blob/56fbf4c2c19ec2a3be8390b84046293f69f5028e/exporter/exporterbatcher/config.go#L46-L60). <!--Describe what testing was performed and which tests were added.--> #### Testing Added <!--Describe the documentation added.--> #### Documentation <!--Please delete paragraphs that you did not use before submitting.--> --------- Co-authored-by: Carson Ip <[email protected]> Co-authored-by: Pablo Baeyens <[email protected]>
1 parent 620cb89 commit 20a49b7

File tree

4 files changed

+56
-8
lines changed

4 files changed

+56
-8
lines changed
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
# Use this changelog template to create an entry for release notes.
2+
3+
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
4+
change_type: bug_fix
5+
6+
# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver)
7+
component: elasticsearchexporter
8+
9+
# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
10+
note: Validate batcher config for exporter
11+
12+
# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists.
13+
issues: [38072]
14+
15+
# (Optional) One or more lines of additional information to render under the primary note.
16+
# These lines will be padded with 2 spaces and then inserted directly into the document.
17+
# Use pipe (|) for multiline entries.
18+
subtext:
19+
20+
# If your change doesn't affect end users or the exported elements of any package,
21+
# you should instead start your pull request title with [chore] or use the "Skip Changelog" label.
22+
# Optional: The change log or logs in which this entry should be included.
23+
# e.g. '[user]' or '[user, api]'
24+
# Include 'user' if the change is relevant to end users.
25+
# Include 'api' if there is a change to a library API.
26+
# Default: '[user]'
27+
change_logs: []

exporter/elasticsearchexporter/config.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -274,6 +274,11 @@ func (cfg *Config) Validate() error {
274274
if cfg.Retry.MaxRetries < 0 {
275275
return errors.New("retry::max_retries should be non-negative")
276276
}
277+
if batcherCfg, ok := cfg.exporterbatcherConfig(); ok {
278+
if err := batcherCfg.Validate(); err != nil {
279+
return fmt.Errorf("invalid batcher config: %w", err)
280+
}
281+
}
277282

278283
return nil
279284
}
@@ -314,6 +319,18 @@ func (cfg *Config) endpoints() ([]string, error) {
314319
return endpoints, nil
315320
}
316321

322+
func (cfg *Config) exporterbatcherConfig() (exporterbatcher.Config, bool) {
323+
if cfg.Batcher.Enabled == nil {
324+
return exporterbatcher.Config{}, false
325+
}
326+
return exporterbatcher.Config{
327+
Enabled: *cfg.Batcher.Enabled,
328+
FlushTimeout: cfg.Batcher.FlushTimeout,
329+
MinSizeConfig: cfg.Batcher.MinSizeConfig,
330+
MaxSizeConfig: cfg.Batcher.MaxSizeConfig,
331+
}, true
332+
}
333+
317334
func validateEndpoint(endpoint string) error {
318335
if endpoint == "" {
319336
return errConfigEmptyEndpoint

exporter/elasticsearchexporter/config_test.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -425,6 +425,16 @@ func TestConfig_Validate(t *testing.T) {
425425
}),
426426
err: `must not specify both retry::max_requests and retry::max_retries`,
427427
},
428+
"batcher max_size_items less than min_size_items": {
429+
config: withDefaultConfig(func(cfg *Config) {
430+
cfg.Endpoints = []string{"http://test:9200"}
431+
cfg.Batcher.MaxSizeItems = 1000
432+
cfg.Batcher.MinSizeItems = 2000
433+
enableBatcher := true
434+
cfg.Batcher.Enabled = &enableBatcher
435+
}),
436+
err: `max_size_items must be greater than or equal to min_size_items`,
437+
},
428438
}
429439

430440
for name, tt := range tests {

exporter/elasticsearchexporter/factory.go

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -203,14 +203,8 @@ func exporterhelperOptions(
203203
exporterhelper.WithShutdown(shutdown),
204204
exporterhelper.WithQueue(cfg.QueueSettings),
205205
}
206-
if cfg.Batcher.Enabled != nil {
207-
batcherConfig := exporterbatcher.Config{
208-
Enabled: *cfg.Batcher.Enabled,
209-
FlushTimeout: cfg.Batcher.FlushTimeout,
210-
MinSizeConfig: cfg.Batcher.MinSizeConfig,
211-
MaxSizeConfig: cfg.Batcher.MaxSizeConfig,
212-
}
213-
opts = append(opts, exporterhelper.WithBatcher(batcherConfig))
206+
if batcherCfg, ok := cfg.exporterbatcherConfig(); ok {
207+
opts = append(opts, exporterhelper.WithBatcher(batcherCfg))
214208

215209
// Effectively disable timeout_sender because timeout is enforced in bulk indexer.
216210
//

0 commit comments

Comments
 (0)