GitHub-Issue#2778: Added CouldWatchLogsService, Tests and RetransmissionException #3023
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…pensearch-project#2910) Create Elasticsearch client, implement search and pit apis for ElasticsearchAccessor Signed-off-by: Taylor Gray <[email protected]> Signed-off-by: Marcos Gonzalez Mayedo <[email protected]>
Added Config Files for CloudWatchLogs Sink. Signed-off-by: Marcos Gonzalez Mayedo <[email protected]>
… syntax) Signed-off-by: Marcos Gonzalez Mayedo <[email protected]>
Added default params for back_off and log_send_interval alongside test cases for ThresholdConfig. Signed-off-by: Marcos Gonzalez Mayedo <[email protected]>
Signed-off-by: Marcos Gonzalez Mayedo <[email protected]>
…ith AwsConfig. Signed-off-by: Marcos Gonzalez Mayedo <[email protected]>
…mer and params to AwsConfig Signed-off-by: Marcos Gonzalez Mayedo <[email protected]>
…tive mapping to tests CwlSink Signed-off-by: Marcos Gonzalez Mayedo <[email protected]>
…xRequestSize Signed-off-by: Marcos Gonzalez Mayedo <[email protected]>
Signed-off-by: Marcos Gonzalez Mayedo <[email protected]>
Signed-off-by: Marcos Gonzalez Mayedo <[email protected]>
Signed-off-by: Marcos Gonzalez Mayedo <[email protected]>
Signed-off-by: Marcos Gonzalez Mayedo <[email protected]>
Signed-off-by: Marcos Gonzalez Mayedo <[email protected]>
Signed-off-by: Marcos Gonzalez Mayedo <[email protected]>
…t the plugin use case more Signed-off-by: Marcos Gonzalez Mayedo <[email protected]>
Signed-off-by: Marcos Gonzalez Mayedo <[email protected]>
Signed-off-by: Marcos Gonzalez Mayedo <[email protected]>
…, made fixes to gradle file to include catalog Signed-off-by: Marcos Gonzalez Mayedo <[email protected]>
Signed-off-by: Marcos Gonzalez Mayedo <[email protected]>
…sionLimitException Signed-off-by: Marcos Gonzalez Mayedo <[email protected]>
Signed-off-by: Marcos Gonzalez Mayedo <[email protected]>
Signed-off-by: Marcos Gonzalez Mayedo <[email protected]>
Signed-off-by: Marcos Gonzalez Mayedo <[email protected]>
…ethods Signed-off-by: Marcos Gonzalez Mayedo <[email protected]>
MaGonzalMayedo
force-pushed
the
adding_cloud_watch_logs_service
branch
2 times, most recently
from
42a26c1
to
3ba1011
Compare
added 3 commits
July 25, 2023 13:21
…ce and Dispatcher and modified backOffTimeBase Signed-off-by: Marcos Gonzalez Mayedo <[email protected]>
Signed-off-by:Marcos Gonzalez Mayedo <[email protected]> Signed-off-by: Marcos Gonzalez Mayedo <[email protected]>
…, and added logString pass in parameter for staging log events. Signed-off-by: Marcos Gonzalez Mayedo <[email protected]>
MaGonzalMayedo
force-pushed
the
adding_cloud_watch_logs_service
branch
from
3ba1011
to
26f18a1
Compare
dlvenable
previously approved these changes
Jul 25, 2023
markkuhn
reviewed
Jul 25, 2023
| .backOffTimeBase(backOffTimeBase) | ||
| .build(); | ||
|
|
||
| asyncExecutor.execute(newTaskDispatcher); |
There was a problem hiding this comment.
I dont think creating an entire CloudWatchLogsDispatcher is the way to go. This class is fairly big and includes a lot of stuff we can run on the same thread and stuff we don't need to re-create all the time.
There was a problem hiding this comment.
Agreed, I will try to minimize the the requirements for a log publisher. This way we can have more lightweight objects running on each thread.
…ure (performance enhancement) Signed-off-by: Marcos Gonzalez Mayedo <[email protected]>
paggynie
reviewed
Jul 26, 2023
| addToBuffer(log, logString); | ||
| } | ||
|
|
||
| bufferLock.unlock(); |
There was a problem hiding this comment.
please put behind finally block
https://docs.oracle.com/javase/8/docs/api/java/util/concurrent/locks/ReentrantLock.html
There was a problem hiding this comment.
Thank you! I missed this while reading the documentation, I have made the change in the latest revision.
MaGonzalMayedo
force-pushed
the
adding_cloud_watch_logs_service
branch
from
27ba715
to
807c5f8
Compare
…ition of Uploader, introduced simple multithread tests for CloudWatchLogsService Signed-off-by: Marcos Gonzalez Mayedo <[email protected]>
MaGonzalMayedo
force-pushed
the
adding_cloud_watch_logs_service
branch
from
807c5f8
to
070beef
Compare
added 2 commits
July 27, 2023 16:17
…tests to the CloudWatchLogsService class Signed-off-by: Marcos Gonzalez Mayedo <[email protected]>
…d scale, and refactoring changes for better code syntax (renaming, refactoring methods for conciseness, etc...) Signed-off-by: Marcos Gonzalez Mayedo <[email protected]>
MaGonzalMayedo
force-pushed
the
adding_cloud_watch_logs_service
branch
from
16441d3
to
69320ec
Compare
Signed-off-by: Marcos Gonzalez Mayedo <[email protected]>
markkuhn
reviewed
Jul 28, 2023
...s/src/main/java/org/opensearch/dataprepper/plugins/sink/client/CloudWatchLogsDispatcher.java
Outdated
Show resolved
Hide resolved
markkuhn
reviewed
Jul 28, 2023
| } | ||
| } | ||
| } catch (InterruptedException e) { | ||
| LOG.warn("Got interrupted while waiting!"); |
There was a problem hiding this comment.
Better log needed here. This doesn't tell the customer anything. Interrupted by what? stacktrace? while waiting for what? in what class?
There was a problem hiding this comment.
Thank you, I have added a more appropriate message alongside the actual thrown error message.
markkuhn
approved these changes
Jul 28, 2023
…rch/dataprepper/plugins/sink/client/CloudWatchLogsDispatcher.java Co-authored-by: Mark Kuhn <[email protected]> Signed-off-by: Marcos Gonzalez Mayedo <[email protected]>
Signed-off-by: Marcos Gonzalez Mayedo <[email protected]>
dlvenable
approved these changes
Jul 31, 2023
|
🚀 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This change adds the CloudWatchLogsService which is the main module to be used by the sink to push logs to CloudWatchLogs Services. It includes a similar iterative style to checking Data-Prepper events for validity and proceeds to wrap them up in InputLogEvent structures before batch sending them to CloudWathcLogs. This change also introduces a new exception for denoting an error that was derived from this plugin (Runtime Exception).
Issues Resolved
This PR will work towards resolving CWL-Sink for Data-Prepper [Issue https://github.com/https://github.com/https://github.com//issues/2778]
Link: #2778 (comment)
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.