Skip to content

feat: add benchmark.py + small logprep-ng adaptions#939

Open
kaya-david wants to merge 5 commits intomainfrom
dev-logprep-ng-benchmark
Open

feat: add benchmark.py + small logprep-ng adaptions#939
kaya-david wants to merge 5 commits intomainfrom
dev-logprep-ng-benchmark

Conversation

@kaya-david
Copy link
Collaborator

@kaya-david kaya-david commented Feb 23, 2026

Description

  • Briefly summarize your changes in a few bullet points (can and should correspond to CHANGELOG.md)
  • Relates to #XXX (insert issue number here), if there is a corresponding GH issue

Assignee

  • The changes adhere to the contribution guidelines
  • I have performed a self-review of my code
  • My changes generate no new warnings (e.g. flake8/mypy/pytest/...) other than deprecations

Documentation

Code Quality

  • Patch test coverage > 95% and does not decrease
  • New code uses correct & specific type hints

How did you verify that the changes work in practice?

  • List of (preferably easy reproducible) tests including OS

Reviewer


The rendered docs for this PR can be found here.

@kaya-david kaya-david self-assigned this Feb 23, 2026
@kaya-david kaya-david requested a review from mhoff February 23, 2026 16:15
@kaya-david
Copy link
Collaborator Author

@mhoff Feel free to take a look and try it out.

@kaya-david kaya-david marked this pull request as draft February 23, 2026 16:16
@codecov-commenter
Copy link

codecov-commenter commented Feb 24, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.15%. Comparing base (0e19175) to head (7c28b90).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #939   +/-   ##
=======================================
  Coverage   96.14%   96.15%           
=======================================
  Files         217      217           
  Lines       14219    14219           
=======================================
+ Hits        13671    13672    +1     
+ Misses        548      547    -1     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@kaya-david kaya-david marked this pull request as ready for review February 24, 2026 08:38
@kaya-david kaya-david requested review from Pablu23 and removed request for mhoff February 24, 2026 08:54
@kaya-david kaya-david force-pushed the dev-logprep-ng-benchmark branch from ef53aca to dea212d Compare February 25, 2026 14:37
Pablu23
Pablu23 previously approved these changes Feb 25, 2026
Copy link
Collaborator

@Pablu23 Pablu23 left a comment

Choose a reason for hiding this comment

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

LGTM, not as strict of a review because its a script for now, but a few points. Like we talked about, change or leave them how you see fit

@kaya-david kaya-david force-pushed the dev-logprep-ng-benchmark branch from f922ddb to 7c28b90 Compare February 26, 2026 12:49
Copy link
Collaborator

@Pablu23 Pablu23 left a comment

Choose a reason for hiding this comment

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

Please remove old benchmark.sh as this is unneccessary now. Update PR Description and How you tested this, probably just a "run benchmark.py" see output. And after that this looks very good thank you for the work :)

Comment on lines +257 to +268
def opensearch_refresh(opensearch_url: str, processed_index: str) -> None:
"""
Force a refresh of the processed index so counts reflect recent writes.

Args:
opensearch_url: OpenSearch base URL.
processed_index: Index name to refresh.
"""
resp = requests.post(f"{opensearch_url}/{processed_index}/_refresh", timeout=10)
if resp.status_code == 404:
return
resp.raise_for_status()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Have you found this to be needed? If yes have you looked into the documentation of why that is? I understood the documentation, that this is for preloading in performance critical paths

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.

4 participants