Remove x-pack code from OSS filebeat.

[vinit-chauhan]

Proposed commit message

To fix the failing linter rule that requires OSS code to not depend on x-pack
code, this change removes the dependency on x-pack/dockerlogbeat/pipelinemock
from the Filebeat filestream input. It does this by creating a mock client
that is local to filestream test code ("A little copying is better than a
little dependency").

Fixes #40293

Remove x-pack code from OSS filebeat.

This PR creates a MockPipelineConnector and MockClient interface in the libbeat to drop the x-pack dependency in OSS code.

Checklist

• 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.next.asciidoc or CHANGELOG-developer.next.asciidoc.

Disruptive User Impact

Author's Checklist

• [ ]

How to test this PR locally

Related issues

• Closes OSS filebeat harvester_test.go should not depend on Elastic-licensed code #40293

Use cases

Screenshots

Logs

[mergify]

This pull request does not have a backport label.
If this is a bug or security fix, could you label this PR @vinit-chauhan? 🙏.
For such, you'll need to label your PR with:

• The upcoming major version of the Elastic Stack
• The upcoming minor version of the Elastic Stack (if you're not pushing a breaking change)

To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

• backport-v8./d.0 is the label to automatically backport to the 8./d branch. /d is the digit

[elasticmachine]

Pinging @elastic/elastic-agent-data-plane (Team:Elastic-Agent-Data-Plane)

[andrzej-stencel]

Hey @vinit-chauhan, thanks for your contribution.

I'm comparing the code you added in libbeat/mock/pipeline_client.go to the original code in x-pack/dockerlogbeat/pipelinemock/pipelines.go, and I'm finding quite a lot of differences. My assumption was the code would be moved as-is, without any changes. Can you explain what was the rationale for the changes?

I'm worried that the changes to the package's API might make migration harder for any other code that currently relies on the x-pack library.

The changes I noticed:

• package name changed from pipelinemock to mock
• MockBeatClient type changed to MockClient
• MockPipelineConnector type replaced with MockPipeline type
• removed MockPipelineConnector methods GetAllEvents and HasConnectedClients.

@leehinman Do you have any thoughts on this?

[vinit-chauhan]

Hey @andrzej-stencel - I'm always happy to contribute to Elastic. :)

The reason for not moving the code as-is is, that this code is used only in the harvestor_test.go file for the OSS codebase. But if you want I can update the PR to move the x-pack code to OSS.

For the points you mentioned,

• Renaming package to mock: The x-pack code mocks functionality by implementing the libbeat features, that's the reason why I think the OSS version should move to libbeat. Additionally, there's already a package containing mocks named mock, so I moved it there all to a single place.
• MockBeatClient -> MockClient: I think the original author of the code added MockBeat because they mocked the libbeat's Client interface outside of the libbeat codebase, and now the code is in beats, I think adding Beats is redundant.
• MockPipelineConnector -> MockPipeline: I have implemented the Pipeline interface whereas the original code implements PipelineConnector interface which is a wrapper to the Pipeline interface.
• GetAllEvents and HasConnectedClients: Those 2 are additional methods added on top of PipelineConnector and used by the tests they have written.

interfaces used:

• https://github.com/elastic/beats/blob/main/libbeat/beat/pipeline.go

That is the explanation for the changes I have made. Let me know your thoughts.

[AndersonQ]

@vinit-chauhan given the limited usage of this new mock and that mock.New is a mockbeat and nothing else. Perhaps the best would be to move your mock code to filebeat/input/filestream/internal/input-logfile/harvester_test.go. There are already other mocks there, so it makes sense to have this one there as well.

@andrzej-stencel does it sound good to u?

[vinit-chauhan]

Hey @AndersonQ - wouldn't that cause the same issue?
If someone decides to use mockbeat in let's say metricbeat, then they'll have to re-write it or reference the filebeat code in metricbeat.

So, I think it's better to keep the libbeat mocks in libbeat, so that we don't cross reference others.

But, let me know what you think, I'll update accordingly. :)

[AndersonQ]

Oh, I meant moving just the mock you created and keeping the mockbeat where it is

[vinit-chauhan]

Oh I see, it makes sense then!

Let's wait for a bit if someone else has any comments, otherwise I'll update the code as per our discusstion.

[leehinman]

sorry for missing this. I like @AndersonQ suggestion.

[andrzej-stencel]

Sorry for the delay, sounds good to me 👍

[pierrehilbert]

/test

[pierrehilbert]

run docs-build

[vinit-chauhan]

Hey team - Apologies for the delay. I have moved the mock to the harvestor_test file as per our discussion.

Let me know if you require anything else.

[andrewkroh]

/test

[ycombinator]

run docs-build