Skip to content

[CSM O11Y] Fix issue when CSM optional labels are present in server metrics#35633

Closed
yijiem wants to merge 4 commits intogrpc:masterfrom
yijiem:labels-injector-patch
Closed

[CSM O11Y] Fix issue when CSM optional labels are present in server metrics#35633
yijiem wants to merge 4 commits intogrpc:masterfrom
yijiem:labels-injector-patch

Conversation

@yijiem
Copy link
Copy Markdown
Member

@yijiem yijiem commented Jan 23, 2024

No description provided.

@yijiem yijiem added the release notes: no Indicates if PR should not be in release notes label Jan 23, 2024
// Some optional labels may only be set on client or server. We use this enum
// to specify the entity of the caller for the APIs that need this
// information.
enum class Entity {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Throughout gRPC Core, we've just been using a simple is_client bool for this. If we were to do this, I would prefer doing it all over.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

+1 -- let's just use a bool here. We're not expecting to have any context other than client or server in the future, so there's no need to leave room for future options.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Done.

@markdroth
Copy link
Copy Markdown
Member

I assume this will need to be back-ported to 1.61 before the release goes out.

yijiem added a commit to yijiem/grpc that referenced this pull request Jan 24, 2024
…etrics (grpc#35633)

<!--

If you know who should review your pull request, please assign it to that
person, otherwise the pull request would get assigned randomly.

If your pull request is for a specific language, please add the appropriate
lang label.

-->

Closes grpc#35633

COPYBARA_INTEGRATE_REVIEW=grpc#35633 from yijiem:labels-injector-patch 6876243
PiperOrigin-RevId: 600931754
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants