Xray performance test #330

okankoAMZ · 2023-08-14T18:40:40Z

Description of the issue

We were missing an automated performance gathering for CloudWatchAgent's new feature, traces.

Description of changes

This PR is adding a performance test for xray traces by adding a mock http server to minimize wasted cloud resources. This is added to the already built in performance test, and does no require any user input. The test will create a mock server automatically and run trace generation on it.

License

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Tests

Ran Performance Test and checked on Dynamo
https://github.com/aws/private-amazon-cloudwatch-agent-staging/actions/runs/6223948452/job/17362419385

Co-authored-by: name <[email protected]>

* Upgrade EKS Version and Fix EKS EMF Log JSON (aws#329) * Multi-System Trace Integration Test (aws#318) Co-authored-by: Jeffrey Chien <[email protected]> --------- Co-authored-by: Seth L <[email protected]> Co-authored-by: Jeffrey Chien <[email protected]>

Traces performance test

generator/test_case_generator.go

SaxyPandaBear · 2023-09-13T14:21:29Z

mockserver/README

@@ -0,0 +1,3 @@
+# Mocked Servers
+
+The servers themselves are very basic. They listen for a message and tend to set a flag to success upon receiving it. Typically, they are set up with two servers: one to mock the backend and one to report on the status of that mock (if endpoint is called, how many times, etc.). There's a built-in 15ms of latency between each message that's received. The status reporter will always be on port 8080 and support the `:8080/` (returns "healthcheck") and the `:8080/check-data` (returns "success" if mock has received messages) endpoints.


nit - I don't think that this documentation is worded in a way that helps people understand how it's used. I think a more procedural guide on running the mocked tests, or a step-by-step guide on what happens would illustrate it better. Avoid words like "typically".

I agree. There's room for improvement with the README.

mockserver/main.go

util/common/traces.go

validator/validators/basic/basic_validator.go

util/common/traces.go

test/xray/generator.go

terraform/performance/main.tf

jefchien · 2023-09-22T16:25:56Z

test/performance/trace/xray/parameters.yml

+
+# Metric that the test needs to validate
+metric_namespace: "CloudWatchAgentPerformance"
+metric_validation: 


So are we just validating that the metrics exist?

jefchien · 2023-09-22T16:27:07Z

test/performance/trace/xray/agent_config.json

+      },
+      "insecure": true,
+      "metrics": {
+        "namespace": "CloudWatchAgentPerformance",


Shouldn't this be in a different namespace since the metrics are specific to this test?

It wasn't for the other performance tests but I can change it

jefchien · 2023-09-22T16:29:24Z

mockserver/main.go

+	atomic.AddUint32(&ts.transactions, 1)
+
+	// Built-in latency
+	fmt.Printf("\033[31m Time: %d | data Received \033[0m \n", time.Now().Unix())


nit: Mixture of fmt and log.

jefchien · 2023-09-22T16:29:59Z

mockserver/main.go

+	if t > 0 {
+		message = SuccessMessage
+	}
+	fmt.Printf("\033[31m Time: %d | checkData msg: %s | %d\033[0m \n", time.Now().Unix(), message, t)


Is this log necessary?

mockserver/main.go

SaxyPandaBear · 2023-10-03T20:07:55Z

terraform/performance/main.tf

@@ -109,6 +109,13 @@ resource "null_resource" "validator_linux" {
  }
  provisioner "remote-exec" {
    inline = [
+      #mock server dependencies getting transfered.
+      "git clone --branch ${var.github_test_repo_branch} ${var.github_test_repo}",
+      "cd amazon-cloudwatch-agent-test && git checkout xray-performance-test", #@TODO remove before PR merge


is this TODO relevant?

yup! i am waiting for the PR to be ready before I remove it so I can test the code before the merge

This should use the github_test_repo_branch var. You can specify it in your test workflow.

SaxyPandaBear · 2023-10-03T20:09:16Z

util/common/traces/common/common.go

We should be cognizant about the file structure we are introducing. Does this need to be util/common/traces/common/common? What is the reason we are moving the file to begin with? Maybe a broader question, if it's supposed to be "common", why is it under /traces?

Still would like an answer for this

util/common/traces/generate.go

mockserver/main.go

jefchien · 2023-10-03T21:26:03Z

terraform/performance/main.tf

@@ -109,6 +109,13 @@ resource "null_resource" "validator_linux" {
  }
  provisioner "remote-exec" {
    inline = [
+      #mock server dependencies getting transfered.
+      "git clone --branch ${var.github_test_repo_branch} ${var.github_test_repo}",
+      "cd amazon-cloudwatch-agent-test && git checkout xray-performance-test", #@TODO remove before PR merge


This should use the github_test_repo_branch var. You can specify it in your test workflow.

util/common/traces/generate.go

mockserver/main.go

SaxyPandaBear · 2023-10-05T18:00:04Z

util/common/traces/common/common.go

Still would like an answer for this

util/common/traces/common/common.go

util/common/traces/base/base.go

jefchien

Some suggestions, but nothing blocking.

mockserver/README

jefchien · 2023-10-05T20:58:51Z

mockserver/README

@@ -0,0 +1,3 @@
+# Mocked Servers
+
+The servers themselves are very basic. They listen for a message and tend to set a flag to success upon receiving it. Typically, they are set up with two servers: one to mock the backend and one to report on the status of that mock (if endpoint is called, how many times, etc.). There's a built-in 15ms of latency between each message that's received. The status reporter will always be on port 8080 and support the `:8080/` (returns "healthcheck") and the `:8080/check-data` (returns "success" if mock has received messages) endpoints.


I agree. There's room for improvement with the README.

jefchien · 2023-10-05T20:59:41Z

mockserver/main.go

+}
+
+type TransactionPayload struct {
+	TransactionsPerMinute float64 `json:"GetNumberOfTransactionsPerMinute"`


nit: Try to keep the tag name and variable name the same.

mockserver/main.go

jefchien · 2023-10-05T21:12:24Z

mockserver/main.go

+}
+
+// Starts an HTTP server that receives request from validator only to verify the data ingestion
+func StartHttpServer() {


Is this used elsewhere? Why isn't this just main?

I'm not opposed to having the start function separate from main. It's redundant here, though.

I was using that for testing. I also like having a separate "main" function and have the argument flags in the func main instead.

validator/models/validation_config.go

SaxyPandaBear · 2023-10-06T19:41:50Z

mockserver/README.md

+
+The verifier component can be accessed via a listener on port 8080. It provides information about the transactions, including:
+
+- **Transactions per Minute:** You can obtain the transactions per minute by making a request to `/tpm`.


super mild nitpick, it would be nice to have an example cURL request, like

curl localhost:8080/tpm # the output is [ something]

This is really trivial, imo, but something that is really helpful to document.

okankoAMZ and others added 11 commits August 8, 2023 13:32

adding mock http server

9264098

added testing to httpserver

b10b434

added graceful shutdown

cdd5866

added check data

a58c332

added certificates for https

98639fc

Co-authored-by: name <[email protected]>

Merge branch 'aws:main' into traces-performance-test

c61614c

added xray performance test

9ee337e

generating traces

c61d69a

Merge branch 'xray-performance-test' into traces-performance-test

a9f3137

Merge pull request #5 from okankoAMZ/traces-performance-test

c25c1a2

Traces performance test

okankoAMZ added the github_actions Pull requests that update GitHub Actions code label Aug 14, 2023

okankoAMZ force-pushed the xray-performance-test branch 18 times, most recently from 4770cee to a38cbff Compare August 16, 2023 19:00

okankoAMZ force-pushed the xray-performance-test branch 3 times, most recently from 24f8e38 to 6491d06 Compare September 6, 2023 14:37

SaxyPandaBear reviewed Sep 13, 2023

View reviewed changes

okankoAMZ force-pushed the xray-performance-test branch from c70faa8 to de1f4ab Compare September 18, 2023 14:52

jefchien reviewed Sep 22, 2023

View reviewed changes

okankoAMZ force-pushed the xray-performance-test branch 3 times, most recently from 2c4c8c1 to 7f07122 Compare October 3, 2023 18:58

SaxyPandaBear reviewed Oct 3, 2023

View reviewed changes

jefchien reviewed Oct 3, 2023

View reviewed changes

okankoAMZ force-pushed the xray-performance-test branch 2 times, most recently from 96ade68 to 9b05e93 Compare October 5, 2023 16:49

okankoAMZ and others added 2 commits October 5, 2023 12:51

merging docker branch

0d6c843

Merge branch 'aws:main' into xray-performance-test

4fec4fd

okankoAMZ force-pushed the xray-performance-test branch from 9b05e93 to 3651eea Compare October 5, 2023 16:51

SaxyPandaBear reviewed Oct 5, 2023

View reviewed changes

okankoAMZ force-pushed the xray-performance-test branch 3 times, most recently from 8f0f12a to a6ea8bc Compare October 5, 2023 20:50

SaxyPandaBear previously approved these changes Oct 5, 2023

View reviewed changes

util/common/traces/base/base.go Show resolved Hide resolved

jefchien previously approved these changes Oct 5, 2023

View reviewed changes

Restructed Xray Test

8a81870

okankoAMZ dismissed stale reviews from jefchien and SaxyPandaBear via 8a81870 October 6, 2023 19:28

okankoAMZ force-pushed the xray-performance-test branch from a6ea8bc to 8a81870 Compare October 6, 2023 19:28

jefchien approved these changes Oct 6, 2023

View reviewed changes

SaxyPandaBear approved these changes Oct 6, 2023

View reviewed changes

okankoAMZ merged commit 7c94832 into aws:main Oct 6, 2023
2 checks passed

okankoAMZ deleted the xray-performance-test branch October 6, 2023 19:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Xray performance test #330

Xray performance test #330

okankoAMZ commented Aug 14, 2023 •

edited

Loading

SaxyPandaBear Sep 13, 2023

jefchien Oct 5, 2023

jefchien Sep 22, 2023

jefchien Sep 22, 2023

okankoAMZ Sep 28, 2023

jefchien Sep 22, 2023

jefchien Sep 22, 2023

SaxyPandaBear Oct 3, 2023

okankoAMZ Oct 3, 2023

jefchien Oct 3, 2023

SaxyPandaBear Oct 3, 2023

SaxyPandaBear Oct 5, 2023

jefchien Oct 3, 2023

SaxyPandaBear Oct 5, 2023

jefchien left a comment •

edited

Loading

jefchien Oct 5, 2023

jefchien Oct 5, 2023

jefchien Oct 5, 2023

SaxyPandaBear Oct 6, 2023

okankoAMZ Oct 6, 2023

SaxyPandaBear Oct 6, 2023

		@@ -0,0 +1,3 @@
		# Mocked Servers

		The servers themselves are very basic. They listen for a message and tend to set a flag to success upon receiving it. Typically, they are set up with two servers: one to mock the backend and one to report on the status of that mock (if endpoint is called, how many times, etc.). There's a built-in 15ms of latency between each message that's received. The status reporter will always be on port 8080 and support the `:8080/` (returns "healthcheck") and the `:8080/check-data` (returns "success" if mock has received messages) endpoints.


		The verifier component can be accessed via a listener on port 8080. It provides information about the transactions, including:

		- Transactions per Minute: You can obtain the transactions per minute by making a request to `/tpm`.

Xray performance test #330

Xray performance test #330

Conversation

okankoAMZ commented Aug 14, 2023 • edited Loading

Description of the issue

Description of changes

License

Tests

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

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

jefchien left a comment • edited Loading

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

okankoAMZ commented Aug 14, 2023 •

edited

Loading

jefchien left a comment •

edited

Loading