Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[loadgenerator] added loadgeneratorFloodHomepage flagd #1486

Merged

Conversation

bornav
Copy link
Contributor

@bornav bornav commented Mar 27, 2024

Changes

Added a loadgeneratorFloodHomepage flagd that when enabled floods the homepage with a large amount of traffic
Depending on the typical load of the sample, the load generation can be overwritten by changing the flagd variant number in the demo.flagd.json

Merge Requirements

For new features contributions please make sure you have completed the following
essential items:

  • CHANGELOG.md updated to document new feature additions
  • Appropriate documentation updates in the docs
  • Appropriate Helm chart updates in the helm-charts

Maintainers will not merge until the above have been completed. If you're unsure
which docs need to be changed ping the
@open-telemetry/demo-approvers.

Copy link

linux-foundation-easycla bot commented Mar 27, 2024

CLA Signed

The committers listed above are authorized under a signed CLA.

@bornav bornav force-pushed the Load-generator-Web-request-spamming branch 4 times, most recently from 6cec944 to 798bb9e Compare March 29, 2024 11:48
@bornav bornav marked this pull request as ready for review March 29, 2024 11:49
@bornav bornav requested a review from a team as a code owner March 29, 2024 11:49
@mviitane
Copy link
Member

I tried to run this PR just before the latest commit one hour ago and got it running, but now, for some reason it fails to checkout this branch. For the other PRs this is working:

$ gh pr checkout 1481
From github.com:open-telemetry/opentelemetry-demo
 * branch            refs/pull/1481/head -> FETCH_HEAD
Already up to date.

$ gh pr checkout 1486
From github.com:open-telemetry/opentelemetry-demo
 ! [rejected]        refs/pull/1486/head -> Load-generator-Web-request-spamming  (non-fast-forward)
failed to run git: exit status 1

$ gh pr checkout 1489
M	src/otelcollector/otelcol-config-extras.yml
Switched to branch 'snyk-upgrade-dd6a7df28d38a21b56af93ecc710f974'

@mviitane
Copy link
Member

With my first tests, I tried to flood with these settings to go a bit higher:

"variants": {
  "high":  1000,
  "medium": 100,
  "low":     10,
  "off":      0
},
"defaultVariant": "high"

You could provide more options than just 100.

In general, this is a really interesting scenario! With this, one can adjust the load levels outside the LoadGen UI.

@bornav
Copy link
Contributor Author

bornav commented Mar 29, 2024

I tried to run this PR just before the latest commit one hour ago and got it running, but now, for some reason it fails to checkout this branch. For the other PRs this is working:

$ gh pr checkout 1481
From github.com:open-telemetry/opentelemetry-demo
 * branch            refs/pull/1481/head -> FETCH_HEAD
Already up to date.

$ gh pr checkout 1486
From github.com:open-telemetry/opentelemetry-demo
 ! [rejected]        refs/pull/1486/head -> Load-generator-Web-request-spamming  (non-fast-forward)
failed to run git: exit status 1

$ gh pr checkout 1489
M	src/otelcollector/otelcol-config-extras.yml
Switched to branch 'snyk-upgrade-dd6a7df28d38a21b56af93ecc710f974'

could not replicate the issue from my side

With my first tests, I tried to flood with these settings to go a bit higher:

"variants": {
  "high":  1000,
  "medium": 100,
  "low":     10,
  "off":      0
},
"defaultVariant": "high"

You could provide more options than just 100.

In general, this is a really interesting scenario! With this, one can adjust the load levels outside the LoadGen UI.

purposely left it on a vague number, for a reason, we can't assume the load high will be the same for all environments.
Lets take 1000 as example, for the current env it more than enough, however for some other environment, the value 1000, may be insignificant

but if there is an agreement on the suggestion, I will gladly apply it

Copy link
Member

@mviitane mviitane left a comment

Choose a reason for hiding this comment

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

Sorry, it took a while, but I got rid of the problem in my branch by creating a new branch with
gh pr checkout 1486 -b Load-generator-Web-request-spamming-new

But this PR seems good to me.

Copy link
Member

@julianocosta89 julianocosta89 left a comment

Choose a reason for hiding this comment

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

2 minor adjusts, but looks good to me!

docker-compose.minimal.yml Show resolved Hide resolved
src/loadgenerator/locustfile.py Show resolved Hide resolved
@github-actions github-actions bot added docs-update-required Requires documentation update helm-update-required Requires an update to the Helm chart when released labels Apr 6, 2024
@bornav bornav force-pushed the Load-generator-Web-request-spamming branch 2 times, most recently from 6b1a94c to 404059e Compare April 8, 2024 12:48
@bornav bornav force-pushed the Load-generator-Web-request-spamming branch from 404059e to 1c4ed93 Compare April 8, 2024 12:56
@bornav bornav force-pushed the Load-generator-Web-request-spamming branch from 1c4ed93 to 7126e96 Compare April 8, 2024 13:17
@austinlparker
Copy link
Member

The integration test failure doesn't seem related to this PR, so I'll go ahead and merge.

@austinlparker austinlparker merged commit bbb99a1 into open-telemetry:main Apr 8, 2024
26 of 27 checks passed
AlexPSplunk pushed a commit to splunk/edu-opentelemetry-demo that referenced this pull request Jul 10, 2024
…y#1486)

* [loadgenerator] added loadgeneratorFloodHomepage flagd

* CHANGELOG

* Added FLAGD_PORT and depends on the dockercompose
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs-update-required Requires documentation update helm-update-required Requires an update to the Helm chart when released
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants