Skip to content

Commit 10cd32d

Browse files
BenWhiteheadBenWhitehead
authored
fix: update grpc client side metrics detection to be graceful when not running on gcp (#3097)
Refactor OpenTelemetryBootstrappingUtils to return a channel configurator function rather than mutating a passed in value. Add ChannelConfigurator interface to make testing easier and to simplify composition with any existing channel configurator function present in the InstantiatingGrpcChannelProvider.
1 parent 4fdb1d4 commit 10cd32d

File tree

4 files changed

+189
-37
lines changed

4 files changed

+189
-37
lines changed

‎google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageOptions.java

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@
6060
import com.google.cloud.spi.ServiceRpcFactory;
6161
import com.google.cloud.storage.GrpcUtils.ZeroCopyBidiStreamingCallable;
6262
import com.google.cloud.storage.Hasher.UncheckedChecksumMismatchException;
63+
import com.google.cloud.storage.OpenTelemetryBootstrappingUtils.ChannelConfigurator;
6364
import com.google.cloud.storage.RetryContext.RetryContextProvider;
6465
import com.google.cloud.storage.Retrying.DefaultRetrier;
6566
import com.google.cloud.storage.Storage.BlobWriteOption;
@@ -332,12 +333,14 @@ private Tuple<StorageSettings, Opts<UserProject>> resolveSettingsAndOpts() throw
332333
}
333334

334335
if (enableGrpcClientMetrics) {
335-
OpenTelemetryBootstrappingUtils.enableGrpcMetrics(
336-
channelProviderBuilder,
337-
endpoint,
338-
this.getProjectId(),
339-
this.getUniverseDomain(),
340-
!grpcClientMetricsManuallyEnabled);
336+
ChannelConfigurator channelConfigurator =
337+
OpenTelemetryBootstrappingUtils.enableGrpcMetrics(
338+
ChannelConfigurator.lift(channelProviderBuilder.getChannelConfigurator()),
339+
endpoint,
340+
this.getProjectId(),
341+
this.getUniverseDomain(),
342+
!grpcClientMetricsManuallyEnabled);
343+
channelProviderBuilder.setChannelConfigurator(channelConfigurator);
341344
}
342345

343346
builder.setTransportChannelProvider(channelProviderBuilder.build());

‎google-cloud-storage/src/main/java/com/google/cloud/storage/OpenTelemetryBootstrappingUtils.java

Lines changed: 75 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717
package com.google.cloud.storage;
1818

1919
import com.google.api.core.ApiFunction;
20-
import com.google.api.gax.grpc.InstantiatingGrpcChannelProvider;
2120
import com.google.api.gax.rpc.PermissionDeniedException;
2221
import com.google.api.gax.rpc.UnavailableException;
2322
import com.google.cloud.opentelemetry.metric.GoogleCloudMetricExporter;
@@ -59,6 +58,8 @@
5958
import java.util.concurrent.atomic.AtomicBoolean;
6059
import java.util.logging.Logger;
6160
import java.util.stream.Collectors;
61+
import org.checkerframework.checker.nullness.qual.NonNull;
62+
import org.checkerframework.checker.nullness.qual.Nullable;
6263

