fix(outputs.dynatrace): Prevent retry on authentication failure #18164
Conversation
mstrandboge
changed the title
telegraf-tiger
bot
added
the
plugin/output
| StatusCode: resp.StatusCode, | ||
| Retryable: false, | ||
| } | ||
| } else if resp.StatusCode < 200 || resp.StatusCode >= 300 { |
There was a problem hiding this comment.
Thank you for the PR, generally I don't think that 1xx status code should be treated as retry-able ones. Do you have some reasoning behind this decision?
Also, not all 5xx errors should be retry-ed. We need to be careful here as for example 501 or 505+ should not be in this category.
There was a problem hiding this comment.
Hello @odubajDT, I agree with you that 1xx status code should not be retried, concerning the 501, 505 or even 400 how do you want to treat them? I can update the pull request to reflect that.
Thank you for the review
There was a problem hiding this comment.
Generally deciding which status code will be retried is a longer process and requires a little bit of research, so I cannot answer you which codes specifically I would retry and which not...
Thank you!
There was a problem hiding this comment.
ok, is there some place in the codebase at your knowledge where this has alrerady been done and we can use the same strategy?
There was a problem hiding this comment.
I will propose a thing with what we discuss :D
Signed-off-by: Florentin Dubois <[email protected]>
FlorentinDUBOIS
force-pushed
the
devel/fdubois/fix/outputs/dynatrace
branch
from
6dc0046 to
76de5aa
Compare
|
Download PR build artifacts for linux_amd64.tar.gz, darwin_arm64.tar.gz, and windows_amd64.zip. 👍 This pull request doesn't change the Telegraf binary size 📦 Click here to get additional PR build artifactsArtifact URLs |
2 participants
Summary
Checklist
Related issues
relates #18119 #18118