Skip to content

Commit 4a51556

Browse files
igorbernstein2igorbernstein2
authored
chore: create a separate otel instance for internal metrics (#2522)
* chore: create a separate otel instance for internal metrics Building on #2515, this creates a private instance of otel for internal metrics. This is necessary for a follow up pr that will introduce a new internal monitored resource Changes: - create a separate setting for disabling internal metrics (since internal metrics will not support customer provided exporters) - create internal metrics as part of BigtableClientContext creation - split off internal views Change-Id: Iaafc327f3f008bcc29510594210e0914342f84fe * fix typo Change-Id: Ia28cb95408ecf7e8ab785c5f54c0149388df202a * try to deflake the metrics test again Change-Id: I9f99e1a0f75f1337d09ba3833fe7833e03818519 * improve comment Change-Id: I3376b36bf41c9395b4fcd62c7905312dd2c3a6c9
1 parent 1ecc267 commit 4a51556

File tree

11 files changed

+150
-39
lines changed

11 files changed

+150
-39
lines changed

‎google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/BigtableDataSettings.java

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,7 @@ public static Builder newBuilderForEmulator(String hostname, int port) {
128128
// disable channel refreshing when creating an emulator
129129
.setRefreshingChannel(false)
130130
.setMetricsProvider(NoopMetricsProvider.INSTANCE) // disable exporting metrics for emulator
131+
.disableInternalMetrics()
131132
.setTransportChannelProvider(
132133
InstantiatingGrpcChannelProvider.newBuilder()
133134
.setMaxInboundMessageSize(256 * 1024 * 1024)
@@ -297,6 +298,11 @@ public MetricsProvider getMetricsProvider() {
297298
return stubSettings.getMetricsProvider();
298299
}
299300

301+
/** Checks if internal metrics are enabled */
302+
public boolean areInternalMetricsEnabled() {
303+
return stubSettings.areInternalMetricsEnabled();
304+
}
305+
300306
/** Returns the underlying RPC settings. */
301307
public EnhancedBigtableStubSettings getStubSettings() {
302308
return stubSettings;
@@ -570,6 +576,15 @@ public MetricsProvider getMetricsProvider() {
570576
return stubSettings.getMetricsProvider();
571577
}
572578

579+
public Builder disableInternalMetrics() {
580+
stubSettings.disableInternalMetrics();
581+
return this;
582+
}
583+
584+
public boolean areInternalMetricsEnabled() {
585+
return stubSettings.areInternalMetricsEnabled();
586+
}
587+
573588
/**
574589
* Returns the underlying settings for making RPC calls. The settings should be changed with
575590
* care.

‎google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/BigtableClientContext.java

Lines changed: 30 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
import com.google.cloud.bigtable.data.v2.stub.metrics.NoopMetricsProvider;
3434
import io.grpc.ManagedChannelBuilder;
3535
import io.opentelemetry.api.OpenTelemetry;
36+
import io.opentelemetry.sdk.OpenTelemetrySdk;
3637
import java.io.IOException;
3738
import java.net.URI;
3839
import java.net.URISyntaxException;
@@ -50,6 +51,7 @@ public class BigtableClientContext {
5051
private static final Logger logger = Logger.getLogger(BigtableClientContext.class.getName());
5152

5253
@Nullable private final OpenTelemetry openTelemetry;
54+
@Nullable private final OpenTelemetrySdk internalOpenTelemetry;
5355
private final ClientContext clientContext;
5456

5557
public static BigtableClientContext create(EnhancedBigtableStubSettings settings)
@@ -84,18 +86,31 @@ public static BigtableClientContext create(EnhancedBigtableStubSettings settings
8486
? ((InstantiatingGrpcChannelProvider) builder.getTransportChannelProvider()).toBuilder()
8587
: null;
8688

87-
ErrorCountPerConnectionMetricTracker errorCountPerConnectionMetricTracker = null;
89+
@Nullable OpenTelemetrySdk internalOtel = null;
90+
@Nullable ErrorCountPerConnectionMetricTracker errorCountPerConnectionMetricTracker = null;
91+
92+
// Internal metrics are scoped to the connections, so we need a mutable transportProvider,
93+
// otherwise there is
94+
// no reason to build the internal OtelProvider
95+
if (transportProvider != null) {
96+
internalOtel =
97+
settings
98+
.getInternalMetricsProvider()
99+
.createOtelProvider(credentials, settings.getMetricsEndpoint());
100+
if (internalOtel != null) {
101+
// Set up per connection error count tracker if all dependencies are met:
102+
// a configurable transport provider + otel
103+
errorCountPerConnectionMetricTracker =
104+
setupPerConnectionErrorTracer(builder, transportProvider, internalOtel);
105+
}
106+
}
88107

89108
if (transportProvider != null) {
90109
// Set up cookie holder if routing cookie is enabled
91110
if (builder.getEnableRoutingCookie()) {
92111
setupCookieHolder(transportProvider);
93112
}
94-
// Set up per connection error count tracker if OpenTelemetry is not null
95-
if (openTelemetry != null) {
96-
errorCountPerConnectionMetricTracker =
97-
setupPerConnectionErrorTracer(builder, transportProvider, openTelemetry);
98-
}
113+
99114
// Inject channel priming if enabled
100115
if (builder.isRefreshingChannel()) {
101116
transportProvider.setChannelPrimer(
@@ -117,12 +132,16 @@ public static BigtableClientContext create(EnhancedBigtableStubSettings settings
117132
clientContext.getExecutor());
118133
}
119134

120-
return new BigtableClientContext(clientContext, openTelemetry);
135+
return new BigtableClientContext(clientContext, openTelemetry, internalOtel);
121136
}
122137

123-
private BigtableClientContext(ClientContext clientContext, OpenTelemetry openTelemetry) {
138+
private BigtableClientContext(
139+
ClientContext clientContext,
140+
OpenTelemetry openTelemetry,
141+
@Nullable OpenTelemetrySdk internalOtel) {
124142
this.clientContext = clientContext;
125143
this.openTelemetry = openTelemetry;
144+
this.internalOpenTelemetry = internalOtel;
126145
}
127146

128147
public OpenTelemetry getOpenTelemetry() {
@@ -137,6 +156,9 @@ public void close() throws Exception {
137156
for (BackgroundResource resource : clientContext.getBackgroundResources()) {
138157
resource.close();
139158
}
159+
if (internalOpenTelemetry != null) {
160+
internalOpenTelemetry.close();
161+
}
140162
}
141163

142164
private static OpenTelemetry getOpenTelemetryFromMetricsProvider(

‎google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubSettings.java

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
import com.google.api.gax.rpc.StubSettings;
3333
import com.google.api.gax.rpc.TransportChannelProvider;
3434
import com.google.api.gax.rpc.UnaryCallSettings;
35+
import com.google.auth.Credentials;
3536
import com.google.bigtable.v2.FeatureFlags;
3637
import com.google.bigtable.v2.PingAndWarmRequest;
3738
import com.google.cloud.bigtable.Version;
@@ -50,13 +51,15 @@
5051
import com.google.cloud.bigtable.data.v2.models.sql.BoundStatement;
5152
import com.google.cloud.bigtable.data.v2.stub.metrics.DefaultMetricsProvider;
5253
import com.google.cloud.bigtable.data.v2.stub.metrics.MetricsProvider;
54+
import com.google.cloud.bigtable.data.v2.stub.metrics.Util;
5355
import com.google.cloud.bigtable.data.v2.stub.mutaterows.MutateRowsBatchingDescriptor;
5456
import com.google.cloud.bigtable.data.v2.stub.readrows.ReadRowsBatchingDescriptor;
5557
import com.google.common.base.MoreObjects;
5658
import com.google.common.base.Preconditions;
5759
import com.google.common.collect.ImmutableList;
5860
import com.google.common.collect.ImmutableMap;
5961
import com.google.common.collect.ImmutableSet;
62+
import io.opentelemetry.sdk.OpenTelemetrySdk;
6063
import java.io.ByteArrayOutputStream;
6164
import java.io.IOException;
6265
import java.nio.charset.StandardCharsets;
@@ -283,6 +286,7 @@ public class EnhancedBigtableStubSettings extends StubSettings<EnhancedBigtableS
283286

284287
private final MetricsProvider metricsProvider;
285288
@Nullable private final String metricsEndpoint;
289+
@Nonnull private final InternalMetricsProvider internalMetricsProvider;
286290

287291
private EnhancedBigtableStubSettings(Builder builder) {
288292
super(builder);
@@ -313,6 +317,7 @@ private EnhancedBigtableStubSettings(Builder builder) {
313317
enableSkipTrailers = builder.enableSkipTrailers;
314318
metricsProvider = builder.metricsProvider;
315319
metricsEndpoint = builder.metricsEndpoint;
320+
internalMetricsProvider = builder.internalMetricsProvider;
316321

317322
// Per method settings.
318323
readRowsSettings = builder.readRowsSettings.build();
@@ -411,6 +416,14 @@ public String getMetricsEndpoint() {
411416
return metricsEndpoint;
412417
}
413418

419+
public boolean areInternalMetricsEnabled() {
420+
return internalMetricsProvider == DEFAULT_INTERNAL_OTEL_PROVIDER;
421+
}
422+
423+
InternalMetricsProvider getInternalMetricsProvider() {
424+
return internalMetricsProvider;
425+
}
426+
414427
/** Returns a builder for the default ChannelProvider for this service. */
415428
public static InstantiatingGrpcChannelProvider.Builder defaultGrpcTransportProviderBuilder() {
416429
InstantiatingGrpcChannelProvider.Builder grpcTransportProviderBuilder =
@@ -761,6 +774,7 @@ public static class Builder extends StubSettings.Builder<EnhancedBigtableStubSet
761774

762775
private MetricsProvider metricsProvider;
763776
@Nullable private String metricsEndpoint;
777+
private InternalMetricsProvider internalMetricsProvider;
764778

765779
/**
766780
* Initializes a new Builder with sane defaults for all settings.
@@ -780,6 +794,7 @@ private Builder() {
780794
this.enableRetryInfo = true;
781795
this.enableSkipTrailers = SKIP_TRAILERS;
782796
metricsProvider = DefaultMetricsProvider.INSTANCE;
797+
this.internalMetricsProvider = DEFAULT_INTERNAL_OTEL_PROVIDER;
783798

784799
// Defaults provider
785800
BigtableStubSettings.Builder baseDefaults = BigtableStubSettings.newBuilder();
@@ -918,6 +933,7 @@ private Builder(EnhancedBigtableStubSettings settings) {
918933
enableRetryInfo = settings.enableRetryInfo;
919934
metricsProvider = settings.metricsProvider;
920935
metricsEndpoint = settings.getMetricsEndpoint();
936+
internalMetricsProvider = settings.internalMetricsProvider;
921937

922938
// Per method settings.
923939
readRowsSettings = settings.readRowsSettings.toBuilder();
@@ -1105,6 +1121,23 @@ public String getMetricsEndpoint() {
11051121
return metricsEndpoint;
11061122
}
11071123

1124+
/** Disable collection of internal metrics that help google detect issues accessing Bigtable. */
1125+
public Builder disableInternalMetrics() {
1126+
return setInternalMetricsProvider(DISABLED_INTERNAL_OTEL_PROVIDER);
1127+
}
1128+
1129+
// For testing
1130+
@InternalApi
1131+
public Builder setInternalMetricsProvider(InternalMetricsProvider internalMetricsProvider) {
1132+
this.internalMetricsProvider = internalMetricsProvider;
1133+
return this;
1134+
}
1135+
1136+
/** Checks if internal metrics are disabled */
1137+
public boolean areInternalMetricsEnabled() {
1138+
return internalMetricsProvider == DISABLED_INTERNAL_OTEL_PROVIDER;
1139+
}
1140+
11081141
@InternalApi("Used for internal testing")
11091142
public Map<String, String> getJwtAudienceMapping() {
11101143
return jwtAudienceMapping;
@@ -1304,7 +1337,21 @@ public String toString() {
13041337
.add("prepareQuerySettings", prepareQuerySettings)
13051338
.add("metricsProvider", metricsProvider)
13061339
.add("metricsEndpoint", metricsEndpoint)
1340+
.add("areInternalMetricsEnabled", internalMetricsProvider == DEFAULT_INTERNAL_OTEL_PROVIDER)
13071341
.add("parent", super.toString())
13081342
.toString();
13091343
}
1344+
1345+
@InternalApi
1346+
@FunctionalInterface
1347+
public interface InternalMetricsProvider {
1348+
@Nullable
1349+
OpenTelemetrySdk createOtelProvider(Credentials creds, @Nullable String endpoint)
1350+
throws IOException;
1351+
}
1352+
1353+
private static final InternalMetricsProvider DEFAULT_INTERNAL_OTEL_PROVIDER =
1354+
Util::newInternalOpentelemetry;
1355+
private static final InternalMetricsProvider DISABLED_INTERNAL_OTEL_PROVIDER =
1356+
(ignored1, ignored2) -> null;
13101357
}

‎google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableCloudMonitoringExporter.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -296,7 +296,7 @@ static class PublicTimeSeriesConverter implements TimeSeriesConverter {
296296
private final String taskId;
297297

298298
PublicTimeSeriesConverter() {
299-
this(BigtableExporterUtils.DEFAULT_TABLE_VALUE.get());
299+
this(BigtableExporterUtils.DEFAULT_TASK_VALUE.get());
300300
}
301301

302302
PublicTimeSeriesConverter(String taskId) {
@@ -326,7 +326,7 @@ static class InternalTimeSeriesConverter implements TimeSeriesConverter {
326326
private final Supplier<MonitoredResource> monitoredResource;
327327

328328
InternalTimeSeriesConverter(Supplier<MonitoredResource> monitoredResource) {
329-
this(monitoredResource, BigtableExporterUtils.DEFAULT_TABLE_VALUE.get());
329+
this(monitoredResource, BigtableExporterUtils.DEFAULT_TASK_VALUE.get());
330330
}
331331

332332
InternalTimeSeriesConverter(Supplier<MonitoredResource> monitoredResource, String taskId) {

‎google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableExporterUtils.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ private BigtableExporterUtils() {}
9595
*/
9696
private static String defaultTaskValue = null;
9797

98-
static final Supplier<String> DEFAULT_TABLE_VALUE =
98+
static final Supplier<String> DEFAULT_TASK_VALUE =
9999
Suppliers.memoize(BigtableExporterUtils::computeDefaultTaskValue);
100100

101101
private static String computeDefaultTaskValue() {

‎google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsConstants.java

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,20 @@ static void defineView(
140140
viewMap.put(selector, view);
141141
}
142142

143+
public static Map<InstrumentSelector, View> getInternalViews() {
144+
ImmutableMap.Builder<InstrumentSelector, View> views = ImmutableMap.builder();
145+
defineView(
146+
views,
147+
PER_CONNECTION_ERROR_COUNT_NAME,
148+
AGGREGATION_PER_CONNECTION_ERROR_COUNT_HISTOGRAM,
149+
InstrumentType.HISTOGRAM,
150+
"1",
151+
ImmutableSet.<AttributeKey>builder()
152+
.add(BIGTABLE_PROJECT_ID_KEY, INSTANCE_ID_KEY, APP_PROFILE_KEY, CLIENT_NAME_KEY)
153+
.build());
154+
return views.build();
155+
}
156+
143157
public static Map<InstrumentSelector, View> getAllViews() {
144158
ImmutableMap.Builder<InstrumentSelector, View> views = ImmutableMap.builder();
145159

@@ -205,16 +219,6 @@ public static Map<InstrumentSelector, View> getAllViews() {
205219
InstrumentType.COUNTER,
206220
"1",
207221
ImmutableSet.<AttributeKey>builder().addAll(COMMON_ATTRIBUTES).add(STATUS_KEY).build());
208-
209-
defineView(
210-
views,
211-
PER_CONNECTION_ERROR_COUNT_NAME,
212-
AGGREGATION_PER_CONNECTION_ERROR_COUNT_HISTOGRAM,
213-
InstrumentType.HISTOGRAM,
214-
"1",
215-
ImmutableSet.<AttributeKey>builder()
216-
.add(BIGTABLE_PROJECT_ID_KEY, INSTANCE_ID_KEY, APP_PROFILE_KEY, CLIENT_NAME_KEY)
217-
.build());
218222
defineView(
219223
views,
220224
REMAINING_DEADLINE_NAME,

‎google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsView.java

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717

1818
import com.google.auth.Credentials;
1919
import com.google.auth.oauth2.GoogleCredentials;
20-
import com.google.common.base.Suppliers;
2120
import io.opentelemetry.sdk.metrics.InstrumentSelector;
2221
import io.opentelemetry.sdk.metrics.SdkMeterProviderBuilder;
2322
import io.opentelemetry.sdk.metrics.View;
@@ -106,20 +105,11 @@ public static void registerBuiltinMetrics(
106105
credentials,
107106
endpoint,
108107
new BigtableCloudMonitoringExporter.PublicTimeSeriesConverter());
109-
MetricExporter internalExporter =
110-
BigtableCloudMonitoringExporter.create(
111-
"application metrics",
112-
credentials,
113-
endpoint,
114-
new BigtableCloudMonitoringExporter.InternalTimeSeriesConverter(
115-
Suppliers.memoize(BigtableExporterUtils::detectResourceSafe)));
116108

117109
for (Map.Entry<InstrumentSelector, View> entry :
118110
BuiltinMetricsConstants.getAllViews().entrySet()) {
119111
builder.registerView(entry.getKey(), entry.getValue());
120112
}
121-
builder
122-
.registerMetricReader(PeriodicMetricReader.create(publicExporter))
123-
.registerMetricReader(PeriodicMetricReader.create(internalExporter));
113+
builder.registerMetricReader(PeriodicMetricReader.create(publicExporter));
124114
}
125115
}

‎google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/Util.java

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import com.google.api.gax.rpc.ApiException;
2323
import com.google.api.gax.rpc.StatusCode;
2424
import com.google.api.gax.rpc.StatusCode.Code;
25+
import com.google.auth.Credentials;
2526
import com.google.bigtable.v2.AuthorizedViewName;
2627
import com.google.bigtable.v2.CheckAndMutateRowRequest;
2728
import com.google.bigtable.v2.GenerateInitialChangeStreamPartitionsRequest;
@@ -33,6 +34,7 @@
3334
import com.google.bigtable.v2.ResponseParams;
3435
import com.google.bigtable.v2.SampleRowKeysRequest;
3536
import com.google.bigtable.v2.TableName;
37+
import com.google.common.base.Suppliers;
3638
import com.google.common.collect.ImmutableMap;
3739
import com.google.protobuf.InvalidProtocolBufferException;
3840
import io.grpc.CallOptions;
@@ -41,6 +43,13 @@
4143
import io.grpc.StatusException;
4244
import io.grpc.StatusRuntimeException;
4345
import io.opencensus.tags.TagValue;
46+
import io.opentelemetry.sdk.OpenTelemetrySdk;
47+
import io.opentelemetry.sdk.metrics.InstrumentSelector;
48+
import io.opentelemetry.sdk.metrics.SdkMeterProvider;
49+
import io.opentelemetry.sdk.metrics.SdkMeterProviderBuilder;
50+
import io.opentelemetry.sdk.metrics.View;
51+
import io.opentelemetry.sdk.metrics.export.PeriodicMetricReader;
52+
import java.io.IOException;
4453
import java.time.Instant;
4554
import java.time.temporal.ChronoUnit;
4655
import java.util.Arrays;
@@ -237,4 +246,24 @@ static GrpcCallContext injectBigtableStreamTracer(
237246
throw new RuntimeException("Unexpected context class: " + context.getClass().getName());
238247
}
239248
}
249+
250+
public static OpenTelemetrySdk newInternalOpentelemetry(
251+
Credentials credentials, @Nullable String metricsEndpoint) throws IOException {
252+
SdkMeterProviderBuilder meterProviderBuilder = SdkMeterProvider.builder();
253+
254+
for (Map.Entry<InstrumentSelector, View> e :
255+
BuiltinMetricsConstants.getInternalViews().entrySet()) {
256+
meterProviderBuilder.registerView(e.getKey(), e.getValue());
257+
}
258+
259+
meterProviderBuilder.registerMetricReader(
260+
PeriodicMetricReader.create(
261+
BigtableCloudMonitoringExporter.create(
262+
"application metrics",
263+
credentials,
264+
metricsEndpoint,
265+
new BigtableCloudMonitoringExporter.InternalTimeSeriesConverter(
266+
Suppliers.memoize(BigtableExporterUtils::detectResourceSafe)))));
267+
return OpenTelemetrySdk.builder().setMeterProvider(meterProviderBuilder.build()).build();
268+
}
240269
}

0 commit comments

Comments
 (0)