Bugfix/issue 84 #86
Bugfix/issue 84 #86
Conversation
There was a problem hiding this comment.
Pull Request Overview
This pull request fixes a race condition in multi-worker scenarios by adding thread IDs to file paths and updates the plugin for Logstash 9.1+ compatibility.
- Addresses race condition by including thread ID in file paths to prevent multiple workers from writing to the same file
- Updates Logstash compatibility from version 6+ to 9.1+ with JDK 21 requirement
- Adds utility scripts for generating test data and improves code formatting consistency
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| lib/logstash/outputs/kusto.rb | Core fix adding thread ID to file paths and code formatting improvements |
| version | Version bump to 2.1.1 |
| README.md | Updated Logstash version requirement and added release notes |
| .github/workflows/build.yml | Updated CI to test against Logstash 9.1.0 |
| random-data.sh | New utility script for generating random Apache-style log data |
| random-data-json.sh | New utility script for generating random JSON log data |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
random-data.sh
Outdated
| random_size=$(( (RANDOM % 65535) + 1 )) | ||
| current_date_time=$(date '+%d/%b/%Y:%H:%M:%S %z') | ||
| echo "$random_ip - - [$current_date_time] \"GET /data.php HTTP/1.1\" 200 $random_size" | tee -a '/tmp/curllogs.txt' | ||
| sleep 0.0000001s |
There was a problem hiding this comment.
The sleep interval of 0.0000001s (100 nanoseconds) is extremely short and may not be respected by the system. Most systems have a minimum sleep resolution of milliseconds. Consider using a more reasonable interval like 0.001s (1ms) or remove the sleep entirely if high-frequency logging is desired.
| sleep 0.0000001s | |
| sleep 0.001s |
Copilot uses AI. Check for mistakes.
lib/logstash/outputs/kusto.rb
Outdated
|
|
||
| begin | ||
| return unless Dir.exist?(new_path) | ||
| @logger.info("Going to recover old files in path #{@new_path}") |
There was a problem hiding this comment.
The variable @new_path is not defined. This should be new_path (without the @ prefix) to reference the local variable defined on line 352.
| @logger.info("Going to recover old files in path #{@new_path}") | |
| @logger.info("Going to recover old files in path #{new_path}") |
Copilot uses AI. Check for mistakes.
ag-ramachandran
temporarily deployed
to
build
GitHub Actions
Inactive
ag-ramachandran
temporarily deployed
to
build
GitHub Actions
Inactive
This pull request introduces several improvements and fixes, most notably addressing a race condition in multi-worker scenarios by including the thread ID in file paths, updating documentation and dependencies for Logstash 9.1+, and adding utility scripts for generating random log data. Below are the most important changes:
Bug Fixes & File Handling Improvements:
lib/logstash/outputs/kusto.rbto prevent race conditions when multiple workers write to the same file, ensuring unique file names per thread. [1] [2]Documentation & Versioning:
README.mdto 9.1+ and noted the requirement for JDK 21.README.md, highlighting the thread ID fix and SDK version bump.versionfile.Dependency Updates:
Developer Utilities:
random-data.shandrandom-data-json.shscripts for generating random log data, useful for testing and development. [1] [2]