internal/testrunner/runners/system: supress absent Records complaint by awss3 input #3023
Conversation
…by awss3 input The awss3 input incorrectly complains about the absence of a Records field in SQS messages. Valid SQS messages are possible without a Records field as described in the AWS documentation[1]. [1]https://docs.aws.amazon.com/AmazonS3/latest/userguide/notification-content-structure.html#notification-content-structure-examples
|
/test |
💚 Build Succeeded
cc @efd6 |
| // This is excluded because the awss3 input incorrectly complains about missing Records fields in s3:TestEvent messages. | ||
| // See https://docs.aws.amazon.com/AmazonS3/latest/userguide/notification-content-structure.html#notification-content-structure-examples. | ||
| regexp.MustCompile(`state changed .* \(HEALTHY->DEGRADED\): .* the message is an invalid S3 notification: missing Records field`), |
There was a problem hiding this comment.
One doubt here, is this exception required even if applying one of the suggestions mentioned in elastic/integrations#15472 (comment) ?
It looks like that if one of those suggestions mentioned in the integrations issue is applied, this exception/exclude would not be needed:
- with the first suggestion, it would imply that there is no error anymore
- with the second suggestion, IIUC the error reported in Elastic Agent logs would be a valid error since it won't be an
s3:TestEventmessage.
Would it be needed to wait to merge this PR depending on the action taken in the integrations issue ?
cc @jsoriano
There was a problem hiding this comment.
I don't see a problem with adding this exception. Even if we apply the changes mentioned in elastic/integrations#15472 (comment), it will be useful for versions of the stack without these fixes.
There was a problem hiding this comment.
The situation here is not ideal, but this is IMO the least worst choice. We can always remove this exception in the future when sufficiently many awss3 input integrations have been migrated to the fixed version of the input.
| // This is excluded because the awss3 input incorrectly complains about missing Records fields in s3:TestEvent messages. | ||
| // See https://docs.aws.amazon.com/AmazonS3/latest/userguide/notification-content-structure.html#notification-content-structure-examples. | ||
| regexp.MustCompile(`state changed .* \(HEALTHY->DEGRADED\): .* the message is an invalid S3 notification: missing Records field`), |
There was a problem hiding this comment.
I don't see a problem with adding this exception. Even if we apply the changes mentioned in elastic/integrations#15472 (comment), it will be useful for versions of the stack without these fixes.
4 participants
The awss3 input incorrectly complains about the absence of a Records field in SQS messages. Valid SQS messages are possible without a Records field as described in the AWS documentation[1].
[1]https://docs.aws.amazon.com/AmazonS3/latest/userguide/notification-content-structure.html#notification-content-structure-examples
See also elastic/integrations#15472 (comment).