feat: Allow Adding Client Level Attributes to MetricsTracerFactory by lqiu96 · Pull Request #2614 · googleapis/sdk-platform-java

lqiu96 · 2024-03-29T14:42:02Z

Allow the MetricsTracerFactory to take in a second parameter (Map of attributes) that will be treated as client level attributes. These attributes will be added to every single MetricsTracer created throughout the lifecycle of the client.

Was able to verify this behavior inside Cloud Monitoring:

Additional Attribute was recorded.

Via:

    InstantiatingGrpcChannelProvider channelProvider =
            InstantiatingGrpcChannelProvider.newBuilder().build();

    Map<String, String> clientAttributesMapping = new HashMap<>();
    clientAttributesMapping.put("directpath_enabled", String.valueOf(channelProvider.canUseDirectPath()));

    ...
    options
      .setApiTracerFactory(new MetricsTracerFactory(recorder, clientAttributesMapping))
      .build();

gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java

lqiu96 · 2024-04-02T21:32:13Z

gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsTracerFactory.java

+    MetricsTracer metricsTracer =
+        new MetricsTracer(
+            MethodName.of(spanName.getClientName(), spanName.getMethodName()), metricsRecorder);
+    for (Map.Entry<String, String> attributeEntrySet : attributes.entrySet()) {
+      metricsTracer.addAttributes(attributeEntrySet.getKey(), attributeEntrySet.getValue());
+    }
+    return metricsTracer;


Possible options:

Use MetricTracer's addAttributes() to add only add client level attributes. The clientName and methodName attributes will continue to be passed via MethodName via the constructor.

Add MethodName attribute with addAttributes() and refactor MetricsTracer to only take in metricsRecorder

Probably going to keep it this way (option 1).

lqiu96 · 2024-04-02T21:36:14Z

gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsTracerFactory.java

+  public MetricsTracerFactory(MetricsRecorder metricsRecorder, Map<String, String> attributes) {
    this.metricsRecorder = metricsRecorder;
+    this.attributes = attributes;
  }


Additional constructor to take in user configured in client level attributes.

In the future, we can have Gax automatically collect attributes and this would need a constructor to take in a set of attribute flags. Class is marked with InternalApi and could be open to more refactoring if we want to pursue this.

gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java

blakeli0 · 2024-04-20T02:47:12Z

showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITOtelMetrics.java

+            "Echo.Echo",
+            MetricsTracer.LANGUAGE_ATTRIBUTE,
+            MetricsTracer.DEFAULT_LANGUAGE,
+            "directpath_enabled",


Can we extract the String literals that are used twice to variables?

showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITOtelMetrics.java

gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsTracerFactory.java

gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java

blakeli0 · 2024-04-20T04:05:45Z

gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java

  @Nullable private final Boolean allowNonDefaultServiceAccount;
  @VisibleForTesting final ImmutableMap<String, ?> directPathServiceConfig;
  @Nullable private final MtlsProvider mtlsProvider;
+  private final SystemProductNameReader systemProductNameReader;


Originally we thought exposing a default scope method would be good enough, but now that we are introducing a new field and a new private class just for testing, I'm not sure it is worth it. I'm now actually leaning towards keep isOnComputeEngine() as it is.
In the future, we can introduce mockito-inline or mockito v5(requires Java 11) that can mock/spy final classes.

There is a bit of change added just for testing. I think there is quite a bit of added benefit, namely that we get to test multiple functions and also to get to test how they interact together:

sdk-platform-java/gax-java/gax-grpc/src/test/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProviderTest.java

Lines 668 to 809 in b1be846

@Test

public void canUseDirectPath_happyPath() {

InstantiatingGrpcChannelProvider provider =

InstantiatingGrpcChannelProvider.newBuilder()

.setEnvProvider(envProvider)

.setAttemptDirectPath(true)

.setCredentials(computeEngineCredentials)

.setSystemProductNameReader(systemProductNameReader)

.setEndpoint(DEFAULT_ENDPOINT)

.build();

Truth.assertThat(provider.canUseDirectPath()).isTrue();

}

