[exporter/awscloudwatchlogsexporter] Improve performance of the awscloudwatchlogs exporter #26692
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
StreamKey uniquely identify an cloudwatch logs tream and is used to show where an event should be submited to. Signed-off-by: Raphael Silva <[email protected]>
The idea is that we will identify that an event belongs to a specific stream Signed-off-by: Raphael Silva <[email protected]>
Properly initialize a cloudwatch.Event in the awsemf exporter, setting the destination log stream of the event Signed-off-by: Raphael Silva <[email protected]>
Signed-off-by: Raphael Silva <[email protected]>
Signed-off-by: Raphael Silva <[email protected]>
Add support to multiple consumers in the cloudwatchlogs expoters. This will allow requests to be sent in parallel to cloudwatch logs Signed-off-by: Raphael Silva <[email protected]>
Signed-off-by: Raphael Silva <[email protected]>
Cloudwatch logs no longer limits to 5 request per second per log stream. Instead the limit is per account. Signed-off-by: Raphael Silva <[email protected]>
Sequence tokens are no longer necessary to make PutLogEvents calls to CloudWatch logs. This changes removes all the logic for managing this token and instead use a "Noop" Signed-off-by: Raphael Silva <[email protected]>
This optmization will reduce the memroy and CPU usage a bit because we will not need to keep a big buffer of events that were translated and insted they can be sent directly to the pusher. Signed-off-by: Raphael Silva <[email protected]>
Signed-off-by: Raphael Silva <[email protected]>
github-actions
bot
added
exporter/awscloudwatchlogs
Signed-off-by: Raphael Silva <[email protected]>
Signed-off-by: Raphael Silva <[email protected]>
| # you should instead start your pull request title with [chore] or use the "Skip Changelog" label. | ||
|
|
||
| # One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' | ||
| change_type: 'enhancement' |
There was a problem hiding this comment.
maybe breaking is ok? since the config is changed.
There was a problem hiding this comment.
I'm not sure if it is considered breaking in this case. The config is extended but the default behavior stays the same. @rapphil can you confirm this statement?
There was a problem hiding this comment.
That is correct, we are extending the configuration and keeping the previous behaviour unchanged. That is why I don't consider this a breaking change.
Aneurysm9
reviewed
Sep 26, 2023
Co-authored-by: Anthony Mirabella <[email protected]>
Signed-off-by: Raphael Silva <[email protected]>
| # you should instead start your pull request title with [chore] or use the "Skip Changelog" label. | ||
|
|
||
| # One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' | ||
| change_type: 'enhancement' |
There was a problem hiding this comment.
I'm not sure if it is considered breaking in this case. The config is extended but the default behavior stays the same. @rapphil can you confirm this statement?
bryan-aguilar
approved these changes
Oct 9, 2023
Codecov ReportAttention:
... and 7 files with indirect coverage changes 📢 Thoughts on this report? Let us know!. |
fatsheep9146
approved these changes
Oct 10, 2023
|
@rapphil there are still some checks are not passed |
|
@fatsheep9146 those failures were unrelated and they were fixed just by merging from main again. thank you for your review. |
bryan-aguilar
added
the
ready to merge
JaredTan95
pushed a commit
to openinsight-proj/opentelemetry-collector-contrib
that referenced
this pull request
Oct 18, 2023
…oudwatchlogs exporter (open-telemetry#26692) Adds support to the to parallelism in the awscloudwatchlogs exporter by leveraging the [exporter helper](https://github.com/open-telemetry/opentelemetry-collector/blob/main/exporter/exporterhelper/README.md). In this PR, we are adding support to the `num_consumers` configuration in the `sending_queue`. This will allow users to specify the number of consumers that will consume from the sending_queue in parallel. It is possible and straightforward to use this approach because CloudWatch logs [no longer requires that you use a token to control access to the stream that you are writing to](https://aws.amazon.com/about-aws/whats-new/2023/01/amazon-cloudwatch-logs-log-stream-transaction-quota-sequencetoken-requirement/). You can write to the same stream in parallel. To achieve this, this PR does the following: * Create Pusher that is able to push to multiple streams at the same time. * Move lifecycle of the Pusher to the function that is used to consume from the sending queue. This allows you to safely send to multiple streams at the same time without any resource contention since each call to consume logs will not share resources with others that are happening in parallel (one exception is the creation of log streams). Besides that I analyzed the code and removed other limitations: * locks that were not necessary * Limiter that was used to limit the number of requests per stream to 5 per second. [The TPS is much higher now and is per account.](https://aws.amazon.com/about-aws/whats-new/2023/01/amazon-cloudwatch-logs-log-stream-transaction-quota-sequencetoken-requirement/) ** How to review this PR: ** The first 3 commits in this PR were used to refactor the code before making the real changes. Please use the commits to simplify the review process. **Link to tracking Issue:** open-telemetry#26360 **Testing:** - Unit tests were added. - Tested locally sending logs to cloudwatch logs. **Documentation:** Documentation was added describing the new parameters. --------- Signed-off-by: Raphael Silva <[email protected]> Co-authored-by: Anthony Mirabella <[email protected]>
jmsnll
pushed a commit
to jmsnll/opentelemetry-collector-contrib
that referenced
this pull request
Nov 12, 2023
…oudwatchlogs exporter (open-telemetry#26692) Adds support to the to parallelism in the awscloudwatchlogs exporter by leveraging the [exporter helper](https://github.com/open-telemetry/opentelemetry-collector/blob/main/exporter/exporterhelper/README.md). In this PR, we are adding support to the `num_consumers` configuration in the `sending_queue`. This will allow users to specify the number of consumers that will consume from the sending_queue in parallel. It is possible and straightforward to use this approach because CloudWatch logs [no longer requires that you use a token to control access to the stream that you are writing to](https://aws.amazon.com/about-aws/whats-new/2023/01/amazon-cloudwatch-logs-log-stream-transaction-quota-sequencetoken-requirement/). You can write to the same stream in parallel. To achieve this, this PR does the following: * Create Pusher that is able to push to multiple streams at the same time. * Move lifecycle of the Pusher to the function that is used to consume from the sending queue. This allows you to safely send to multiple streams at the same time without any resource contention since each call to consume logs will not share resources with others that are happening in parallel (one exception is the creation of log streams). Besides that I analyzed the code and removed other limitations: * locks that were not necessary * Limiter that was used to limit the number of requests per stream to 5 per second. [The TPS is much higher now and is per account.](https://aws.amazon.com/about-aws/whats-new/2023/01/amazon-cloudwatch-logs-log-stream-transaction-quota-sequencetoken-requirement/) ** How to review this PR: ** The first 3 commits in this PR were used to refactor the code before making the real changes. Please use the commits to simplify the review process. **Link to tracking Issue:** open-telemetry#26360 **Testing:** - Unit tests were added. - Tested locally sending logs to cloudwatch logs. **Documentation:** Documentation was added describing the new parameters. --------- Signed-off-by: Raphael Silva <[email protected]> Co-authored-by: Anthony Mirabella <[email protected]>
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: Adds support to the to parallelism in the awscloudwatchlogs exporter by leveraging the exporter helper.
In this PR, we are adding support to the
num_consumersconfiguration in thesending_queue. This will allow users to specify the number of consumers that will consume from the sending_queue in parallel.It is possible and straightforward to use this approach because CloudWatch logs no longer requires that you use a token to control access to the stream that you are writing to. You can write to the same stream in parallel.
To achieve this, this PR does the following:
Besides that I analyzed the code and removed other limitations:
** How to review this PR: **
The first 3 commits in this PR were used to refactor the code before making the real changes. Please use the commits to simplify the review process.
Fixes #26360
Testing:
Documentation: Documentation was added describing the new parameters.