Skip to content

Conversation

@leehinman
Copy link
Contributor

What does this PR do?

Allows the hosts part of the beat output to be a string instead of a slice.

This is to match the behavior of beats which allows this.

Why is it important?

This means that more hand written configuration will work "out of the box" with beat receivers.

Checklist

  • I have read and understood the pull request guidelines of this project.
  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have made corresponding change to the default configuration files
  • I have added tests that prove my fix is effective or that my feature works
  • I have added an entry in ./changelog/fragments using the changelog tool
  • I have added an integration test or an E2E test

Disruptive User Impact

None.

How to test this PR locally

cd internal/pkg/otel/translate
go test -run TestToOtelConfig

Related issues

Questions to ask yourself

  • How are we going to support this in production?
  • How are we going to measure its adoption?
  • How are we going to debug this?
  • What are the metrics I should take care of?
  • ...

@mergify
Copy link
Contributor

mergify bot commented Nov 24, 2025

This pull request does not have a backport label. Could you fix it @leehinman? 🙏
To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

  • backport-./d./d is the label that automatically backports to the 8./d branch. /d is the digit
  • backport-active-all is the label that automatically backports to all active branches.
  • backport-active-8 is the label that automatically backports to all active minor branches for the 8 major.
  • backport-active-9 is the label that automatically backports to all active minor branches for the 9 major.

@leehinman leehinman force-pushed the 11352_translate_hosts branch from 7db7e5c to df5d2ac Compare November 24, 2025 21:30
@leehinman leehinman added enhancement New feature or request backport-8.19 Automated backport to the 8.19 branch backport-9.1 Automated backport to the 9.1 branch backport-9.2 Automated backport to the 9.2 branch labels Nov 24, 2025
@leehinman leehinman marked this pull request as ready for review November 24, 2025 22:17
@leehinman leehinman requested a review from a team as a code owner November 24, 2025 22:17
@elasticmachine
Copy link
Contributor

mauri870
mauri870 previously approved these changes Nov 25, 2025
swiatekm
swiatekm previously approved these changes Nov 25, 2025
ycombinator
ycombinator previously approved these changes Nov 25, 2025
Copy link
Contributor

@ycombinator ycombinator left a comment

Choose a reason for hiding this comment

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

I didn't quite follow why the "before" implementation wouldn't allow hosts to be a string but that might be because I don't have enough context.

The "after" implementation makes a lot of sense, though, since it's using the hosts from the Beat config object and I know that supports either string or list of strings for hosts.

@leehinman
Copy link
Contributor Author

I didn't quite follow why the "before" implementation wouldn't allow hosts to be a string but that might be because I don't have enough context.

Yeah, it isn't obvious. But in elastic-agent we had an embedded HostWorkerConfig, and that has Hosts as a slice of strings. So when you did the decode it would complain about the type mismatch.

The "fix" here treats the configuration more like beats does. Like beats we don't unpack with the HostWorkerConfig, we use the same function ReadHostList to handle the Hosts and Worker value.

The "after" implementation makes a lot of sense, though, since it's using the hosts from the Beat config object and I know that supports either string or list of strings for hosts.

I'm cheating. Since we want bug for bug compatibility I'm just using the same code ;-)

@leehinman leehinman dismissed stale reviews from ycombinator, swiatekm, and mauri870 via 9a8b371 November 25, 2025 19:46
@cmacknz cmacknz enabled auto-merge (squash) November 25, 2025 21:35
@cmacknz cmacknz merged commit 70ef801 into elastic:main Nov 25, 2025
23 of 24 checks passed
mergify bot pushed a commit that referenced this pull request Nov 25, 2025
* [otel config translate] allow hosts to be string not just slice

* update changelog to bugfix

(cherry picked from commit 70ef801)
mergify bot pushed a commit that referenced this pull request Nov 25, 2025
* [otel config translate] allow hosts to be string not just slice

* update changelog to bugfix

(cherry picked from commit 70ef801)

# Conflicts:
#	internal/pkg/otel/translate/output_elasticsearch.go
#	internal/pkg/otel/translate/output_elasticsearch_test.go
mergify bot pushed a commit that referenced this pull request Nov 25, 2025
* [otel config translate] allow hosts to be string not just slice

* update changelog to bugfix

(cherry picked from commit 70ef801)
cmacknz pushed a commit that referenced this pull request Nov 25, 2025
…) (#11432)

* [otel config translate] allow hosts to be string not just slice

* update changelog to bugfix

(cherry picked from commit 70ef801)

Co-authored-by: Lee E Hinman <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport-8.19 Automated backport to the 8.19 branch backport-9.1 Automated backport to the 9.1 branch backport-9.2 Automated backport to the 9.2 branch enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

OTel manager failed: failed to generate otel config: error translating config for output: ... 'hosts' source data must be an array or slice, got string

6 participants