@Test

public void canUseDirectPath_directPathEnvVarDisabled() {

EnvironmentProvider envProvider = Mockito.mock(EnvironmentProvider.class);

Mockito.when(

envProvider.getenv(

InstantiatingGrpcChannelProvider.DIRECT_PATH_ENV_DISABLE_DIRECT_PATH))

.thenReturn("true");

InstantiatingGrpcChannelProvider provider =

InstantiatingGrpcChannelProvider.newBuilder()

.setEnvProvider(envProvider)

.setAttemptDirectPath(true)

.setCredentials(computeEngineCredentials)

.setSystemProductNameReader(systemProductNameReader)

.setEndpoint(DEFAULT_ENDPOINT)

.build();

Truth.assertThat(provider.canUseDirectPath()).isFalse();

}

@Test

public void canUseDirectPath_directPathEnvVarNotSet_attemptDirectPathIsTrue() {

EnvironmentProvider envProvider = Mockito.mock(EnvironmentProvider.class);

// If system property is not set, then System.getProperty() returns null

Mockito.when(

envProvider.getenv(

InstantiatingGrpcChannelProvider.DIRECT_PATH_ENV_DISABLE_DIRECT_PATH))

.thenReturn(null);

InstantiatingGrpcChannelProvider provider =

InstantiatingGrpcChannelProvider.newBuilder()

.setEnvProvider(envProvider)

.setAttemptDirectPath(true)

.setCredentials(computeEngineCredentials)

.setSystemProductNameReader(systemProductNameReader)

.setEndpoint(DEFAULT_ENDPOINT)

.build();

Truth.assertThat(provider.canUseDirectPath()).isTrue();

}

@Test

public void canUseDirectPath_directPathEnvVarNotSet_attemptDirectPathIsFalse() {

EnvironmentProvider envProvider = Mockito.mock(EnvironmentProvider.class);

Mockito.when(

envProvider.getenv(

InstantiatingGrpcChannelProvider.DIRECT_PATH_ENV_DISABLE_DIRECT_PATH))

.thenReturn(null);

InstantiatingGrpcChannelProvider provider =

InstantiatingGrpcChannelProvider.newBuilder()

.setEnvProvider(envProvider)

.setAttemptDirectPath(false)

.setCredentials(computeEngineCredentials)

.setSystemProductNameReader(systemProductNameReader)

.setEndpoint(DEFAULT_ENDPOINT)

.build();

Truth.assertThat(provider.canUseDirectPath()).isFalse();

}

@Test

public void canUseDirectPath_nonComputeCredentials() {

Credentials credentials = Mockito.mock(Credentials.class);

InstantiatingGrpcChannelProvider provider =

InstantiatingGrpcChannelProvider.newBuilder()

.setEnvProvider(envProvider)

.setAttemptDirectPath(true)

.setCredentials(credentials)

.setSystemProductNameReader(systemProductNameReader)

.setEndpoint(DEFAULT_ENDPOINT)

.build();

Truth.assertThat(provider.canUseDirectPath()).isFalse();

}

@Test

public void canUseDirectPath_systemProductNameInvalid() throws IOException {

InstantiatingGrpcChannelProvider.SystemProductNameReader systemProductNameReader =

Mockito.mock(InstantiatingGrpcChannelProvider.SystemProductNameReader.class);

Mockito.when(systemProductNameReader.getSystemProductName()).thenReturn("testing");

InstantiatingGrpcChannelProvider provider =

InstantiatingGrpcChannelProvider.newBuilder()

.setEnvProvider(envProvider)

.setAttemptDirectPath(true)

.setCredentials(computeEngineCredentials)

.setSystemProductNameReader(systemProductNameReader)

.setEndpoint(DEFAULT_ENDPOINT)

.build();

Truth.assertThat(provider.canUseDirectPath()).isFalse();

}

@Test

