Skip to content

Add test for the processor re-use issue#34870

Merged
rdner merged 3 commits intoelastic:mainfrom
rdner:processor-reuse-test
Mar 21, 2023
Merged

Add test for the processor re-use issue#34870
rdner merged 3 commits intoelastic:mainfrom
rdner:processor-reuse-test

Conversation

@rdner
Copy link
Copy Markdown
Member

@rdner rdner commented Mar 21, 2023 •
edited
Loading

What does this PR do?

It's a follow-up to #34761

This test makes sure that none of the critical configuration fields are re-used between instances of the pipeline client.

Why is it important?

So, we don't have regressions in the future.

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.

How to test this PR locally

I reverted the change made in #34761 and saw the new test failing:

Screenshot 2023-03-21 at 15 02 29

Related issues

rdner added 2 commits March 21, 2023 15:08
@rdner
Add test for the processor re-use issue
5e051d4 
It's a follow-up to elastic#34761

This test makes sure that none of the critical configuration fields
are re-used between instances of the pipeline client.
@rdner
Fix linter issues
bf1eae4
@rdner rdner added bug Team:Elastic-Agent-Data-Plane Label for the Agent Data Plane team labels Mar 21, 2023
@rdner rdner self-assigned this Mar 21, 2023
@rdner rdner requested a review from a team as a code owner March 21, 2023 14:15
@rdner rdner requested review from belimawr and faec and removed request for a team March 21, 2023 14:15
@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Mar 21, 2023

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

@botelastic botelastic bot added needs_team Indicates that the issue/PR needs a Team:* label and removed needs_team Indicates that the issue/PR needs a Team:* label labels Mar 21, 2023
@mergify
Copy link
Copy Markdown
Contributor

mergify bot commented Mar 21, 2023

This pull request does not have a backport label.
If this is a bug or security fix, could you label this PR @rdner? 🙏.
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

@jlind23 jlind23 requested a review from leehinman March 21, 2023 14:48
@rdner rdner added backport-7.17 Automated backport to the 7.17 branch with mergify backport-v8.6.0 Automated backport with mergify backport-v8.7.0 Automated backport with mergify labels Mar 21, 2023
faec
faec reviewed Mar 21, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@faec faec left a comment

Choose a reason for hiding this comment

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

Looks good, a couple requests around mocking style

@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Mar 21, 2023 •
edited by jenkins-beats-ci bot
Loading

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview preview

Expand to view the summary

Build stats

  • Start Time: 2023-03-21T15:53:14.007+0000

  • Duration: 69 min 9 sec

Test stats 🧪

Test Results
Failed 0
Passed 7581
Skipped 746
Total 8327

💚 Flaky test report

Tests succeeded.

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

  • /package : Generate the packages and run the E2E tests.

  • /beats-tester : Run the installation tests with beats-tester.

  • run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

@rdner rdner merged commit 3d917c8 into elastic:main Mar 21, 2023
@rdner rdner deleted the processor-reuse-test branch March 21, 2023 17:11
mergify bot pushed a commit that referenced this pull request Mar 21, 2023
@rdner @mergify
Add test for the processor re-use issue (#34870)
878da01 
It's a follow-up to #34761

This test makes sure that none of the critical configuration fields
are re-used between instances of the pipeline client.

(cherry picked from commit 3d917c8)

# Conflicts:
#	filebeat/channel/runner.go
#	filebeat/channel/runner_test.go
@mergify mergify bot mentioned this pull request Mar 21, 2023
Merged
mergify bot pushed a commit that referenced this pull request Mar 21, 2023
@rdner @mergify
Add test for the processor re-use issue (#34870)
c893981 
It's a follow-up to #34761

This test makes sure that none of the critical configuration fields
are re-used between instances of the pipeline client.

(cherry picked from commit 3d917c8)
mergify bot pushed a commit that referenced this pull request Mar 21, 2023
@rdner @mergify
Add test for the processor re-use issue (#34870)
3fbb36b 
It's a follow-up to #34761

This test makes sure that none of the critical configuration fields
are re-used between instances of the pipeline client.

(cherry picked from commit 3d917c8)

# Conflicts:
#	filebeat/channel/runner.go
This was referenced Mar 21, 2023
Merged
Merged
rdner added a commit that referenced this pull request Mar 21, 2023
@mergify @rdner
Add test for the processor re-use issue (#34870) (#34881)
d920e81 
It's a follow-up to #34761

This test makes sure that none of the critical configuration fields
are re-used between instances of the pipeline client.

(cherry picked from commit 3d917c8)

Co-authored-by: Denis <denis.rechkunov@elastic.co>
rdner added a commit that referenced this pull request Mar 21, 2023
@mergify @rdner
[8.6](backport #34870) Add test for the processor re-use issue (#34880)
5842767 
* Add test for the processor re-use issue (#34870)

It's a follow-up to #34761

This test makes sure that none of the critical configuration fields
are re-used between instances of the pipeline client.

(cherry picked from commit 3d917c8)

# Conflicts:
#	filebeat/channel/runner.go

* Resolve conflicts

---------

Co-authored-by: Denis <denis.rechkunov@elastic.co>
rdner added a commit that referenced this pull request Mar 21, 2023
@mergify @rdner
[7.17](backport #34870) Add test for the processor re-use issue (#34879)
838902e 
* Add test for the processor re-use issue (#34870)

It's a follow-up to #34761

This test makes sure that none of the critical configuration fields
are re-used between instances of the pipeline client.

(cherry picked from commit 3d917c8)

# Conflicts:
#	filebeat/channel/runner.go
#	filebeat/channel/runner_test.go

* Resolve conflicts

---------

Co-authored-by: Denis <denis.rechkunov@elastic.co>
chrisberkhout pushed a commit that referenced this pull request Jun 1, 2023
@rdner @chrisberkhout
Add test for the processor re-use issue (#34870)
bd933ac 
It's a follow-up to #34761

This test makes sure that none of the critical configuration fields
are re-used between instances of the pipeline client.
@haesbaert
Copy link
Copy Markdown
Contributor

haesbaert commented Feb 18, 2025

There's a bug in this test reveled by a fix in testify 1.10, which was incorrectly not checking if the arguments are pointers:
stretchr/testify@118fb83

The test doesn't pass pointers, I'm assuming you wanted NotEqualf instead of NotSamef, as the pointers will never be the same since they're on different storage.

This is blocking me at #42032

@haesbaert haesbaert mentioned this pull request Feb 18, 2025
Merged
6 tasks
@haesbaert
Copy link
Copy Markdown
Contributor

haesbaert commented Feb 18, 2025

Tentative fix at ee6a27f

With that the Meta comparison fails, likely both maps are nil, processors comparison also fail:
https://buildkite.com/elastic/filebeat/builds/13588#01951802-0522-4b17-ac09-4da08884a10d

@haesbaert haesbaert mentioned this pull request Feb 18, 2025
Merged
3 tasks
This was referenced Mar 4, 2025
Merged
Merged
@mergify mergify bot mentioned this pull request Mar 18, 2025
Merged
3 tasks
@mergify mergify bot mentioned this pull request Apr 18, 2025
Merged
3 tasks
@mergify mergify bot mentioned this pull request Nov 13, 2025
Merged
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@faec faec faec approved these changes

@belimawr belimawr Awaiting requested review from belimawr belimawr is a code owner automatically assigned from elastic/elastic-agent-data-plane

@leehinman leehinman Awaiting requested review from leehinman

Assignees

@rdner rdner

Labels

backport-7.17 Automated backport to the 7.17 branch with mergify backport-v8.6.0 Automated backport with mergify backport-v8.7.0 Automated backport with mergify bug Team:Elastic-Agent-Data-Plane Label for the Agent Data Plane team

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Cover possible processor re-use with tests

4 participants

@rdner @elasticmachine @haesbaert @faec