Skip to content

[exporter/elasticsearch] Update batcher config to adopt changes in core#41225

Merged
dmitryax merged 2 commits into
open-telemetry:mainfrom
dmitryax:update-elastic-exporter
Jul 10, 2025
Merged

[exporter/elasticsearch] Update batcher config to adopt changes in core#41225
dmitryax merged 2 commits into
open-telemetry:mainfrom
dmitryax:update-elastic-exporter

Conversation

@dmitryax

@dmitryax dmitryax commented Jul 10, 2025

Copy link
Copy Markdown
Member

Adopt to open-telemetry/opentelemetry-collector#13338 to unblock the core dependency upgrade

For the end user, everything stays as is for now. Going forward, the batcher section should be deprecated and removed. See #41224 as an example. I'm leaving that part to the code owners.

@dmitryax dmitryax requested a review from a team as a code owner July 10, 2025 05:07
@dmitryax dmitryax requested a review from songy23 July 10, 2025 05:07
@dmitryax dmitryax force-pushed the update-elastic-exporter branch from deccbc6 to 4fc12ea Compare July 10, 2025 05:09
Comment thread exporter/elasticsearchexporter/exporter_test.go

@axw axw left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for taking care of this, LGTM.

Comment thread .chloggen/elasticsearchexporter-update-batcher-cfg-api.yaml Outdated
@dmitryax dmitryax force-pushed the update-elastic-exporter branch from 4fc12ea to 35ee875 Compare July 10, 2025 05:17
@dmitryax dmitryax force-pushed the update-elastic-exporter branch from 35ee875 to 7e091e2 Compare July 10, 2025 05:19
Comment thread exporter/elasticsearchexporter/factory.go
// to ensure sending data to the background workers will not block indefinitely.
opts = append(opts, exporterhelper.WithTimeout(exporterhelper.TimeoutConfig{Timeout: 0}))
}
opts = append(opts, exporterhelper.WithQueue(qs))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

if you do

	if err := qs.Validate(); err != nil {
		panic("invalid config " + err.Error())
	}

it is failing with

`batch` supports only `items` or `bytes` sizer`

which is what's failing integration tests. We did not validate our manually transformed config and this misconfiguration causes batcher to fail.

songy23

This comment was marked as resolved.

Co-authored-by: Carson Ip <carsonip@users.noreply.github.com>
@dmitryax dmitryax merged commit 31093d5 into open-telemetry:main Jul 10, 2025
177 checks passed
@github-actions github-actions Bot added this to the next release milestone Jul 10, 2025
@dmitryax dmitryax deleted the update-elastic-exporter branch July 10, 2025 22:16
Dylan-M pushed a commit to Dylan-M/opentelemetry-collector-contrib that referenced this pull request Aug 5, 2025
…re (open-telemetry#41225)

Adopt to
open-telemetry/opentelemetry-collector#13338 to
unblock the core dependency upgrade

For the end user, everything stays as is for now. Going forward, the
`batcher` section should be deprecated and removed. See
open-telemetry#41224
as an example. I'm leaving that part to the code owners.

---------

Co-authored-by: Carson Ip <carsonip@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants