-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
[exporter/sumologicexporter]: Added timeout validation #33522
[exporter/sumologicexporter]: Added timeout validation #33522
Conversation
I think the easy cla check is still failing because I didn't set global user email in git until the most recent commit 🤔 |
@chan-tim-sumo Please squash commits, maybe it will help with the CLA bot |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
65d1f10
to
6de559d
Compare
oh i think i know why, even though after rebasing to one commit, the first commit's email is:
after amending the author to sumo email:
|
6de559d
to
f793d0c
Compare
can anyone from @open-telemetry/collector-contrib-approvers take a look at this? 😄 |
If a code owner can check to ensure the new minimum timeout limit looks good, I can add |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
@dmitryax can you take a look at this? ready to merge :D |
…y#33522) - Default timeout is 30 seconds with a max limit of 55. There're no validation checks for when [timeout is more than 55 seconds](https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/exporter/sumologicexporter/README.md?plain=1#L104). **Link to tracking Issue:** Closes open-telemetry#33151 **Testing:** - Unit test
Description:
Link to tracking Issue:
Testing:
Documentation: