Skip to content

feat: add the missing xds.authority label #12018

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

Merged
merged 9 commits into from
Apr 22, 2025

Conversation

AgraVator
Copy link
Contributor

@AgraVator AgraVator commented Apr 14, 2025

This aims to complete the XDS client metrics by adding the remaining grpc.xds.authority metric.

Copy link
Contributor

@kannanjgithub kannanjgithub left a comment

Choose a reason for hiding this comment

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

Review comments.

…a as in the existing flow

2. xds: modifies existing tests to account for authority
Copy link
Member

@ejona86 ejona86 left a comment

Choose a reason for hiding this comment

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

The commit/PR needs more context. Notably, it should include a reference to A78.

@AgraVator AgraVator requested a review from ejona86 April 21, 2025 07:17
@ejona86
Copy link
Member

ejona86 commented Apr 21, 2025

@AgraVator, you've copied/rebased commits from master to this PR. That's broken. Those need to be removed; I'd normally do "git rebase -i" and deleting the lines for the commits you didn't create. But I don't know what operation you did and why.

@AgraVator AgraVator force-pushed the add-xds-authority-label branch from da6df80 to ecf8e55 Compare April 21, 2025 13:34
2. xds: considers authority as a separate dimension
@AgraVator AgraVator force-pushed the add-xds-authority-label branch from ecf8e55 to 9ed94ac Compare April 21, 2025 13:35
@AgraVator AgraVator requested a review from ejona86 April 21, 2025 16:25
2. xds: move getAuthorityFromResourceName() near other static methods
2. xds: move getAuthorityFromResourceName() near other static methods
@AgraVator AgraVator requested a review from ejona86 April 22, 2025 05:30
Copy link
Contributor

@kannanjgithub kannanjgithub left a comment

Choose a reason for hiding this comment

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

Some nits.

@AgraVator AgraVator requested a review from kannanjgithub April 22, 2025 07:32
@AgraVator AgraVator merged commit 6cd007d into grpc:master Apr 22, 2025
15 of 16 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.

3 participants