out_cloudwatch_logs: auto retry invalid requests #4189
Conversation
Signed-off-by: Wesley Pettit <[email protected]>
| if (retry == FLB_TRUE) { | ||
| flb_plg_debug(ctx->ins, "Recieved code 200 but response was invalid, %s header not found", | ||
| AMZN_REQUEST_ID_HEADER); | ||
| } | ||
| else { | ||
| flb_plg_error(ctx->ins, "Recieved code 200 but response was invalid, %s header not found", | ||
| AMZN_REQUEST_ID_HEADER); | ||
| } |
There was a problem hiding this comment.
This is confusing. Same behavior and message, one is being treated as debug log and another error type. I think we can remove this check and print a warn. Then if needed when the auto retry fails, we can print another error log before returning -1.
There was a problem hiding this comment.
so the idea is that we give 1 retry. If the retry is available, then we don't print an error. This prevents the users from seeing lots of errors in their logs that they can't do anything about and then will scare them into thinking something is going wrong. I only want to bring the user's attention to this after the retry fails.
There was a problem hiding this comment.
If the retry succeeds, then the plugin was successful, and the user should not be alerted to any error. Hence, no error log in that case.
This is similar to our go plugins which use the AWS SDK go which will silently auto-retry requests IIRC.
There was a problem hiding this comment.
I unerstand your point. Thats why I suggested maybe we can print an error log only before returning the final -1. Before that we can make them either debug or warn which carries similar values in my opinion. This way we can reduce an extra condition.
Either way you can't reduce the number of logs (debug/error) you are printing, right?
There was a problem hiding this comment.
oh I see, you just want me to remove the if else
There was a problem hiding this comment.
Yea. just keep one log whatever you prfer (debug/warn/error). This way your six lines of code become one.
There was a problem hiding this comment.
@PettitWesley Is this resolved?
There was a problem hiding this comment.
@nokute78 Sorry- fixed it.
|
This PR is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 10 days. |
Signed-off-by: Wesley Pettit <[email protected]>
Signed-off-by: Wesley Pettit [email protected]
Enter
[N/A]in the box, if an item is not applicable to your change.Testing
Before we can approve your change; please submit the following in a comment:
Documentation
Fluent Bit is licensed under Apache 2.0, by submitting this pull request I understand that this code will be released under the terms of that license.