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

canUseDirectPath returning false when DirectPath is enabled #3134

Closed
surbhigarg92 opened this issue Aug 26, 2024 · 0 comments · Fixed by #3170
Closed

canUseDirectPath returning false when DirectPath is enabled #3134

surbhigarg92 opened this issue Aug 26, 2024 · 0 comments · Fixed by #3170
Assignees
Labels
priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

Comments

@surbhigarg92
Copy link
Contributor

Spanner Client library is trying to use canUseDirectPath from the GapicSpannerRPC class

This method is always returning false irrespective of the DirectPath is enabled or not.

It is because of the needsCredentials which always returns "true" as credentials are always null.

Same method is working for gax because the credentials are set in ClientContext before the transportChannel is created.

However for Spanner library there is no way to use the canUseDirectPath method as the actual channel provider never gets updated.

@blakeli0 blakeli0 added type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. priority: p2 Moderately-important priority. Fix may not be included in next release. labels Aug 29, 2024
ldetmer pushed a commit that referenced this issue Sep 17, 2024
…#3170)

This should fix #3134.

Spanner needs to know if `GrpcSpannerStub`’s transportChannel uses
DirectPath for tracing purposes.
`canUseDirectPath()` from `stubSettings.getTransportChannelProvider()`
does not provide accurate information because credentials are set
afterwards when creating the transportChannel for `GrpcSpannerStub`
(`GrpcSpannerStub.create(stubSettings)`)

Proposed change:
Expose a property in `GrpcTransportChannel` and pass this information
down from channel provider.
Spanner can access from `ClientContext` as shown below
```
SpannerStubSettings stubSettings = options
        .getSpannerStubSettings()
        .toBuilder()
        .setTransportChannelProvider(channelProvider)
        .setCredentialsProvider(credentialsProvider)
        .setStreamWatchdogProvider(watchdogProvider)
        .setTracerFactory(options.getApiTracerFactory())
        .build();
ClientContext clientContext = ClientContext.create(stubSettings);
// create GrpcSpannerStub from clientContext instead. 
// this is equivelant to GrpcSpannerStub.create(stubSettings)
this.spannerStub =
    GrpcSpannerStub.create(clientContext);
// access this property from transport channel.
((GrpcTransportChannel) clientContext.getTransportChannel()).isDirectPath();
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants
@zhumin8 @blakeli0 @surbhigarg92 and others