Skip to content

Commit

Permalink
feat: expose property in GrpcTransportChannel if it uses direct path. (
Browse files Browse the repository at this point in the history
…#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();
```
  • Loading branch information
zhumin8 authored and ldetmer committed Sep 17, 2024
1 parent 2b9a7f1 commit 8efc704
Show file tree
Hide file tree
Showing 6 changed files with 85 additions and 13 deletions.
10 changes: 10 additions & 0 deletions gax-java/gax-grpc/clirr-ignored-differences.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<?xml version="1.0" encoding="UTF-8"?>
<!-- see http://www.mojohaus.org/clirr-maven-plugin/examples/ignored-differences.html -->
<differences>
<!-- Add AutoValue abstract method isDirectPath -->
<difference>
<differenceType>7013</differenceType>
<className>com/google/api/gax/grpc/GrpcTransportChannel</className>
<method>boolean isDirectPath()</method>
</difference>
</differences>
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,8 @@ public GrpcCallContext getEmptyCallContext() {
/** The channel in use. */
abstract ManagedChannel getManagedChannel();

public abstract boolean isDirectPath();

public Channel getChannel() {
return getManagedChannel();
}
Expand Down Expand Up @@ -100,7 +102,7 @@ public void close() {
}

public static Builder newBuilder() {
return new AutoValue_GrpcTransportChannel.Builder();
return new AutoValue_GrpcTransportChannel.Builder().setDirectPath(false);
}

public static GrpcTransportChannel create(ManagedChannel channel) {
Expand All @@ -111,6 +113,8 @@ public static GrpcTransportChannel create(ManagedChannel channel) {
public abstract static class Builder {
public abstract Builder setManagedChannel(ManagedChannel value);

abstract Builder setDirectPath(boolean value);

public abstract GrpcTransportChannel build();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -258,9 +258,12 @@ public TransportChannel getTransportChannel() throws IOException {
}

private TransportChannel createChannel() throws IOException {
return GrpcTransportChannel.create(
ChannelPool.create(
channelPoolSettings, InstantiatingGrpcChannelProvider.this::createSingleChannel));
return GrpcTransportChannel.newBuilder()
.setManagedChannel(
ChannelPool.create(
channelPoolSettings, InstantiatingGrpcChannelProvider.this::createSingleChannel))
.setDirectPath(this.canUseDirectPath())
.build();
}

private boolean isDirectPathEnabled() {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
/*
* Copyright 2024 Google LLC
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions are
* met:
*
* * Redistributions of source code must retain the above copyright
* notice, this list of conditions and the following disclaimer.
* * Redistributions in binary form must reproduce the above
* copyright notice, this list of conditions and the following disclaimer
* in the documentation and/or other materials provided with the
* distribution.
* * Neither the name of Google LLC nor the names of its
* contributors may be used to endorse or promote products derived from
* this software without specific prior written permission.
*
* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
* "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
* LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
* A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
* OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
* SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
* LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
* DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
* THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
* (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
* OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
*/

package com.google.api.gax.grpc;

import static org.junit.jupiter.api.Assertions.*;

import io.grpc.ManagedChannel;
import org.junit.jupiter.api.Test;
import org.mockito.Mockito;

class GrpcTransportChannelTest {

@Test
void testBuilderDefaults() {
ManagedChannel channel = Mockito.mock(ManagedChannel.class);
GrpcTransportChannel grpcTransportChannel =
GrpcTransportChannel.newBuilder().setManagedChannel(channel).build();
assertFalse(grpcTransportChannel.isDirectPath());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -706,7 +706,7 @@ void testLogDirectPathMisconfigNotOnGCE() throws Exception {
}

@Test
public void canUseDirectPath_happyPath() {
public void canUseDirectPath_happyPath() throws IOException {
System.setProperty("os.name", "Linux");
EnvironmentProvider envProvider = Mockito.mock(EnvironmentProvider.class);
Mockito.when(
Expand All @@ -718,14 +718,20 @@ public void canUseDirectPath_happyPath() {
.setAttemptDirectPath(true)
.setCredentials(computeEngineCredentials)
.setEndpoint(DEFAULT_ENDPOINT)
.setEnvProvider(envProvider);
.setEnvProvider(envProvider)
.setHeaderProvider(Mockito.mock(HeaderProvider.class));
InstantiatingGrpcChannelProvider provider =
new InstantiatingGrpcChannelProvider(builder, GCE_PRODUCTION_NAME_AFTER_2016);
Truth.assertThat(provider.canUseDirectPath()).isTrue();

// verify this info is passed correctly to transport channel
TransportChannel transportChannel = provider.getTransportChannel();
Truth.assertThat(((GrpcTransportChannel) transportChannel).isDirectPath()).isTrue();
transportChannel.shutdownNow();
}

@Test
public void canUseDirectPath_directPathEnvVarDisabled() {
public void canUseDirectPath_directPathEnvVarDisabled() throws IOException {
System.setProperty("os.name", "Linux");
EnvironmentProvider envProvider = Mockito.mock(EnvironmentProvider.class);
Mockito.when(
Expand All @@ -737,10 +743,16 @@ public void canUseDirectPath_directPathEnvVarDisabled() {
.setAttemptDirectPath(true)
.setCredentials(computeEngineCredentials)
.setEndpoint(DEFAULT_ENDPOINT)
.setEnvProvider(envProvider);
.setEnvProvider(envProvider)
.setHeaderProvider(Mockito.mock(HeaderProvider.class));
InstantiatingGrpcChannelProvider provider =
new InstantiatingGrpcChannelProvider(builder, GCE_PRODUCTION_NAME_AFTER_2016);
Truth.assertThat(provider.canUseDirectPath()).isFalse();

// verify this info is passed correctly to transport channel
TransportChannel transportChannel = provider.getTransportChannel();
Truth.assertThat(((GrpcTransportChannel) transportChannel).isDirectPath()).isFalse();
transportChannel.shutdownNow();
}

@Test
Expand Down
5 changes: 0 additions & 5 deletions gax-java/gax/clirr-ignored-differences.xml
Original file line number Diff line number Diff line change
Expand Up @@ -66,11 +66,6 @@
<className>com/google/api/gax/*/*</className>
<method>*org.threeten.bp.Duration *()</method>
</difference>
<difference>
<differenceType>7002</differenceType>
<className>com/google/api/gax/grpc/InstantiatingGrpcChannelProvider$Builder</className>
<method>* *</method>
</difference>
<!-- Add a default Endpoint Getter -->
<difference>
<differenceType>7012</differenceType>
Expand Down

0 comments on commit 8efc704

Please sign in to comment.