public void canUseDirectPath_systemPropertyIsNotLinux() throws IOException {

System.setProperty("os.name", "Windows");

InstantiatingGrpcChannelProvider provider =

InstantiatingGrpcChannelProvider.newBuilder()

.setEnvProvider(envProvider)

.setAttemptDirectPath(true)

.setCredentials(computeEngineCredentials)

.setSystemProductNameReader(systemProductNameReader)

.setEndpoint(DEFAULT_ENDPOINT)

.build();

Truth.assertThat(provider.canUseDirectPath()).isFalse();

}

@Test

public void canUseDirectPath_systemProductNameThrowsIOException() throws IOException {

InstantiatingGrpcChannelProvider.SystemProductNameReader systemProductNameReader =

Mockito.mock(InstantiatingGrpcChannelProvider.SystemProductNameReader.class);

Mockito.when(systemProductNameReader.getSystemProductName()).thenThrow(new IOException());

InstantiatingGrpcChannelProvider provider =

InstantiatingGrpcChannelProvider.newBuilder()

.setEnvProvider(envProvider)

.setAttemptDirectPath(true)

.setCredentials(computeEngineCredentials)

.setSystemProductNameReader(systemProductNameReader)

.setEndpoint(DEFAULT_ENDPOINT)

.build();

Truth.assertThat(provider.canUseDirectPath()).isFalse();

}

@Test

public void canUseDirectPath_nonGDUUniverseDomain() {

String nonGDUEndpoint = "test.random.com:443";

InstantiatingGrpcChannelProvider provider =

InstantiatingGrpcChannelProvider.newBuilder()

.setEnvProvider(envProvider)

.setAttemptDirectPath(true)

.setCredentials(computeEngineCredentials)

.setSystemProductNameReader(systemProductNameReader)

.setEndpoint(nonGDUEndpoint)

.build();

Truth.assertThat(provider.canUseDirectPath()).isFalse();

}

. I believe the behavior is kept the same and I am adding wrappers for the same functionality.

The added classes and methods are all package-private scope and should not impact the customers at all. And since they are package-private, we should be able to easily remove and refactor to use mockito v5 for this same purpose.

The logic is the same but the behavior is slightly different. isOnComputeEngine() used to be a static method, now it is not because we have to initialize an instance of SystemProductNameReader. It would be great if we can keep isOnComputeEngine() static. Or even better, isOnComputeEngine() does not have to be a static method, it could be a static block so that it does not get executed every time we call it. The benefit is marginal though since InstantiatingGrpcChannelProvider should only be called during client initialization.

I see, I have overlooked the intention behind isOnComputeEngine() being static. Let me see if I can find a way around this (either with keeping it static or moving it to a static clock).

I would also prefer to not change the behavior if possible.

sonarqubecloud · 2024-04-23T16:11:59Z

Quality Gate failed for 'gapic-generator-java-root'

Failed conditions
69.4% Coverage on New Code (required ≥ 80%)

See analysis details on SonarCloud

sonarqubecloud · 2024-04-23T16:15:12Z

Quality Gate failed for 'java_showcase_integration_tests'

Failed conditions
55.3% Coverage on New Code (required ≥ 80%)

See analysis details on SonarCloud

surbhigarg92 · 2024-05-01T16:25:43Z

gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java

