Add Disk Queue configuration by leehinman · Pull Request #138 · elastic/elastic-agent-shipper

leehinman · 2022-10-17T17:59:01Z

What does this PR do?

In configuration file we will have:

queue:
  disk:
    path:
    max_size:
    segment_size:
    read_ahead:
    write_ahead:
    retry_interval:
    max_retry_interval:
    encryption_password:
    use_compression:

These are the same as beats except for
use_compression, and encryption_password. Which are new to
support encryption & compression.

Why is it important?

Need to be able to configure the disk queue to use it in the shipper.

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.md or CHANGELOG-developer.md.

How to test this PR locally

mage integrationTest

Related issues

Close add disk queue configuration to shipper configuration #119

mergify · 2022-10-17T17:59:50Z

This pull request is now in conflicts. Could you fix it? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

git fetch upstream
git checkout -b 119_disk_queue_configuration upstream/119_disk_queue_configuration
git merge upstream/main
git push upstream 119_disk_queue_configuration

mergify · 2022-10-17T17:59:51Z

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

- runs server in stand alone mode - checks that server starts - checks that it fails to start with bad config - checks that a message can be published and is written to output

elasticmachine · 2022-10-17T18:21:34Z

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

Start Time: 2022-10-25T15:27:34.226+0000
Duration: 12 min 45 sec

❕ Flaky test report

No test was executed to be analysed.

🤖 GitHub comments

Expand to view the GitHub comments

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

/test : Re-trigger the build.

- config options taken from beats - add `use_compression` options - add `use_encryption` option - add `encryption_password` option for stand alone and testing, elastic-agent use case will query for encryption key (to do). Closes elastic#119

cmacknz

Generally looks good, a few questions about tests and how we should get the encryption key.

Please review elastic/elastic-agent#1527 and make sure the configuration there is in sync with this one. Likely this we have another variant of the shared configuration problem between the agent and the shipper, where we have no way to guarantee the two configs stay in sync yet.

queue/config.go

queue/queue_test.go

integration/integration_test.go

queue/config.go

fearful-symmetry · 2022-10-20T22:04:19Z

integration/integration_test.go

-		"string": unique,
-		"number": 42,
+	events, err := createEvents([]string{unique})
+	if err != nil {


Might be best to use the "github.com/stretchr/testify/require" library we use everywhere else; will clean up the tests at least.

reworked with require, also added an environment Fatalf, so when the test fails you see the stdout & stderr so you have some idea what happened.

fearful-symmetry · 2022-10-20T22:17:50Z

integration/integration_test.go

-		Fields:   sampleValues,
-	}
-	events := []*messages.Event{e}
+	client, err := env.NewClient("localhost:50052")


A little worried that the hard-coded ports are going to collide with something, but it looks like the rest of the tests already do that, so unless you're particularly interested in modifying the framework to assign random free ports, that's probably a task for another day.

yeah it is a risk. Going to wait and see if it is a problem. If it is, my guess is we should move to Docker for starting the Shipper.

leehinman requested a review from a team as a code owner October 17, 2022 17:59

leehinman requested review from cmacknz and fearful-symmetry and removed request for a team October 17, 2022 17:59

mergify bot assigned leehinman Oct 17, 2022

leehinman added enhancement New feature or request Team:Elastic-Agent-Data-Plane Label for the Agent Data Plane team labels Oct 17, 2022

leehinman added 2 commits October 17, 2022 13:08

Add basic integration test

7304bbd

- runs server in stand alone mode - checks that server starts - checks that it fails to start with bad config - checks that a message can be published and is written to output

add context to environment

4fac152

leehinman force-pushed the 119_disk_queue_configuration branch from 276e948 to aac1a29 Compare October 17, 2022 18:08

leehinman force-pushed the 119_disk_queue_configuration branch 2 times, most recently from 3663b99 to b402062 Compare October 18, 2022 13:41

Disk Queue configuration

31394c3

- config options taken from beats - add `use_compression` options - add `use_encryption` option - add `encryption_password` option for stand alone and testing, elastic-agent use case will query for encryption key (to do). Closes elastic#119

leehinman force-pushed the 119_disk_queue_configuration branch from b402062 to 31394c3 Compare October 18, 2022 14:12

leehinman requested a review from faec October 18, 2022 15:25

pierrehilbert removed the request for review from faec October 19, 2022 12:13

cmacknz reviewed Oct 20, 2022

View reviewed changes

queue/config.go Outdated Show resolved Hide resolved

queue/queue_test.go Show resolved Hide resolved

integration/integration_test.go Outdated Show resolved Hide resolved

queue/config.go Show resolved Hide resolved

fearful-symmetry reviewed Oct 20, 2022

View reviewed changes

address feedback

bacb596

leehinman force-pushed the 119_disk_queue_configuration branch from 444c219 to bacb596 Compare October 25, 2022 15:27

cmacknz approved these changes Oct 25, 2022

View reviewed changes

faec approved these changes Oct 26, 2022

View reviewed changes

leehinman merged commit 5a56a16 into elastic:main Oct 27, 2022

leehinman deleted the 119_disk_queue_configuration branch October 27, 2022 13:58

This was referenced Oct 31, 2022

Add code to request encryption key from elastic-agent in managed mode #121

Closed

Add option to supply disk queue encryption key #120

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Disk Queue configuration#138

Add Disk Queue configuration#138
leehinman merged 4 commits intoelastic:mainfrom
leehinman:119_disk_queue_configuration

leehinman commented Oct 17, 2022 •

edited

Loading

Uh oh!

mergify bot commented Oct 17, 2022

Uh oh!

mergify bot commented Oct 17, 2022

Uh oh!

elasticmachine commented Oct 17, 2022 •

edited

Loading

Build stats

Uh oh!

cmacknz left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

fearful-symmetry Oct 20, 2022

Uh oh!

leehinman Oct 25, 2022

Uh oh!

fearful-symmetry Oct 20, 2022

Uh oh!

leehinman Oct 25, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

leehinman commented Oct 17, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this PR do?

Why is it important?

Checklist

How to test this PR locally

Related issues

Uh oh!

mergify bot commented Oct 17, 2022

Uh oh!

mergify bot commented Oct 17, 2022

Uh oh!

elasticmachine commented Oct 17, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

💚 Build Succeeded

Build stats

❕ Flaky test report

🤖 GitHub comments

Uh oh!

cmacknz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

fearful-symmetry Oct 20, 2022

Choose a reason for hiding this comment

Uh oh!

leehinman Oct 25, 2022

Choose a reason for hiding this comment

Uh oh!

fearful-symmetry Oct 20, 2022

Choose a reason for hiding this comment

Uh oh!

leehinman Oct 25, 2022

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

leehinman commented Oct 17, 2022 •

edited

Loading

elasticmachine commented Oct 17, 2022 •

edited

Loading