Skip to content

[libbeat] Cleaning up some unneeded helper types#31290

Merged
faec merged 8 commits into
elastic:mainfrom
faec:pipeline-cleanup-4
Apr 14, 2022
Merged

[libbeat] Cleaning up some unneeded helper types#31290
faec merged 8 commits into
elastic:mainfrom
faec:pipeline-cleanup-4

Conversation

@faec

@faec faec commented Apr 13, 2022

Copy link
Copy Markdown

What does this PR do?

Merges and simplifies some helper types in the publisher pipeline:

  • Removed pipelineEventer. The only purpose of this wrapper type was to implement the queue.ACKListener interface for Pipeline. However, the values in it were already in Pipeline, resulting in duplication of its fields and confusing ownership. The new code just implements the ACKListener interface directly on Pipeline.
  • Removed waitCloser. This type was just a wrapper around a wait group, seemingly to simplify sharing the outstanding event state between Pipeline and pipelineEventer. However, with pipelineEventer gone there's no need for this coordination. I replaced the former waitCloser field with an explicit wait group and removed the now-unused wrapper functions. Code that previously tested whether to report things based on whether waitCloser was nil now use the more descriptive waitOnClose flag which is initialized equivalently.
  • Removed the WaitOnClientClose value for WaitCloseMode. It caused some confusing checks, but it is unused (both in the sense that nothing refers to it and that there doesn't seem to have been any way to actually use it, since the waiting code was gated exclusively on the (very slightly) more common WaitOnPipelineClose mode.

None of these changes should cause any visible behavior changes in the pipeline.

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.

@faec faec added cleanup Team:Elastic-Agent-Data-Plane Label for the Agent Data Plane team labels Apr 13, 2022
@faec faec requested a review from a team as a code owner April 13, 2022 22:08
@faec faec requested review from cmacknz and kvch and removed request for a team April 13, 2022 22:08
@elasticmachine

Copy link
Copy Markdown
Contributor

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 Apr 13, 2022
@mergify mergify Bot assigned faec Apr 13, 2022
@elasticmachine

elasticmachine commented Apr 13, 2022

Copy link
Copy Markdown
Contributor

💚 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: 2022-04-14T11:19:11.464+0000

  • Duration: 64 min 16 sec

Test stats 🧪

Test Results
Failed 0
Passed 22299
Skipped 1935
Total 24234

💚 Flaky test report

Tests succeeded.

🤖 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!)

@faec faec merged commit 2f761cc into elastic:main Apr 14, 2022
@faec faec deleted the pipeline-cleanup-4 branch April 14, 2022 12:31
v1v added a commit to v1v/beats that referenced this pull request Apr 18, 2022
…er-tar-gz

* upstream/main: (139 commits)
  [Automation] Update elastic stack version to 8.3.0-c655cda8 for testing (elastic#31322)
  Define a queue metrics reporter interface  (elastic#31289)
  [Oracle Module] Change tablespace metricset collection period (elastic#31259)
  libbeat/reader/syslog: relax timestamp parsing to allow leading zero (elastic#31254)
  [Automation] Update elastic stack version to 8.3.0-55ba6f37 for testing (elastic#31311)
  [libbeat] Remove unused fields and functions in the memory queue (elastic#31302)
  [libbeat] Cleaning up some unneeded helper types (elastic#31290)
  Readme for kibana module (elastic#31276)
  [Automation] Update elastic stack version to 8.3.0-4be61f32 for testing (elastic#31296)
  x-pack/winlogbeat/module/routing/ingest: fix typo for channel name (elastic#31291)
  Small pipeline cleanup removing some unused data fields (elastic#31288)
  removing info log (elastic#30971)
  Simplify TLS config deserialization (elastic#31168)
  Detect new files under known paths in filestream input (elastic#31268)
  Add support for port mapping in docker hints (elastic#31243)
  Update qa-labels.yml (elastic#31260)
  libbeat: log debug for `proxy_url` and fixed docs (elastic#31130)
  [heartbeat][docs] Add note about ensuring correct index settings for uptime (elastic#31146)
  [Automation] Update elastic stack version to 8.3.0-2c8f9574 for testing (elastic#31256)
  [Filebeat] fix m365_defender pipeline bug (elastic#31227)
  ...
kush-elastic pushed a commit to kush-elastic/beats that referenced this pull request May 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cleanup Team:Elastic-Agent-Data-Plane Label for the Agent Data Plane team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants