Skip to content

Comments

feat(transport/grpc): add function to get reason for direct path incompatibility#3491

Open
cpriti-os wants to merge 9 commits intogoogleapis:mainfrom
cpriti-os:main
Open

feat(transport/grpc): add function to get reason for direct path incompatibility#3491
cpriti-os wants to merge 9 commits intogoogleapis:mainfrom
cpriti-os:main

Conversation

@cpriti-os
Copy link

@cpriti-os cpriti-os commented Feb 11, 2026

Adds DirectPathStatus and CheckWithReason to categorize why DirectPath is used or disabled into fallback reasons. This enables better observability.

@cpriti-os cpriti-os requested a review from a team as a code owner February 11, 2026 05:42
@quartzmo
Copy link
Member

@quartzmo quartzmo requested a review from shollyman February 11, 2026 16:33
@cpriti-os
Copy link
Author

@cpriti-os Why is this component here in https://github.com/googleapis/google-api-go-client/tree/main/transport/grpc and not for example in the newer equivalent package https://github.com/googleapis/google-cloud-go/tree/main/auth/grpctransport ?

Thanks for pointing out, couldn't find it when I raised this, will try to move the code there.

@cpriti-os
Copy link
Author

cpriti-os commented Feb 12, 2026

@cpriti-os Why is this component here in https://github.com/googleapis/google-api-go-client/tree/main/transport/grpc and not for example in the newer equivalent package https://github.com/googleapis/google-cloud-go/tree/main/auth/grpctransport ?

Thanks for pointing out, couldn't find it when I raised this, will try to move the code there.

I was checking the storage dependencies and I dont think the storage package uses the new auth library, therefore adding the change there will not help us get the reason in storage layer without migrating.

}
}

func TestIsDirectPathEnabled(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Are these tests relevant to the changes?

Copy link
Author

Choose a reason for hiding this comment

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

Earlier this particular test was since I was reusing this function in the logic I added and its good to be covered. Didn't see the harm in keeping the test even if logic was updated to be more granular

Copy link
Contributor

@shollyman shollyman left a comment

Choose a reason for hiding this comment

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

I'm still a bit confused a bit by how this code is intended to be leveraged, given the mix of resolving dial options, checking resolved grpc, and resolving auth configuration. Is there a doc that can help me understand this a bit better? We've had a few similar requests recently around complexities resolving client options, and I'm trying to figure out if there's common functionality we should be building out. This very detailed exported function that appears to rationalize why a behavior didn't happen, and I wonder if we can simplify it with some underlying designs.

@cpriti-os
Copy link
Author

I'm still a bit confused a bit by how this code is intended to be leveraged, given the mix of resolving dial options, checking resolved grpc, and resolving auth configuration. Is there a doc that can help me understand this a bit better? We've had a few similar requests recently around complexities resolving client options, and I'm trying to figure out if there's common functionality we should be building out. This very detailed exported function that appears to rationalize why a behavior didn't happen, and I wonder if we can simplify it with some underlying designs.

@shollyman thank you for checking this. Our current goal is pretty simple, provide as granular a reason as possible as to why the client chose not to use Direct Path. The added function would be called by GCS Go SDK while creating a new gRPC client and pass the received reason to the backend using headers. The backend can then log this telemetry and in the near future reject a cloudpath request with the reason (in this PR) to help customer resolve the issue more efficiently. I have also provided a one pager discussing this in the chat space. Let me know if this answers the question. Happy to discuss more.

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