vector icon indicating copy to clipboard operation
vector copied to clipboard

fix(aws_s3 source): Report SQS fetch errors as warnings

Open yalinglee opened this issue 7 months ago • 2 comments

Summary

This change reports an SQS fetch error (e.g. when a connection is dropped) as a warning instead of an error, since it will retry on the next poll interval.

Change Type

  • [x] Bug fix
  • [ ] New feature
  • [ ] Non-functional (chore, refactoring, docs)
  • [ ] Performance

Is this a breaking change?

  • [ ] Yes
  • [x] No

How did you test this PR?

I ran make test SCOPE="sources::aws_s3" and all tests passed.

Does this PR include user facing changes?

  • [ ] Yes. Please add a changelog fragment based on our guidelines.
  • [x] No. A maintainer will apply the "no-changelog" label to this PR.

Notes

  • Please read our Vector contributor resources.
  • Do not hesitate to use @vectordotdev/vector to reach out to us regarding this PR.
  • The CI checks run only after we manually approve them.
    • We recommend adding a pre-push hook, please see this template.
    • Alternatively, we recommend running the following locally before pushing to the remote branch:
      • cargo fmt --all
      • cargo clippy --workspace --all-targets -- -D warnings
      • cargo nextest run --workspace (alternatively, you can run cargo test --all)
      • ./scripts/check_changelog_fragments.sh
  • After a review is requested, please avoid force pushes to help us review incrementally.
    • Feel free to push as many commits as you want. They will be squashed into one before merging.
    • For example, you can run git merge origin master and git push.
  • If this PR introduces changes Vector dependencies (modifies Cargo.lock), please run cargo vdev build licenses to regenerate the license inventory and commit the changes (if any). More details here.

References

  • Closes: https://github.com/vectordotdev/vector/issues/18855

yalinglee avatar May 14 '25 05:05 yalinglee

@jszwedko Would you mind taking a look to see if the change in this PR is enough to make Vector report SQS fetch errors as warnings? Thank you!

yalinglee avatar May 22 '25 18:05 yalinglee

Making SqsMessageReceiveError (emphasis on the Error) produce a warning sounds semantically awkward to me. Currently SqsMessageReceiveError is emitted in two places. Maybe you want to stop emitting it in once case and produce a warning there instead?

pront avatar Jun 10 '25 18:06 pront