-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
Promtail: Add handler timeout for GCP Logs Push target #7401
Conversation
./tools/diff_coverage.sh ../loki-main/test_results.txt test_results.txt ingester,distributor,querier,querier/queryrange,iter,storage,chunkenc,logql,loki Change in test coverage per package. Green indicates 0 or positive change, red indicates that test coverage for a package fell. + ingester 0%
+ distributor 0.2%
+ querier 0%
- querier/queryrange -0.1%
+ iter 0%
+ storage 0%
+ chunkenc 0%
+ logql 0%
+ loki 0% |
./tools/diff_coverage.sh ../loki-main/test_results.txt test_results.txt ingester,distributor,querier,querier/queryrange,iter,storage,chunkenc,logql,loki Change in test coverage per package. Green indicates 0 or positive change, red indicates that test coverage for a package fell. + ingester 0%
+ distributor 0%
+ querier 0%
- querier/queryrange -0.1%
+ iter 0%
+ storage 0%
+ chunkenc 0%
+ logql 0%
+ loki 0% |
./tools/diff_coverage.sh ../loki-main/test_results.txt test_results.txt ingester,distributor,querier,querier/queryrange,iter,storage,chunkenc,logql,loki Change in test coverage per package. Green indicates 0 or positive change, red indicates that test coverage for a package fell. + ingester 0%
+ distributor 0%
+ querier 0%
+ querier/queryrange 0%
+ iter 0%
+ storage 0%
+ chunkenc 0%
+ logql 0%
+ loki 0.6% |
./tools/diff_coverage.sh ../loki-main/test_results.txt test_results.txt ingester,distributor,querier,querier/queryrange,iter,storage,chunkenc,logql,loki Change in test coverage per package. Green indicates 0 or positive change, red indicates that test coverage for a package fell. + ingester 0%
+ distributor 0%
+ querier 0%
+ querier/queryrange 0.1%
+ iter 0%
+ storage 0%
+ chunkenc 0%
+ logql 0%
+ loki 0% |
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.
Looks overall good. thanks @thepalbi Left few minor comments.
./tools/diff_coverage.sh ../loki-main/test_results.txt test_results.txt ingester,distributor,querier,querier/queryrange,iter,storage,chunkenc,logql,loki Change in test coverage per package. Green indicates 0 or positive change, red indicates that test coverage for a package fell. + ingester 0%
+ distributor 0%
+ querier 0%
+ querier/queryrange 0%
+ iter 0%
+ storage 0%
+ chunkenc 0%
+ logql 0%
+ loki 0% |
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. thanks @thepalbi
**What this PR does / why we need it**: After been running the GCP Logs Push target in actual workloads, we've noticed that GCP sends a lot of traffic, and adjusts that send rate according to the rate at which the receiver can process. To perform that adjustment, GCP considers as a NACK [non 2xx HTTP responses](https://cloud.google.com/pubsub/docs/push#receive_push). This PR adds a target handler timeout (that includes as well the `api.Entry` channel send) to allow the user to configure this maximum processing time, therefore giving GCP notice that the sending rate is too high. This new timeout is optional, and disabled by default. **Which issue(s) this PR fixes**: Related to https://github.com/grafana/cloud-onboarding/issues/2067 **Special notes for your reviewer**: **Checklist** - [ ] Reviewed the `CONTRIBUTING.md` guide - [x] Documentation added - [x] Tests updated - [x] `CHANGELOG.md` updated - [x] Changes that require user attention or interaction to upgrade are documented in `docs/sources/upgrading/_index.md`
**What this PR does / why we need it**: After been running the GCP Logs Push target in actual workloads, we've noticed that GCP sends a lot of traffic, and adjusts that send rate according to the rate at which the receiver can process. To perform that adjustment, GCP considers as a NACK [non 2xx HTTP responses](https://cloud.google.com/pubsub/docs/push#receive_push). This PR adds a target handler timeout (that includes as well the `api.Entry` channel send) to allow the user to configure this maximum processing time, therefore giving GCP notice that the sending rate is too high. This new timeout is optional, and disabled by default. **Which issue(s) this PR fixes**: Related to https://github.com/grafana/cloud-onboarding/issues/2067 **Special notes for your reviewer**: **Checklist** - [ ] Reviewed the `CONTRIBUTING.md` guide - [x] Documentation added - [x] Tests updated - [x] `CHANGELOG.md` updated - [x] Changes that require user attention or interaction to upgrade are documented in `docs/sources/upgrading/_index.md`
**What this PR does / why we need it**: After been running the GCP Logs Push target in actual workloads, we've noticed that GCP sends a lot of traffic, and adjusts that send rate according to the rate at which the receiver can process. To perform that adjustment, GCP considers as a NACK [non 2xx HTTP responses](https://cloud.google.com/pubsub/docs/push#receive_push). This PR adds a target handler timeout (that includes as well the `api.Entry` channel send) to allow the user to configure this maximum processing time, therefore giving GCP notice that the sending rate is too high. This new timeout is optional, and disabled by default. **Which issue(s) this PR fixes**: Related to https://github.com/grafana/cloud-onboarding/issues/2067 **Special notes for your reviewer**: **Checklist** - [ ] Reviewed the `CONTRIBUTING.md` guide - [x] Documentation added - [x] Tests updated - [x] `CHANGELOG.md` updated - [x] Changes that require user attention or interaction to upgrade are documented in `docs/sources/upgrading/_index.md`
What this PR does / why we need it:
After been running the GCP Logs Push target in actual workloads, we've noticed that GCP sends a lot of traffic, and adjusts that send rate according to the rate at which the receiver can process. To perform that adjustment, GCP considers as a NACK non 2xx HTTP responses.
This PR adds a target handler timeout (that includes as well the
api.Entry
channel send) to allow the user to configure this maximum processing time, therefore giving GCP notice that the sending rate is too high.This new timeout is optional, and disabled by default.
Which issue(s) this PR fixes:
Related to https://github.com/grafana/cloud-onboarding/issues/2067
Special notes for your reviewer:
Checklist
CONTRIBUTING.md
guideCHANGELOG.md
updateddocs/sources/upgrading/_index.md