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

Using drop_overload category in EDS to report drop_overload stats in LRS #36047

Merged
merged 8 commits into from
Sep 19, 2024

Conversation

yanjunxiang-google
Copy link
Contributor

@yanjunxiang-google yanjunxiang-google commented Sep 9, 2024

Using drop_overload category in EDS to report drop_overload stats in LRS.

This is a follow up PR to support drop_overload load report service: #31384

Currently it is reporting with a fixed category "drop_overload". This PR changes it into the category passed in by cluster or EDS policy configuration.

…LRS.

Signed-off-by: Yanjun Xiang <yanjunxiang@google.com>
@yanjunxiang-google yanjunxiang-google changed the title Using drop_overload category in EDS to report drop_overload stats in … Using drop_overload category in EDS to report drop_overload stats in LRS Sep 9, 2024
Signed-off-by: Yanjun Xiang <yanjunxiang@google.com>
@yanjunxiang-google
Copy link
Contributor Author

/assign @adisuissa @fuqianggao

Copy link

@fuqianggao cannot be assigned to this issue.

🐱

Caused by: a #36047 (comment) was created by @yanjunxiang-google.

see: more, trace.

Copy link
Contributor

@adisuissa adisuissa left a comment

Choose a reason for hiding this comment

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

Thanks!
Left some minor comments.
Please add a release note.

source/common/upstream/cluster_manager_impl.cc Outdated Show resolved Hide resolved
envoy/upstream/thread_local_cluster.h Outdated Show resolved Hide resolved
test/mocks/upstream/cluster.h Show resolved Hide resolved
Signed-off-by: Yanjun Xiang <yanjunxiang@google.com>
Signed-off-by: Yanjun Xiang <yanjunxiang@google.com>
adisuissa
adisuissa previously approved these changes Sep 16, 2024
Copy link
Contributor

@adisuissa adisuissa 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!
Please add a release note as this is a user-facing change.
/assign-from @envoyproxy/senior-maintainers

source/server/admin/config_dump_handler.cc Show resolved Hide resolved
Copy link

@envoyproxy/senior-maintainers assignee is @mattklein123

🐱

Caused by: a #36047 (review) was submitted by @adisuissa.

see: more, trace.

Signed-off-by: Yanjun Xiang <yanjunxiang@google.com>
Copy link
Member

@mattklein123 mattklein123 left a comment

Choose a reason for hiding this comment

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

LGTM please merge main.

/wait

Signed-off-by: Yanjun Xiang <yanjunxiang@google.com>
@yanjunxiang-google
Copy link
Contributor Author

/retest

1 similar comment
@yanjunxiang-google
Copy link
Contributor Author

/retest

@yanjunxiang-google yanjunxiang-google marked this pull request as draft September 17, 2024 19:54
Signed-off-by: Yanjun Xiang <yanjunxiang@google.com>
@yanjunxiang-google yanjunxiang-google marked this pull request as ready for review September 17, 2024 22:30
@yanjunxiang-google
Copy link
Contributor Author

/retest

@yanjunxiang-google
Copy link
Contributor Author

Kind Ping

test/mocks/upstream/cluster.h Show resolved Hide resolved
changelogs/current.yaml Outdated Show resolved Hide resolved
Signed-off-by: Yanjun Xiang <yanjunxiang@google.com>
@mattklein123 mattklein123 enabled auto-merge (squash) September 18, 2024 23:04
@mattklein123 mattklein123 merged commit 6069f86 into envoyproxy:main Sep 19, 2024
40 checks passed
@yanjunxiang-google yanjunxiang-google deleted the lrs_string branch September 19, 2024 13:35
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.

4 participants