+   * @return if DirectPath is enabled for the client AND if the configurations are valid
+   */
+  @InternalApi
+  public boolean canUseDirectPath() {


We should also add a check for isDirectPathXdsEnabled

@mohanli-ml I believe you had helped implement this logic before. We're trying to expose a getter for the conditions that would enable DirectPath for this gRPC channel. Should isDirectPathXdsEnabled() be added here?

I copied over the original configs set:

sdk-platform-java/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java

Lines 373 to 376 in 16aec03

if (isDirectPathEnabled()

&& isCredentialDirectPathCompatible()

&& isOnComputeEngine()

&& canUseDirectPathWithUniverseDomain()) {

I think there are conditions that we have canUseDirectPath is true but isDirectPathXdsEnabled is false based on the current logic, maybe we can expose isDirectPathXdsEnabled as a public method, and the Spanner team can set a client level attribute based on canUseDirectPath() && isDirectPathXdsEnabled()?

I'll make isDirectPathXdsEnabled() public with @InternalApi annotation.

@surbhigarg92 Would you be fine with the changes above?

@lqiu96 LGTM.

lqiu96 · 2024-05-23T20:18:33Z

Error:

Error:  Failures: 
Error:    OpencensusTracerFactoryTest.testImplicitParentSpan:102 
Wanted but not invoked:
internalTracer.spanBuilderWithExplicitParent(
    <any string>,
    same(Mock for Span, hashCode: 125992315)
);
-> at com.google.api.gax.tracing.OpencensusTracerFactoryTest.testImplicitParentSpan(OpencensusTracerFactoryTest.java:102)

However, there were exactly 2 interactions with this mock:
internalTracer.getCurrentSpan();
-> at com.google.api.gax.tracing.OpencensusTracerFactory.newTracer(OpencensusTracerFactory.java:101)

internalTracer.spanBuilderWithExplicitParent(
    "FakeClient.FakeMethod",
    null
);
-> at com.google.api.gax.tracing.OpencensusTracerFactory.newTracer(OpencensusTracerFactory.java:110)


Error:    OpencensusTracerTest.testLongRunningExample:152 
Argument(s) are different! Wanted:
span.addAnnotation(
    "Operation started",
    {}
);
-> at com.google.api.gax.tracing.OpencensusTracerTest.testLongRunningExample(OpencensusTracerTest.java:152)
Actual invocations have different arguments:
span.addAnnotation(
    "Scheduling next poll",
    {"delay ms" = AttributeValueLong{longValue=5}, "attempt" = AttributeValueLong{longValue=0}, "status" = AttributeValueString{stringValue=OK}}
);
-> at com.google.api.gax.tracing.OpencensusTracer.attemptFailed(OpencensusTracer.java:353)
span.addAnnotation(
    "Operation started"
);
-> at com.google.api.gax.tracing.OpencensusTracer.lroStartSucceeded(OpencensusTracer.java:290)
span.addAnnotation(
    "Polling completed",
    {"attempt" = AttributeValueLong{longValue=1}}
);
-> at com.google.api.gax.tracing.OpencensusTracer.attemptSucceeded(OpencensusTracer.java:323)
span.putAttributes(
    {"attempt count" = AttributeValueLong{longValue=2}}
);
-> at com.google.api.gax.tracing.OpencensusTracer.operationSucceeded(OpencensusTracer.java:254)
span.end(
    
);
-> at com.google.api.gax.tracing.OpencensusTracer.operationSucceeded(OpencensusTracer.java:255)

[INFO] 
Error:  Tests run: 614, Failures: 2, Errors: 0, Skipped: 0

It seems I have somehow broke OpenCensus with this change.

This seems to due to the added mockito-inline dependency used to mock static methods. I'm not sure why it's causing issues in non-related unit tests, so I'm going to remove this and look for a different way to test.

lqiu96 · 2024-05-24T15:53:29Z

gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java

+  static {
+    try {
+      systemProductName =
+          Files.asCharSource(new File("/sys/class/dmi/id/product_name"), StandardCharsets.UTF_8)
+              .readFirstLine();
+    } catch (IOException e) {
+      // Keep existing behavior the same (null means it is not on compute engine)
+      systemProductName = null;
+    }
+  }


SystemProductName logic moved to a static block and initialized once. Stored in a variable so that it can be overriden.

lqiu96 · 2024-05-24T15:53:57Z

gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java

+    /**
+     * Package-Private scope as it is used to test DirectPath functionality in tests. This overrides
+     * the computed systemProductName when the class is initialized.
+     */
+    @VisibleForTesting
+    Builder setSystemProductName(String systemProductName) {
+      this.systemProductName = systemProductName;
+      return this;
+    }


Package-Private setter to allow tests to override the computed SystemProductName

lqiu96 · 2024-05-24T20:03:18Z

Based on #2818, I'll need to remove junit-pioneer and add mocks for EnvProvider.

lqiu96 · 2024-05-24T20:44:25Z

gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java

+   * <p>If productName is null, that represents the result of an IOException
+   */
+  @VisibleForTesting
+  InstantiatingGrpcChannelProvider(Builder builder, String productName) {


Adding the EnvProvider as another param would need to make the envProvider variable non-final. I kept the package-private setter for now until we can migrate to using junit-pioneer to set the env vars.

blakeli0 · 2024-05-24T20:28:29Z

gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java

+              .readFirstLine();
+    } catch (IOException e) {
+      // Keep existing behavior the same (null means it is not on compute engine)
+      systemProductName = null;


Maybe we can return an empty String so that we don't have to do a null check below?

Makes sense. Updating.

blakeli0 · 2024-05-24T21:12:51Z

gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java

+   * @return if DirectPath is enabled for the client AND if the configurations are valid
+   */
+  @InternalApi
+  public boolean canUseDirectPath() {


I think there are conditions that we have canUseDirectPath is true but isDirectPathXdsEnabled is false based on the current logic, maybe we can expose isDirectPathXdsEnabled as a public method, and the Spanner team can set a client level attribute based on canUseDirectPath() && isDirectPathXdsEnabled()?

sonarqubecloud · 2024-05-30T20:24:46Z

Quality Gate failed for 'gapic-generator-java-root'

Failed conditions
71.8% Coverage on New Code (required ≥ 80%)

See analysis details on SonarCloud

sonarqubecloud · 2024-05-30T20:28:19Z

Quality Gate failed for 'java_showcase_integration_tests'

Failed conditions
41.0% Coverage on New Code (required ≥ 80%)

See analysis details on SonarCloud

…2614) Allow the MetricsTracerFactory to take in a second parameter (Map of attributes) that will be treated as client level attributes. These attributes will be added to every single MetricsTracer created throughout the lifecycle of the client. Was able to verify this behavior inside Cloud Monitoring: ![image](https://github.com/googleapis/sdk-platform-java/assets/6621793/832f485e-2834-4765-b381-9a15e2913412) Additional Attribute was recorded. Via: ``` InstantiatingGrpcChannelProvider channelProvider = InstantiatingGrpcChannelProvider.newBuilder().build(); Map<String, String> clientAttributesMapping = new HashMap<>(); clientAttributesMapping.put("directpath_enabled", String.valueOf(channelProvider.canUseDirectPath())); ... options .setApiTracerFactory(new MetricsTracerFactory(recorder, clientAttributesMapping)) .build(); ``` --------- Co-authored-by: Blake Li <blakeli@google.com>

… static method (#2874) The static field `systemProductName` should only be used for testing purposes. In production environment, the logic of checking `isOnComputeEngine` should be exactly the same as before #2614. Fixes b/346215860

🤖 I have created a release *beep* *boop* --- <details><summary>2.42.0</summary> ## [2.42.0](v2.41.0...v2.42.0) (2024-06-25) ### Features * Allow Adding Client Level Attributes to MetricsTracerFactory ([#2614](#2614)) ([f122c6f](f122c6f)) * gapic-generator-java to perform a no-op when no services are detected ([#2460](#2460)) ([c0b5646](c0b5646)) * Make Layout Parser generally available in V1 ([e508ae6](e508ae6)) * populate `.repo-metadata.json` from highest version ([#2890](#2890)) ([f587541](f587541)) * push SNAPSHOT versions of the hermetic build docker image ([#2888](#2888)) ([81df866](81df866)) ### Bug Fixes * **deps:** update the Java code generator (gapic-generator-java) to 1.2.3 ([e508ae6](e508ae6)) * Expose Gax meter name ([#2865](#2865)) ([6c5d6ce](6c5d6ce)) * Move the logic of getting systemProductName from static block to static method ([#2874](#2874)) ([536f1eb](536f1eb)) * Update default Otel Attribute from method_name to method ([#2833](#2833)) ([af10a9e](af10a9e)) ### Dependencies * update dependency com.google.auto.value:auto-value to v1.11.0 ([#2842](#2842)) ([dd27fdf](dd27fdf)) * update dependency com.google.auto.value:auto-value-annotations to v1.11.0 ([#2843](#2843)) ([bf8e67f](bf8e67f)) * update dependency com.google.cloud:grpc-gcp to v1.6.1 ([#2943](#2943)) ([9f16b40](9f16b40)) * update dependency org.checkerframework:checker-qual to v3.44.0 ([#2848](#2848)) ([7a99c50](7a99c50)) * update dependency org.easymock:easymock to v5.3.0 ([#2871](#2871)) ([c243f7d](c243f7d)) * update google api dependencies ([#2846](#2846)) ([b5ef698](b5ef698)) * update googleapis/java-cloud-bom digest to 17cc5ec ([#2882](#2882)) ([d6abd8e](d6abd8e)) * update netty dependencies to v4.1.111.final ([#2877](#2877)) ([b5f10b9](b5f10b9)) * update opentelemetry-java monorepo to v1.39.0 ([#2863](#2863)) ([9d1f3a8](9d1f3a8)) </details> --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com> Co-authored-by: Joe Wang <joewa@google.com>

product-auto-label bot added the size: s Pull request size is small. label Mar 29, 2024

lqiu96 added the owlbot:run Add this label to trigger the Owlbot post processor. label Mar 29, 2024

gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Mar 29, 2024

lqiu96 requested a review from blakeli0 April 1, 2024 19:07

lqiu96 marked this pull request as ready for review April 1, 2024 19:07

lqiu96 requested a review from a team April 1, 2024 19:07

blakeli0 reviewed Apr 2, 2024

View reviewed changes

gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java Show resolved Hide resolved

gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java Show resolved Hide resolved

lqiu96 marked this pull request as draft April 2, 2024 20:47

product-auto-label bot added size: l Pull request size is large. size: m Pull request size is medium. and removed size: s Pull request size is small. size: l Pull request size is large. labels Apr 2, 2024

lqiu96 force-pushed the directpath-compatible-getter branch from 0d9781f to 05b1cde Compare April 2, 2024 21:10

lqiu96 commented Apr 2, 2024

View reviewed changes

lqiu96 changed the title ~~feat: Add DirectPath Compatible Getter to gRPC channel provider~~ feat: Allow Adding Client Level Attributes to MetricsTracerFactory Apr 9, 2024

lqiu96 marked this pull request as ready for review April 9, 2024 21:11

lqiu96 added the owlbot:run Add this label to trigger the Owlbot post processor. label Apr 12, 2024

gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Apr 12, 2024

lqiu96 requested a review from blakeli0 April 12, 2024 21:04

product-auto-label bot added size: l Pull request size is large. and removed size: m Pull request size is medium. labels Apr 16, 2024

lqiu96 commented Apr 18, 2024

View reviewed changes

gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java Show resolved Hide resolved

blakeli0 reviewed Apr 20, 2024

View reviewed changes

surbhigarg92 reviewed May 1, 2024

View reviewed changes

product-auto-label bot added size: xl Pull request size is extra large. and removed size: l Pull request size is large. size: xl Pull request size is extra large. labels May 22, 2024

lqiu96 force-pushed the directpath-compatible-getter branch from 7e641ee to 5896193 Compare May 23, 2024 20:13

lqiu96 added the owlbot:run Add this label to trigger the Owlbot post processor. label May 23, 2024

gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label May 23, 2024

chore: Add junit pioneer to gax-grpc test deps

e11d8fb

lqiu96 added the owlbot:run Add this label to trigger the Owlbot post processor. label May 23, 2024

gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label May 23, 2024

chore: Add additional comments

2b52ce3

lqiu96 commented May 24, 2024

View reviewed changes

lqiu96 added 3 commits May 24, 2024 20:24

Merge branch 'main' into directpath-compatible-getter

2e11ee2

chore: Remove junit-pioneer for env var tests

b3dcdef

chore: Move systemProductName configuration to constructor

626b490

lqiu96 commented May 24, 2024

View reviewed changes

blakeli0 reviewed May 24, 2024

View reviewed changes

lqiu96 added 2 commits May 28, 2024 10:43

chore: Address PR comments

f2f9f39

Merge branch 'main' into directpath-compatible-getter

b272ba4

lqiu96 requested a review from blakeli0 May 28, 2024 14:45

Merge branch 'main' into directpath-compatible-getter

e79dfa5

blakeli0 approved these changes May 31, 2024

View reviewed changes

lqiu96 merged commit f122c6f into main Jun 3, 2024

lqiu96 deleted the directpath-compatible-getter branch June 3, 2024 14:27

release-please bot mentioned this pull request Jun 3, 2024

chore(main): release 2.42.0 #2847

Merged

blakeli0 mentioned this pull request Jun 10, 2024

fix: Move the logic of getting systemProductName from static block to static method #2874

Merged

	@Test
	public void canUseDirectPath_happyPath() {
	InstantiatingGrpcChannelProvider provider =
	InstantiatingGrpcChannelProvider.newBuilder()
	.setEnvProvider(envProvider)
	.setAttemptDirectPath(true)
	.setCredentials(computeEngineCredentials)
	.setSystemProductNameReader(systemProductNameReader)
	.setEndpoint(DEFAULT_ENDPOINT)
	.build();
	Truth.assertThat(provider.canUseDirectPath()).isTrue();
	}

	@Test
	public void canUseDirectPath_directPathEnvVarDisabled() {
	EnvironmentProvider envProvider = Mockito.mock(EnvironmentProvider.class);
	Mockito.when(
	envProvider.getenv(
	InstantiatingGrpcChannelProvider.DIRECT_PATH_ENV_DISABLE_DIRECT_PATH))
	.thenReturn("true");
	InstantiatingGrpcChannelProvider provider =
	InstantiatingGrpcChannelProvider.newBuilder()
	.setEnvProvider(envProvider)
	.setAttemptDirectPath(true)
	.setCredentials(computeEngineCredentials)
	.setSystemProductNameReader(systemProductNameReader)
	.setEndpoint(DEFAULT_ENDPOINT)
	.build();
	Truth.assertThat(provider.canUseDirectPath()).isFalse();
	}

	@Test
	public void canUseDirectPath_directPathEnvVarNotSet_attemptDirectPathIsTrue() {
	EnvironmentProvider envProvider = Mockito.mock(EnvironmentProvider.class);
	// If system property is not set, then System.getProperty() returns null
	Mockito.when(
	envProvider.getenv(
	InstantiatingGrpcChannelProvider.DIRECT_PATH_ENV_DISABLE_DIRECT_PATH))
	.thenReturn(null);
	InstantiatingGrpcChannelProvider provider =
	InstantiatingGrpcChannelProvider.newBuilder()
	.setEnvProvider(envProvider)
	.setAttemptDirectPath(true)
	.setCredentials(computeEngineCredentials)
	.setSystemProductNameReader(systemProductNameReader)
	.setEndpoint(DEFAULT_ENDPOINT)
	.build();
	Truth.assertThat(provider.canUseDirectPath()).isTrue();
	}

	@Test
	public void canUseDirectPath_directPathEnvVarNotSet_attemptDirectPathIsFalse() {
	EnvironmentProvider envProvider = Mockito.mock(EnvironmentProvider.class);
	Mockito.when(
	envProvider.getenv(
	InstantiatingGrpcChannelProvider.DIRECT_PATH_ENV_DISABLE_DIRECT_PATH))
	.thenReturn(null);
	InstantiatingGrpcChannelProvider provider =
	InstantiatingGrpcChannelProvider.newBuilder()
	.setEnvProvider(envProvider)
	.setAttemptDirectPath(false)
	.setCredentials(computeEngineCredentials)
	.setSystemProductNameReader(systemProductNameReader)
	.setEndpoint(DEFAULT_ENDPOINT)
	.build();
	Truth.assertThat(provider.canUseDirectPath()).isFalse();
	}

	@Test
	public void canUseDirectPath_nonComputeCredentials() {
	Credentials credentials = Mockito.mock(Credentials.class);
	InstantiatingGrpcChannelProvider provider =
	InstantiatingGrpcChannelProvider.newBuilder()
	.setEnvProvider(envProvider)
	.setAttemptDirectPath(true)
	.setCredentials(credentials)
	.setSystemProductNameReader(systemProductNameReader)
	.setEndpoint(DEFAULT_ENDPOINT)
	.build();
	Truth.assertThat(provider.canUseDirectPath()).isFalse();
	}

	@Test
	public void canUseDirectPath_systemProductNameInvalid() throws IOException {
	InstantiatingGrpcChannelProvider.SystemProductNameReader systemProductNameReader =
	Mockito.mock(InstantiatingGrpcChannelProvider.SystemProductNameReader.class);
	Mockito.when(systemProductNameReader.getSystemProductName()).thenReturn("testing");
	InstantiatingGrpcChannelProvider provider =
	InstantiatingGrpcChannelProvider.newBuilder()
	.setEnvProvider(envProvider)
	.setAttemptDirectPath(true)
	.setCredentials(computeEngineCredentials)
	.setSystemProductNameReader(systemProductNameReader)
	.setEndpoint(DEFAULT_ENDPOINT)
	.build();
	Truth.assertThat(provider.canUseDirectPath()).isFalse();
	}

	@Test
	public void canUseDirectPath_systemPropertyIsNotLinux() throws IOException {
	System.setProperty("os.name", "Windows");

	InstantiatingGrpcChannelProvider provider =
	InstantiatingGrpcChannelProvider.newBuilder()
	.setEnvProvider(envProvider)
	.setAttemptDirectPath(true)
	.setCredentials(computeEngineCredentials)
	.setSystemProductNameReader(systemProductNameReader)
	.setEndpoint(DEFAULT_ENDPOINT)
	.build();
	Truth.assertThat(provider.canUseDirectPath()).isFalse();
	}

	@Test
	public void canUseDirectPath_systemProductNameThrowsIOException() throws IOException {
	InstantiatingGrpcChannelProvider.SystemProductNameReader systemProductNameReader =
	Mockito.mock(InstantiatingGrpcChannelProvider.SystemProductNameReader.class);
	Mockito.when(systemProductNameReader.getSystemProductName()).thenThrow(new IOException());
	InstantiatingGrpcChannelProvider provider =
	InstantiatingGrpcChannelProvider.newBuilder()
	.setEnvProvider(envProvider)
	.setAttemptDirectPath(true)
	.setCredentials(computeEngineCredentials)
	.setSystemProductNameReader(systemProductNameReader)
	.setEndpoint(DEFAULT_ENDPOINT)
	.build();
	Truth.assertThat(provider.canUseDirectPath()).isFalse();
	}

	@Test
	public void canUseDirectPath_nonGDUUniverseDomain() {
	String nonGDUEndpoint = "test.random.com:443";
	InstantiatingGrpcChannelProvider provider =
	InstantiatingGrpcChannelProvider.newBuilder()
	.setEnvProvider(envProvider)
	.setAttemptDirectPath(true)
	.setCredentials(computeEngineCredentials)
	.setSystemProductNameReader(systemProductNameReader)
	.setEndpoint(nonGDUEndpoint)
	.build();
	Truth.assertThat(provider.canUseDirectPath()).isFalse();
	}

	if (isDirectPathEnabled()
	&& isCredentialDirectPathCompatible()
	&& isOnComputeEngine()
	&& canUseDirectPathWithUniverseDomain()) {

Conversation

lqiu96 commented Mar 29, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud bot commented Apr 23, 2024

Quality Gate failed for 'gapic-generator-java-root'

Uh oh!

sonarqubecloud bot commented Apr 23, 2024

Quality Gate failed for 'java_showcase_integration_tests'

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lqiu96 commented May 23, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lqiu96 commented May 24, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud bot commented May 30, 2024

Quality Gate failed for 'gapic-generator-java-root'

Uh oh!

sonarqubecloud bot commented May 30, 2024

Quality Gate failed for 'java_showcase_integration_tests'

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

lqiu96 commented Mar 29, 2024 •

edited

Loading