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

Add load-tester #487

Merged
merged 14 commits into from
Feb 6, 2024
Merged

Add load-tester #487

merged 14 commits into from
Feb 6, 2024

Conversation

ppcad
Copy link
Collaborator

@ppcad ppcad commented Dec 1, 2023

This PR adds two generator for load tests, both working against two different targets. The first one can send events to kafka and the second one can send events to an http endpoint.
Both generators are integrated into the new CLI.

@ppcad ppcad requested a review from ekneg54 December 1, 2023 08:28
@ppcad ppcad self-assigned this Dec 1, 2023
@codecov-commenter
Copy link

codecov-commenter commented Dec 1, 2023

Codecov Report

Attention: 82 lines in your changes are missing coverage. Please review.

Comparison is base (bf0e41f) 92.14% compared to head (c2668f1) 91.47%.
Report is 2 commits behind head on main.

Files Patch % Lines
logprep/event_generator/kafka/process_runner.py 37.25% 32 Missing ⚠️
logprep/event_generator/kafka/kafka_connector.py 73.17% 11 Missing ⚠️
logprep/event_generator/http/output.py 81.08% 7 Missing ⚠️
logprep/event_generator/kafka/run_load_tester.py 61.11% 7 Missing ⚠️
logprep/event_generator/kafka/document_sender.py 86.36% 6 Missing ⚠️
logprep/event_generator/kafka/configuration.py 90.38% 5 Missing ⚠️
logprep/event_generator/kafka/document_loader.py 92.85% 4 Missing ⚠️
logprep/event_generator/http/manipulator.py 93.18% 3 Missing ⚠️
logprep/event_generator/http/controller.py 96.36% 2 Missing ⚠️
logprep/event_generator/kafka/logger.py 88.88% 2 Missing ⚠️
... and 2 more
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #487      +/-   ##
==========================================
- Coverage   92.14%   91.47%   -0.67%     
==========================================
  Files         130      143      +13     
  Lines        9524    10221     +697     
==========================================
+ Hits         8776     9350     +574     
- Misses        748      871     +123     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ppcad ppcad marked this pull request as ready for review December 1, 2023 08:37
@ppcad ppcad marked this pull request as draft December 1, 2023 12:25
@dtrai2
Copy link
Collaborator

dtrai2 commented Jan 24, 2024

I removed the cli reimplementation from this pull request again, as I will merge it separately with #513.

@dtrai2
Copy link
Collaborator

dtrai2 commented Jan 26, 2024

so we have added another generator that can send events to a http endpoint. @ppcad could you please do a review on this PR and check how we have integrated both generators into the new cli.

As you can't approve your own pull request could you just give your feedback here as a comment. Then if everything is okay @djkhl can give her final approve.

@dtrai2 dtrai2 requested review from djkhl and removed request for ekneg54 January 26, 2024 08:56
@dtrai2 dtrai2 marked this pull request as ready for review January 26, 2024 09:09
Copy link
Collaborator Author

@ppcad ppcad left a comment

Choose a reason for hiding this comment

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

@djkhl
I made some comments in code regarding the load tester.
Furthermore, I'm not sure how to use the event generator.

Using a config.yaml like this:

target_path: /some_endpoint
timestamps:
  - key: TIMESTAMP_FIELD_1
    format: "%Y%m%d"
  - key: TIMESTAMP_FIELD_1
    format: "%H%M%S"
    time_shift: "+0200"  # Optional, sets time shift in hours and minutes, if needed ([+-]HHMM)

yields the following error:

requests.exceptions.InvalidSchema: No connection adapters were found for '127.0.0.1:9000/some_endpoint'

However, the endpoint works if I use curl with the following command:

curl -X POST http://127.0.0.1:9000/some_endpoint -H 'Content-Type: application/json' -d '{"foo": "bar"}' --insecure


def run(self):
"""Start function for the load-tester"""
config = load_config(self.config, self.file)
Copy link
Collaborator Author

@ppcad ppcad Jan 29, 2024

Choose a reason for hiding this comment

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

self.file is a bool (comes from --file via click, to activate using files), but it is used as source_file path instead of the source_file configuration option.
Therefore, the load tester is not working with a source file.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I just turned the cli --file flag into an actual path. Can you please try it out again?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thank you it works.
I did notice however that "--file" only works if a connection to Kafka exists, even if the documents are not retrieved from Kafka.
I believe that's a problem my original commit already had.
Maybe this could be changed in the future.

README.md Show resolved Hide resolved
@dtrai2
Copy link
Collaborator

dtrai2 commented Jan 31, 2024

yields the following error:

requests.exceptions.InvalidSchema: No connection adapters were found for '127.0.0.1:9000/some_endpoint'

Looking at the error I assume you called the generator with --target-domain 127.0.0.1:9000? It actually should include the schema http/https as well. In order to reduce this confusion I have just changed the cli option target-domain to target-url. Can you please check if that was the mistake and tell me if that will reduce future confusions?

@ppcad
Copy link
Collaborator Author

ppcad commented Feb 1, 2024

yields the following error:
requests.exceptions.InvalidSchema: No connection adapters were found for '127.0.0.1:9000/some_endpoint'

Looking at the error I assume you called the generator with --target-domain 127.0.0.1:9000? It actually should include the schema http/https as well. In order to reduce this confusion I have just changed the cli option target-domain to target-url. Can you please check if that was the mistake and tell me if that will reduce future confusions?

Thank you for the clarification, it works.

Everything looks good to me, except that a Kafka connection is required even if using "--file".

@ppcad ppcad requested a review from dtrai2 February 2, 2024 05:53
@ekneg54
Copy link
Collaborator

ekneg54 commented Feb 6, 2024

rebased from main

Copy link
Collaborator

@djkhl djkhl left a comment

Choose a reason for hiding this comment

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

looks good!

@ekneg54 ekneg54 merged commit d1717d6 into main Feb 6, 2024
10 checks passed
@ekneg54 ekneg54 deleted the dev-load-tester branch February 7, 2024 14:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants