functionaltest: Add one re-route processor test #15960

ericywl · 2025-03-03T12:07:05Z

Changes

Based on #15951

Add CreateRerouteProcessors and PerformManualRollovers to ES client
Add re-route processor test for 8.13.4 -> 8.16.0
Parallelize functional tests

How to test these changes

Run functional test on this branch.

Test run: https://github.com/elastic/apm-server/actions/runs/13675996351

mergify · 2025-03-03T12:07:46Z

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

backport-7.17 is the label to automatically backport to the 7.17 branch.
backport-8./d is the label to automatically backport to the 8./d branch. /d is the digit.
backport-9./d is the label to automatically backport to the 9./d branch. /d is the digit.
backport-8.x is the label to automatically backport to the 8.x branch.
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.

endorama · 2025-03-04T17:21:22Z

Given that this PR is in draft, but is also doing a refactoring that may block other work (see #16011 for example) can you please extract the refactoring in a separate PR so we can merge it faster?

ericywl · 2025-03-05T02:53:06Z

Given that this PR is in draft, but is also doing a refactoring that may block other work (see #16011 for example) can you please extract the refactoring in a separate PR so we can merge it faster?

The refactoring is in #15951.

mergify · 2025-03-05T02:53:45Z

This pull request is now in conflicts. Could you fix it @ericywl? 🙏
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 functionaltests-reroute-processor upstream/functionaltests-reroute-processor
git merge upstream/main
git push upstream functionaltests-reroute-processor

…actor

mergify · 2025-03-05T03:05:17Z

This pull request is now in conflicts. Could you fix it @ericywl? 🙏
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 functionaltests-reroute-processor upstream/functionaltests-reroute-processor
git merge upstream/functionaltests-refactor
git push upstream functionaltests-reroute-processor

mergify · 2025-03-05T11:06:37Z

This pull request is now in conflicts. Could you fix it @ericywl? 🙏
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 functionaltests-reroute-processor upstream/functionaltests-reroute-processor
git merge upstream/functionaltests-refactor
git push upstream functionaltests-reroute-processor

mergify · 2025-03-05T12:21:55Z

This pull request is now in conflicts. Could you fix it @ericywl? 🙏
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 functionaltests-reroute-processor upstream/functionaltests-reroute-processor
git merge upstream/main
git push upstream functionaltests-reroute-processor

…oute-processor

…kipped ds

ericywl · 2025-03-06T07:27:05Z

Moved back to draft since it's failing.

endorama · 2025-03-06T08:18:30Z

functionaltests/9_0_test.go

@@ -22,6 +22,7 @@ import (
 )

 func TestUpgrade_8_18_0_to_9_0_0(t *testing.T) {
+	t.Parallel()


let's do this in a separate PR

endorama · 2025-03-06T08:20:51Z

functionaltests/internal/esclient/client.go

+// CreateRerouteProcessors creates re-route processors with the specified namespace for logs, metrics and traces.
+//
+// Refer to https://www.elastic.co/guide/en/elasticsearch/reference/current/reroute-processor.html.
+func (c *Client) CreateRerouteProcessors(ctx context.Context, name string) error {


I would suggest to name this:

Suggested change

func (c *Client) CreateRerouteProcessors(ctx context.Context, name string) error {

func (c *Client) CreateRerouteIngestPipeline(ctx context.Context, name string) error {

To make this method more generic, what do you think about having a CreateIngestPipeline method that accepts a []types.ProcessorContainer?

endorama · 2025-03-06T08:22:01Z

functionaltests/internal/esclient/client.go

+	return nil
+}
+
+func (c *Client) performManualRollover(ctx context.Context, dataStream string) error {


If this method is not used anywhere else, let's just include it in the public one. It reduces the API surface we will need to maintain.

If this is done to have a generic implementation (that does not use fixed data stream names for example) I would suggest to promote this as the public method on this struct and create a test helper func in the main tests folder.

endorama · 2025-03-06T08:24:45Z

functionaltests/main_test.go

 // assertDocCount check if specified document count is equal to expected minus
 // documents count from a previous state.
-func assertDocCount(t *testing.T, docsCount, previous, expected esclient.APMDataStreamsDocCount) {
+func assertDocCount(t *testing.T, docsCount, previous, expected esclient.APMDataStreamsDocCount, skippedDataStreams []string) {


What's the benefit of introducing skippedDataStreams?

In my case, it's so that we can assert if there are some unexpected data streams. It was helpful for the reroute test as it helped identify that I forgot to change the expected data streams to rerouted (i.e. the assertions weren't happening because it was trying to find default).

endorama · 2025-03-06T08:27:44Z

functionaltests/single_upgrade_test.go

@@ -34,6 +34,37 @@ import (
 	"github.com/elastic/apm-server/functionaltests/internal/terraform"
 )

+type dependencies struct {


What do you think of adding this fields to the test struct? As these are part of the test itself, I see this struct as tightly coupled with the test one, and I think this would be better expressed by having these fields there.

endorama · 2025-03-06T08:28:42Z

functionaltests/single_upgrade_test.go

+	return newDefaultConfigWithNamespace(defaultNamespace)
+}
+
+type additionalFn func(t *testing.T, ctx context.Context, cfg *config, deps dependencies) (continueTest bool)


What is the benefit of the pointer to config here? Do you expect this func to change the configuration?

In the case of reroute, I used this config to change the expected docs and skipped data streams midtest.

ericywl and others added 11 commits February 28, 2025 18:53

Add some comments to section off code; Add pre ingestion setup

d940101

Refactor some functionaltests and add comments

30a82e1

Optimize imports

85cdd2f

Fix README.md

d2dd4a8

Fix README.md again

fcca77d

Run fmt

82099aa

Update ec_deployment .gitignore

0fcca93

Revert versions from test name change and remove post-upgrade setup

8ede83e

Add back comment

f6a5dda

Replace time.Now().Sub() with time.Since()

ff83058

Merge branch 'main' into functionaltests-refactor

7f20148

ericywl added the backport-skip Skip notification from the automated backport with mergify label Mar 4, 2025

Add gitignore in functionaltests

f57b350

ericywl force-pushed the functionaltests-reroute-processor branch from c5c9d1f to 7abe748 Compare March 4, 2025 07:24

ericywl changed the base branch from main to functionaltests-refactor March 5, 2025 02:54

Merge remote-tracking branch 'upstream/main' into functionaltests-ref…

7d58e7b

…actor

ericywl force-pushed the functionaltests-reroute-processor branch from 06c0c1b to 469b30b Compare March 5, 2025 03:00

Remove CreateAPMAPIKey

24c870b

ericywl force-pushed the functionaltests-reroute-processor branch from 469b30b to 0eb5de2 Compare March 5, 2025 03:05

Run fmt

177df13

ericywl force-pushed the functionaltests-reroute-processor branch from 0eb5de2 to 53468ac Compare March 5, 2025 03:12

Resolve review comments

9d0466f

ericywl added 2 commits March 5, 2025 19:07

Move setup log into pre-ingestion setup block

0817d1c

Add CreateRerouteProcessors

6974180

ericywl added 8 commits March 5, 2025 19:11

Add reroute processor test

d0a0810

Add test parallel

0d58157

Add comments

0483999

Update outputs.tf in reroute test

a04aabd

Update test to add manual rollover

87ad3f6

Update comment were -> are

51578bf

Add check for unexpected documents

bc3d89f

Add logs to setups

d26c69a

ericywl force-pushed the functionaltests-reroute-processor branch from a60e228 to d26c69a Compare March 5, 2025 11:15

Update comment into log

ba15998

Base automatically changed from functionaltests-refactor to main March 5, 2025 12:21

Merge remote-tracking branch 'upstream/main' into functionaltests-rer…

5d5cef4

…oute-processor

ericywl requested a review from endorama March 6, 2025 03:03

Merge branch 'main' into functionaltests-reroute-processor

c01e150

ericywl marked this pull request as ready for review March 6, 2025 03:04

ericywl requested a review from a team as a code owner March 6, 2025 03:04

Move post upgrade setup; Change config to include expected docs and s…

2617ee9

…kipped ds

ericywl marked this pull request as draft March 6, 2025 07:26

endorama reviewed Mar 6, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

functionaltest: Add one re-route processor test #15960

functionaltest: Add one re-route processor test #15960

ericywl commented Mar 3, 2025 •

edited

Loading

mergify bot commented Mar 3, 2025

endorama commented Mar 4, 2025

ericywl commented Mar 5, 2025 •

edited

Loading

mergify bot commented Mar 5, 2025

mergify bot commented Mar 5, 2025

mergify bot commented Mar 5, 2025

mergify bot commented Mar 5, 2025

ericywl commented Mar 6, 2025

endorama Mar 6, 2025

endorama Mar 6, 2025

endorama Mar 6, 2025

endorama Mar 6, 2025

ericywl Mar 6, 2025

endorama Mar 6, 2025

endorama Mar 6, 2025

ericywl Mar 6, 2025

	func (c *Client) CreateRerouteProcessors(ctx context.Context, name string) error {
	func (c *Client) CreateRerouteIngestPipeline(ctx context.Context, name string) error {

functionaltest: Add one re-route processor test #15960

Are you sure you want to change the base?

functionaltest: Add one re-route processor test #15960

Conversation

ericywl commented Mar 3, 2025 • edited Loading

Changes

How to test these changes

mergify bot commented Mar 3, 2025

endorama commented Mar 4, 2025

ericywl commented Mar 5, 2025 • edited Loading

mergify bot commented Mar 5, 2025

mergify bot commented Mar 5, 2025

mergify bot commented Mar 5, 2025

mergify bot commented Mar 5, 2025

ericywl commented Mar 6, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ericywl commented Mar 3, 2025 •

edited

Loading

ericywl commented Mar 5, 2025 •

edited

Loading