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

GitHub-Issue#2778: Added CouldWatchLogsService, Tests and RetransmissionException #3023

Merged

Conversation

MaGonzalMayedo
Copy link
Contributor

@MaGonzalMayedo MaGonzalMayedo commented Jul 12, 2023

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

  • New functionality includes testing.
  • New functionality has been documented.
    • New functionality has javadoc added
  • Commits are signed with a real name per the DCO

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.

graytaylor0 and others added 28 commits July 3, 2023 12:56
…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]>
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]>
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]>
…, 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]>
@MaGonzalMayedo MaGonzalMayedo force-pushed the adding_cloud_watch_logs_service branch 2 times, most recently from 42a26c1 to 3ba1011 Compare July 25, 2023 20:14
Marcos Gonzalez Mayedo 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 MaGonzalMayedo force-pushed the adding_cloud_watch_logs_service branch from 3ba1011 to 26f18a1 Compare July 25, 2023 20:22
dlvenable
dlvenable previously approved these changes Jul 25, 2023
Copy link
Member

@dlvenable dlvenable left a comment

Choose a reason for hiding this comment

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

This looks good to me. Thanks @MaGonzalMayedo !

.backOffTimeBase(backOffTimeBase)
.build();

asyncExecutor.execute(newTaskDispatcher);

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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]>
addToBuffer(log, logString);
}

bufferLock.unlock();

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you! I missed this while reading the documentation, I have made the change in the latest revision.

…ition of Uploader, introduced simple multithread tests for CloudWatchLogsService

Signed-off-by: Marcos Gonzalez Mayedo <[email protected]>
@MaGonzalMayedo MaGonzalMayedo force-pushed the adding_cloud_watch_logs_service branch from 807c5f8 to 070beef Compare July 27, 2023 22:49
Marcos Gonzalez Mayedo 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 MaGonzalMayedo force-pushed the adding_cloud_watch_logs_service branch from 16441d3 to 69320ec Compare July 28, 2023 18:56
Signed-off-by: Marcos Gonzalez Mayedo <[email protected]>
}
}
} catch (InterruptedException e) {
LOG.warn("Got interrupted while waiting!");

Choose a reason for hiding this comment

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

Better log needed here. This doesn't tell the customer anything. Interrupted by what? stacktrace? while waiting for what? in what class?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you, I have added a more appropriate message alongside the actual thrown error message.

MaGonzalMayedo and others added 2 commits July 28, 2023 14:40
…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 dlvenable merged commit 93d06db into opensearch-project:main Jul 31, 2023
24 checks passed
@markkuhn
Copy link

🚀

@MaGonzalMayedo MaGonzalMayedo deleted the adding_cloud_watch_logs_service branch August 1, 2023 22:28
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