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

Added LogBuffering validation. #729

Merged
merged 3 commits into from
Nov 15, 2023
Merged

Added LogBuffering validation. #729

merged 3 commits into from
Nov 15, 2023

Conversation

kazhura-aws
Copy link
Contributor

The validation will provide a meaningful error in case the LogBuffering is not configured according to telemetry-api-buffering docs.

Issue #, if available:

Description of changes:
Updated the LogBuffering struct in. Added validation and unit tests. Also update the run method of the extension to utilize the validation and return an error if applicable.

By submitting this pull request

  • I confirm that my contribution is made under the terms of the Apache 2.0 license.
  • I confirm that I've made a best effort attempt to update all relevant documentation.

Copy link
Contributor

@calavera calavera left a comment

Choose a reason for hiding this comment

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

Thanks a lot for the improvements. I left you a couple of small suggestions if you want to make the code a little bit cleaner.

Comment on lines 229 to 232
let log_buffering_validation_result = validate_buffering_configuration(self.log_buffering);
if log_buffering_validation_result.is_err() {
return log_buffering_validation_result;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe you can just do this, no need to use the result explicitly.

Suggested change
let log_buffering_validation_result = validate_buffering_configuration(self.log_buffering);
if log_buffering_validation_result.is_err() {
return log_buffering_validation_result;
}
validate_buffering_configuration(self.log_buffering)?;

Comment on lines 271 to 274
let telemetry_buffering_validation_result = validate_buffering_configuration(self.telemetry_buffering);
if telemetry_buffering_validation_result.is_err() {
return telemetry_buffering_validation_result;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe you can just do this, no need to use the result explicitly.

Suggested change
let telemetry_buffering_validation_result = validate_buffering_configuration(self.telemetry_buffering);
if telemetry_buffering_validation_result.is_err() {
return telemetry_buffering_validation_result;
}
validate_buffering_configuration(self.log_buffering)?;

@calavera calavera merged commit 32207ea into awslabs:main Nov 15, 2023
4 checks passed
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.

2 participants