6364
final class OpenTelemetryBootstrappingUtils {
6465
private static final Collection<String> METRICS_TO_ENABLE =
@@ -88,15 +89,41 @@ final class OpenTelemetryBootstrappingUtils {
8889

8990
static final Logger log = Logger.getLogger(OpenTelemetryBootstrappingUtils.class.getName());
9091

91-
static void enableGrpcMetrics(
92-
InstantiatingGrpcChannelProvider.Builder channelProviderBuilder,
92+
@NonNull
93+
static ChannelConfigurator enableGrpcMetrics(
94+
@Nullable ChannelConfigurator channelConfigurator,
9395
String endpoint,
94-
String projectId,
96+
@Nullable String projectId,
9597
String universeDomain,
9698
boolean shouldSuppressExceptions) {
99+
GCPResourceProvider resourceProvider = new GCPResourceProvider();
100+
Attributes detectedAttributes = resourceProvider.getAttributes();
101+
102+
@Nullable String detectedProjectId =
103+
detectedAttributes.get(AttributeKey.stringKey("cloud.account.id"));
104+
if (projectId == null && detectedProjectId == null) {
105+
log.warning(
106+
"Unable to determine the Project ID in order to report metrics. No gRPC client metrics will be reported.");
107+
return channelConfigurator != null ? channelConfigurator : ChannelConfigurator.identity();
108+
}
109+
110+
String projectIdToUse = detectedProjectId == null ? projectId : detectedProjectId;
111+
if (!projectIdToUse.equals(projectId)) {
112+
log.warning(
113+
"The Project ID configured for gRPC client metrics is "
114+
+ projectIdToUse
115+
+ ", but the Project ID of the storage client is "
116+
+ projectId
117+
+ ". Make sure that the service account in use has the required metric writing role "
118+
+ "(roles/monitoring.metricWriter) in the project "
119+
+ projectIdToUse
120+
+ ", or metrics will not be written.");
121+
}
122+
97123
String metricServiceEndpoint = getCloudMonitoringEndpoint(endpoint, universeDomain);
98124
SdkMeterProvider provider =
99-
createMeterProvider(metricServiceEndpoint, projectId, shouldSuppressExceptions);
125+
createMeterProvider(
126+
metricServiceEndpoint, projectIdToUse, detectedAttributes, shouldSuppressExceptions);
100127

101128
OpenTelemetrySdk openTelemetrySdk =
102129
OpenTelemetrySdk.builder().setMeterProvider(provider).build();
@@ -106,16 +133,48 @@ static void enableGrpcMetrics(
106133
.addOptionalLabel("grpc.lb.locality")
107134
.enableMetrics(METRICS_TO_ENABLE)
108135
.build();
109-
ApiFunction<ManagedChannelBuilder, ManagedChannelBuilder> channelConfigurator =
110-
channelProviderBuilder.getChannelConfigurator();
111-
channelProviderBuilder.setChannelConfigurator(
136+
ChannelConfigurator otelConfigurator =
112137
b -> {
113138
grpcOpenTelemetry.configureChannelBuilder(b);
114-
if (channelConfigurator != null) {
115-
return channelConfigurator.apply(b);
116-
}
117139
return b;
118-
});
140+
};
141+
return otelConfigurator.andThen(channelConfigurator);
142+
}
143+
144+
@SuppressWarnings("rawtypes") // ManagedChannelBuilder
145+
@FunctionalInterface
146+
interface ChannelConfigurator extends ApiFunction<ManagedChannelBuilder, ManagedChannelBuilder> {
147+
@NonNull
148+
default ChannelConfigurator andThen(@Nullable ChannelConfigurator then) {
149+
if (then == null) {
150+
return this;
151+
}
152+
return b -> then.apply(this.apply(b));
153+
}
154+
155+
static ChannelConfigurator identity() {
156+
return IdentityChannelConfigurator.INSTANCE;
157+
}
158+
159+
static ChannelConfigurator lift(
160+
@Nullable ApiFunction<ManagedChannelBuilder, ManagedChannelBuilder> f) {
161+
if (f == null) {
162+
return identity();
163+
}
164+
return f::apply;
165+
}
166+
}
167+
168+
@SuppressWarnings("rawtypes") // ManagedChannelBuilder
169+
private static final class IdentityChannelConfigurator implements ChannelConfigurator {
170+
private static final IdentityChannelConfigurator INSTANCE = new IdentityChannelConfigurator();
171+
172+
private IdentityChannelConfigurator() {}
173+
174+
@Override
175+
public ManagedChannelBuilder apply(ManagedChannelBuilder input) {
176+
return input;
177+
}
119178
}
120179

121180
@VisibleForTesting
@@ -147,24 +206,10 @@ static String getCloudMonitoringEndpoint(String endpoint, String universeDomain)
147206

148207
@VisibleForTesting
149208
static SdkMeterProvider createMeterProvider(
150-
String metricServiceEndpoint, String projectId, boolean shouldSuppressExceptions) {
151-
GCPResourceProvider resourceProvider = new GCPResourceProvider();
152-
Attributes detectedAttributes = resourceProvider.getAttributes();
153-
154-
String detectedProjectId = detectedAttributes.get(AttributeKey.stringKey("cloud.account.id"));
155-
String projectIdToUse = detectedProjectId == null ? projectId : detectedProjectId;
156-
157-
if (!projectIdToUse.equals(projectId)) {
158-
log.warning(
159-
"The Project ID configured for metrics is "
160-
+ projectIdToUse
161-
+ ", but the Project ID of the storage client is "
162-
+ projectId
163-
+ ". Make sure that the service account in use has the required metric writing role "
164-
+ "(roles/monitoring.metricWriter) in the project "
165-
+ projectIdToUse
166-
+ ", or metrics will not be written.");
167-
}
209+
String metricServiceEndpoint,
210+
String projectIdToUse,
211+
Attributes detectedAttributes,
212+
boolean shouldSuppressExceptions) {
168213

169214
MonitoredResourceDescription monitoredResourceDescription =
170215
new MonitoredResourceDescription(

‎google-cloud-storage/src/test/java/com/google/cloud/storage/ITGrpcMetricsTest.java

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@
2121
import com.google.cloud.storage.it.runner.StorageITRunner;
2222
import com.google.cloud.storage.it.runner.annotations.Backend;
2323
import com.google.cloud.storage.it.runner.annotations.SingleBackend;
24+
import io.opentelemetry.api.common.Attributes;
25+
import io.opentelemetry.contrib.gcp.resource.GCPResourceProvider;
2426
import io.opentelemetry.sdk.metrics.SdkMeterProvider;
2527
import org.junit.Test;
2628
import org.junit.runner.RunWith;
@@ -36,9 +38,14 @@ public void testGrpcMetrics() {
3638
"storage.googleapis.com:443", "storage.googleapis.com"))
3739
.isEqualTo("monitoring.googleapis.com:443");
3840

41+
GCPResourceProvider resourceProvider = new GCPResourceProvider();
42+
Attributes detectedAttributes = resourceProvider.getAttributes();
3943
SdkMeterProvider provider =
4044
OpenTelemetryBootstrappingUtils.createMeterProvider(
41-
"monitoring.googleapis.com:443", grpcStorageOptions.getProjectId(), false);
45+
"monitoring.googleapis.com:443",
46+
grpcStorageOptions.getProjectId(),
47+
detectedAttributes,
48+
false);
4249

4350
/*
4451
* SDKMeterProvider doesn't expose the relevant fields we want to test, but they are present in

‎google-cloud-storage/src/test/java/com/google/cloud/storage/OpenTelemetryBootstrappingUtilsTest.java

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
/*
2+
* Copyright 2025 Google LLC
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package com.google.cloud.storage;
18+
19+
import static com.google.common.truth.Truth.assertThat;
20+
import static org.junit.Assume.assumeFalse;
21+
import static org.mockito.Mockito.mock;
22+
23+
import com.google.cloud.storage.OpenTelemetryBootstrappingUtils.ChannelConfigurator;
24+
import io.grpc.ManagedChannelBuilder;
25+
import java.util.concurrent.atomic.AtomicBoolean;
26+
import org.junit.Test;
27+
28+
public final class OpenTelemetryBootstrappingUtilsTest {
29+
30+
@Test
31+
public void noErrorIfNotRunningOnGcp() {
32+
assumeFalse("Skipping because running on GCP", TestUtils.isOnComputeEngine());
33+
34+
ChannelConfigurator cc = ChannelConfigurator.identity();
35+
36+
String endpoint = "storage.googleapis.com:443";
37+
String projectId = null;
38+
String universeDomain = null;
39+
ChannelConfigurator actual =
40+
OpenTelemetryBootstrappingUtils.enableGrpcMetrics(
41+
cc, endpoint, projectId, universeDomain, true);
42+
43+
assertThat(actual).isSameInstanceAs(cc);
44+
}
45+
46+
@SuppressWarnings("rawtypes") // ManagedChannelBuilder
47+
@Test
48+
public void channelConfigurator_andThen() {
49+
ManagedChannelBuilder b1 = mock(ManagedChannelBuilder.class, "b1");
50+
ManagedChannelBuilder b2 = mock(ManagedChannelBuilder.class, "b2");
51+
ManagedChannelBuilder b3 = mock(ManagedChannelBuilder.class, "b2");
52+
53+
ChannelConfigurator cc1 =
54+
b -> {
55+
assertThat(b).isSameInstanceAs(b1);
56+
return b2;
57+
};
58+
ChannelConfigurator cc2 =
59+
b -> {
60+
assertThat(b).isSameInstanceAs(b2);
61+
return b3;
62+
};
63+
64+
ChannelConfigurator cc3 = cc1.andThen(cc2);
65+
66+
ManagedChannelBuilder apply = cc3.apply(b1);
67+
assertThat(apply).isSameInstanceAs(b3);
68+
}
69+
70+
@Test
71+
public void channelConfigurator_lift_nullToIdentity() {
72+
ChannelConfigurator actual = ChannelConfigurator.lift(null);
73+
assertThat(actual).isSameInstanceAs(ChannelConfigurator.identity());
74+
}
75+
76+
@SuppressWarnings("rawtypes") // ManagedChannelBuilder
77+
@Test
78+
public void channelConfigurator_lift_plumbingWorks() {
79+
ManagedChannelBuilder b1 = mock(ManagedChannelBuilder.class, "b1");
80+
AtomicBoolean called = new AtomicBoolean(false);
81+
ChannelConfigurator lifted =
82+
ChannelConfigurator.lift(
83+
b -> {
84+
called.compareAndSet(false, true);
85+
return b;
86+
});
87+
ManagedChannelBuilder actual = lifted.apply(b1);
88+
assertThat(actual).isSameInstanceAs(b1);
89+
assertThat(called.get()).isTrue();
90+
}
91+
92+
@Test
93+
public void channelConfigurator_andThen_nullsafe() {
94+
ChannelConfigurator actual = ChannelConfigurator.identity().andThen(null);
95+
assertThat(actual).isSameInstanceAs(ChannelConfigurator.identity());
96+
}
97+
}

0 commit comments

Comments
 (0)