Skip to content
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

feat: add trace for rate-limit #2974

Merged
merged 7 commits into from
Apr 11, 2024
Merged

feat: add trace for rate-limit #2974

merged 7 commits into from
Apr 11, 2024

Conversation

ShyunnY
Copy link
Contributor

@ShyunnY ShyunnY commented Mar 18, 2024

What type of PR is this?

feat: add trace for rate-limit

Which issue(s) this PR fixes:

Fixes #2899

@ShyunnY ShyunnY requested a review from a team as a code owner March 18, 2024 12:00
Copy link

codecov bot commented Mar 18, 2024

Codecov Report

Attention: Patch coverage is 78.63248% with 25 lines in your changes are missing coverage. Please review.

Project coverage is 58.20%. Comparing base (29946b0) to head (7a115ab).
Report is 30 commits behind head on main.

❗ Current head 7a115ab differs from pull request most recent head 771a3d7. Consider uploading reports for the commit 771a3d7 to get more accurate results

Files Patch % Lines
api/v1alpha1/zz_generated.deepcopy.go 0.00% 25 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2974      +/-   ##
==========================================
- Coverage   66.51%   58.20%   -8.31%     
==========================================
  Files         161      165       +4     
  Lines       22673    27573    +4900     
==========================================
+ Hits        15080    16050     +970     
- Misses       6720    10572    +3852     
- Partials      873      951      +78     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ShyunnY ShyunnY force-pushed the trace-rl branch 2 times, most recently from de3225e to 1e482a9 Compare March 18, 2024 12:51
@ShyunnY
Copy link
Contributor Author

ShyunnY commented Mar 18, 2024

/retest

@ShyunnY ShyunnY marked this pull request as draft March 28, 2024 14:30
@ShyunnY ShyunnY marked this pull request as ready for review March 29, 2024 13:26
@ShyunnY ShyunnY marked this pull request as draft April 1, 2024 11:03
@ShyunnY

This comment was marked as resolved.

@ShyunnY ShyunnY marked this pull request as ready for review April 1, 2024 12:15
zirain
zirain previously approved these changes Apr 1, 2024
Signed-off-by: ShyunnY <1147212064@qq.com>
ShyunnY added 2 commits April 10, 2024 19:15
Signed-off-by: ShyunnY <1147212064@qq.com>
Signed-off-by: ShyunnY <1147212064@qq.com>
@yuluo-yx
Copy link
Contributor

/retest

@arkodg
Copy link
Contributor

arkodg commented Apr 10, 2024

PR is looking good @ShyunnY ! added some minor comments

ShyunnY and others added 3 commits April 10, 2024 20:21
Signed-off-by: ShyunnY <1147212064@qq.com>
Signed-off-by: yuluo-yx <yuluo08290126@gmail.com>
@ShyunnY ShyunnY requested a review from arkodg April 10, 2024 14:32
Copy link
Contributor

@arkodg arkodg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM thanks @ShyunnY ! thanks for addressing the feedback

@arkodg arkodg requested review from zirain and a team April 10, 2024 16:11
Copy link
Contributor

@zirain zirain left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, would be better add/update e2e test for this.

@ShyunnY
Copy link
Contributor Author

ShyunnY commented Apr 11, 2024

LGTM, would be better add/update e2e test for this.

Thank you for your review, I will push e2e related updates in the near future

@arkodg arkodg merged commit aa4e3a0 into envoyproxy:main Apr 11, 2024
19 of 20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support Tracing for Rate Limit Service
4 participants