Skip to content

Commit

Permalink
fix: support override monitoring endpoint (#2364)
Browse files Browse the repository at this point in the history
* fix: support override monitoring endpoint

* format

* update default

* add nullable annotation

* update

* update
  • Loading branch information
mutianf authored Oct 1, 2024
1 parent 9d24c45 commit a341eb8
Show file tree
Hide file tree
Showing 8 changed files with 113 additions and 24 deletions.
10 changes: 10 additions & 0 deletions google-cloud-bigtable/clirr-ignored-differences.xml
Original file line number Diff line number Diff line change
Expand Up @@ -265,4 +265,14 @@
<className>com/google/cloud/bigtable/admin/v2/stub/EnhancedBigtableTableAdminStub</className>
<method>*</method>
</difference>
<difference>
<differenceType>7004</differenceType>
<className>com/google/cloud/bigtable/data/v2/stub/metrics/BigtableCloudMonitoringExporter</className>
<method>*</method>
</difference>
<difference>
<differenceType>7004</differenceType>
<className>com/google/cloud/bigtable/data/v2/stub/metrics/DefaultMetricsProvider</className>
<method>*</method>
</difference>
</differences>
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,8 @@ public static BigtableDataClientFactory create(BigtableDataSettings defaultSetti
EnhancedBigtableStub.getOpenTelemetry(
defaultSettings.getProjectId(),
defaultSettings.getMetricsProvider(),
sharedClientContext.getCredentials());
sharedClientContext.getCredentials(),
defaultSettings.getStubSettings().getMetricsEndpoint());
} catch (Throwable t) {
logger.log(Level.WARNING, "Failed to get OTEL, will skip exporting client side metrics", t);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,8 @@ public static EnhancedBigtableStub create(EnhancedBigtableStubSettings settings)
getOpenTelemetry(
settings.getProjectId(),
settings.getMetricsProvider(),
clientContext.getCredentials());
clientContext.getCredentials(),
settings.getMetricsEndpoint());
} catch (Throwable t) {
logger.log(Level.WARNING, "Failed to get OTEL, will skip exporting client side metrics", t);
}
Expand Down Expand Up @@ -268,7 +269,11 @@ public static ClientContext createClientContext(EnhancedBigtableStubSettings set
// We don't want client side metrics to crash the client, so catch any exception when getting
// the OTEL instance and log the exception instead.
openTelemetry =
getOpenTelemetry(settings.getProjectId(), settings.getMetricsProvider(), credentials);
getOpenTelemetry(
settings.getProjectId(),
settings.getMetricsProvider(),
credentials,
settings.getMetricsEndpoint());
} catch (Throwable t) {
logger.log(Level.WARNING, "Failed to get OTEL, will skip exporting client side metrics", t);
}
Expand Down Expand Up @@ -378,7 +383,10 @@ public static ApiTracerFactory createBigtableTracerFactory(

@Nullable
public static OpenTelemetry getOpenTelemetry(
String projectId, MetricsProvider metricsProvider, @Nullable Credentials defaultCredentials)
String projectId,
MetricsProvider metricsProvider,
@Nullable Credentials defaultCredentials,
@Nullable String metricsEndpoint)
throws IOException {
if (metricsProvider instanceof CustomOpenTelemetryMetricsProvider) {
CustomOpenTelemetryMetricsProvider customMetricsProvider =
Expand All @@ -390,7 +398,7 @@ public static OpenTelemetry getOpenTelemetry(
? BigtableDataSettings.getMetricsCredentials()
: defaultCredentials;
DefaultMetricsProvider defaultMetricsProvider = (DefaultMetricsProvider) metricsProvider;
return defaultMetricsProvider.getOpenTelemetry(projectId, credentials);
return defaultMetricsProvider.getOpenTelemetry(projectId, metricsEndpoint, credentials);
} else if (metricsProvider instanceof NoopMetricsProvider) {
return null;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@
import java.util.Set;
import java.util.logging.Logger;
import javax.annotation.Nonnull;
import javax.annotation.Nullable;
import org.threeten.bp.Duration;

/**
Expand Down Expand Up @@ -250,6 +251,7 @@ public class EnhancedBigtableStubSettings extends StubSettings<EnhancedBigtableS
private final FeatureFlags featureFlags;

private final MetricsProvider metricsProvider;
@Nullable private final String metricsEndpoint;

private EnhancedBigtableStubSettings(Builder builder) {
super(builder);
Expand Down Expand Up @@ -278,6 +280,7 @@ private EnhancedBigtableStubSettings(Builder builder) {
enableRoutingCookie = builder.enableRoutingCookie;
enableRetryInfo = builder.enableRetryInfo;
metricsProvider = builder.metricsProvider;
metricsEndpoint = builder.metricsEndpoint;

// Per method settings.
readRowsSettings = builder.readRowsSettings.build();
Expand Down Expand Up @@ -362,6 +365,15 @@ public boolean getEnableRetryInfo() {
return enableRetryInfo;
}

/**
* Gets the Google Cloud Monitoring endpoint for publishing client side metrics. If it's null,
* client will publish metrics to the default monitoring endpoint.
*/
@Nullable
public String getMetricsEndpoint() {
return metricsEndpoint;
}

/** Returns a builder for the default ChannelProvider for this service. */
public static InstantiatingGrpcChannelProvider.Builder defaultGrpcTransportProviderBuilder() {
Boolean isDirectpathEnabled = Boolean.parseBoolean(System.getenv(CBT_ENABLE_DIRECTPATH));
Expand Down Expand Up @@ -684,6 +696,7 @@ public static class Builder extends StubSettings.Builder<EnhancedBigtableStubSet
private FeatureFlags.Builder featureFlags;

private MetricsProvider metricsProvider;
@Nullable private String metricsEndpoint;

/**
* Initializes a new Builder with sane defaults for all settings.
Expand Down Expand Up @@ -831,6 +844,7 @@ private Builder(EnhancedBigtableStubSettings settings) {
enableRoutingCookie = settings.enableRoutingCookie;
enableRetryInfo = settings.enableRetryInfo;
metricsProvider = settings.metricsProvider;
metricsEndpoint = settings.getMetricsEndpoint();

// Per method settings.
readRowsSettings = settings.readRowsSettings.toBuilder();
Expand Down Expand Up @@ -999,6 +1013,24 @@ public MetricsProvider getMetricsProvider() {
return this.metricsProvider;
}

/**
* Built-in client side metrics are published through Google Cloud Monitoring endpoint. This
* setting overrides the default endpoint for publishing the metrics.
*/
public Builder setMetricsEndpoint(String endpoint) {
this.metricsEndpoint = endpoint;
return this;
}

/**
* Get the Google Cloud Monitoring endpoint for publishing client side metrics. If it's null,
* client will publish metrics to the default monitoring endpoint.
*/
@Nullable
public String getMetricsEndpoint() {
return metricsEndpoint;
}

@InternalApi("Used for internal testing")
public Map<String, String> getJwtAudienceMapping() {
return jwtAudienceMapping;
Expand Down Expand Up @@ -1184,6 +1216,7 @@ public String toString() {
.add("pingAndWarmSettings", pingAndWarmSettings)
.add("executeQuerySettings", executeQuerySettings)
.add("metricsProvider", metricsProvider)
.add("metricsEndpoint", metricsEndpoint)
.add("parent", super.toString())
.toString();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@
import com.google.cloud.monitoring.v3.MetricServiceClient;
import com.google.cloud.monitoring.v3.MetricServiceSettings;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.MoreObjects;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
Expand Down Expand Up @@ -79,11 +78,12 @@ public final class BigtableCloudMonitoringExporter implements MetricExporter {
Logger.getLogger(BigtableCloudMonitoringExporter.class.getName());

// This system property can be used to override the monitoring endpoint
// to a different environment. It's meant for internal testing only.
private static final String MONITORING_ENDPOINT =
MoreObjects.firstNonNull(
System.getProperty("bigtable.test-monitoring-endpoint"),
MetricServiceSettings.getDefaultEndpoint());
// to a different environment. It's meant for internal testing only and
// will be removed in future versions. Use settings in EnhancedBigtableStubSettings
// to override the endpoint.
@Deprecated @Nullable
private static final String MONITORING_ENDPOINT_OVERRIDE_SYS_PROP =
System.getProperty("bigtable.test-monitoring-endpoint");

private static final String APPLICATION_RESOURCE_PROJECT_ID = "project_id";

Expand Down Expand Up @@ -126,14 +126,22 @@ public final class BigtableCloudMonitoringExporter implements MetricExporter {
.collect(ImmutableList.toImmutableList());

public static BigtableCloudMonitoringExporter create(
String projectId, @Nullable Credentials credentials) throws IOException {
String projectId, @Nullable Credentials credentials, @Nullable String endpoint)
throws IOException {
MetricServiceSettings.Builder settingsBuilder = MetricServiceSettings.newBuilder();
CredentialsProvider credentialsProvider =
Optional.ofNullable(credentials)
.<CredentialsProvider>map(FixedCredentialsProvider::create)
.orElse(NoCredentialsProvider.create());
settingsBuilder.setCredentialsProvider(credentialsProvider);
settingsBuilder.setEndpoint(MONITORING_ENDPOINT);
if (MONITORING_ENDPOINT_OVERRIDE_SYS_PROP != null) {
logger.warning(
"Setting the monitoring endpoint through system variable will be removed in future versions");
settingsBuilder.setEndpoint(MONITORING_ENDPOINT_OVERRIDE_SYS_PROP);
}
if (endpoint != null) {
settingsBuilder.setEndpoint(endpoint);
}

org.threeten.bp.Duration timeout = Duration.ofMinutes(1);
// TODO: createServiceTimeSeries needs special handling if the request failed. Leaving
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,19 +37,36 @@ private BuiltinMetricsView() {}

/**
* Register built-in metrics on the {@link SdkMeterProviderBuilder} with application default
* credentials.
* credentials and default endpoint.
*/
public static void registerBuiltinMetrics(String projectId, SdkMeterProviderBuilder builder)
throws IOException {
BuiltinMetricsView.registerBuiltinMetrics(
projectId, GoogleCredentials.getApplicationDefault(), builder);
}

/** Register built-in metrics on the {@link SdkMeterProviderBuilder} with credentials. */
/**
* Register built-in metrics on the {@link SdkMeterProviderBuilder} with custom credentials and
* default endpoint.
*/
public static void registerBuiltinMetrics(
String projectId, @Nullable Credentials credentials, SdkMeterProviderBuilder builder)
throws IOException {
MetricExporter metricExporter = BigtableCloudMonitoringExporter.create(projectId, credentials);
BuiltinMetricsView.registerBuiltinMetrics(projectId, credentials, builder, null);
}

/**
* Register built-in metrics on the {@link SdkMeterProviderBuilder} with custom credentials and
* endpoint.
*/
public static void registerBuiltinMetrics(
String projectId,
@Nullable Credentials credentials,
SdkMeterProviderBuilder builder,
@Nullable String endpoint)
throws IOException {
MetricExporter metricExporter =
BigtableCloudMonitoringExporter.create(projectId, credentials, endpoint);
for (Map.Entry<InstrumentSelector, View> entry :
BuiltinMetricsConstants.getAllViews().entrySet()) {
builder.registerView(entry.getKey(), entry.getValue());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,12 +42,14 @@ public final class DefaultMetricsProvider implements MetricsProvider {
private DefaultMetricsProvider() {}

@InternalApi
public OpenTelemetry getOpenTelemetry(String projectId, @Nullable Credentials credentials)
public OpenTelemetry getOpenTelemetry(
String projectId, String metricsEndpoint, @Nullable Credentials credentials)
throws IOException {
this.projectId = projectId;
if (openTelemetry == null) {
SdkMeterProviderBuilder meterProvider = SdkMeterProvider.builder();
BuiltinMetricsView.registerBuiltinMetrics(projectId, credentials, meterProvider);
BuiltinMetricsView.registerBuiltinMetrics(
projectId, credentials, meterProvider, metricsEndpoint);
openTelemetry = OpenTelemetrySdk.builder().setMeterProvider(meterProvider.build()).build();
}
return openTelemetry;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ public void settingsAreNotLostTest() {
Duration watchdogInterval = Duration.ofSeconds(12);
boolean enableRoutingCookie = false;
boolean enableRetryInfo = false;
String metricsEndpoint = "test-endpoint:443";

EnhancedBigtableStubSettings.Builder builder =
EnhancedBigtableStubSettings.newBuilder()
Expand All @@ -93,7 +94,8 @@ public void settingsAreNotLostTest() {
.setStreamWatchdogProvider(watchdogProvider)
.setStreamWatchdogCheckInterval(watchdogInterval)
.setEnableRoutingCookie(enableRoutingCookie)
.setEnableRetryInfo(enableRetryInfo);
.setEnableRetryInfo(enableRetryInfo)
.setMetricsEndpoint(metricsEndpoint);

verifyBuilder(
builder,
Expand All @@ -106,7 +108,8 @@ public void settingsAreNotLostTest() {
watchdogProvider,
watchdogInterval,
enableRoutingCookie,
enableRetryInfo);
enableRetryInfo,
metricsEndpoint);
verifySettings(
builder.build(),
projectId,
Expand All @@ -118,7 +121,8 @@ public void settingsAreNotLostTest() {
watchdogProvider,
watchdogInterval,
enableRoutingCookie,
enableRetryInfo);
enableRetryInfo,
metricsEndpoint);
verifyBuilder(
builder.build().toBuilder(),
projectId,
Expand All @@ -130,7 +134,8 @@ public void settingsAreNotLostTest() {
watchdogProvider,
watchdogInterval,
enableRoutingCookie,
enableRetryInfo);
enableRetryInfo,
metricsEndpoint);
}

private void verifyBuilder(
Expand All @@ -144,7 +149,8 @@ private void verifyBuilder(
WatchdogProvider watchdogProvider,
Duration watchdogInterval,
boolean enableRoutingCookie,
boolean enableRetryInfo) {
boolean enableRetryInfo,
String metricsEndpoint) {
assertThat(builder.getProjectId()).isEqualTo(projectId);
assertThat(builder.getInstanceId()).isEqualTo(instanceId);
assertThat(builder.getAppProfileId()).isEqualTo(appProfileId);
Expand All @@ -155,6 +161,7 @@ private void verifyBuilder(
assertThat(builder.getStreamWatchdogCheckInterval()).isEqualTo(watchdogInterval);
assertThat(builder.getEnableRoutingCookie()).isEqualTo(enableRoutingCookie);
assertThat(builder.getEnableRetryInfo()).isEqualTo(enableRetryInfo);
assertThat(builder.getMetricsEndpoint()).isEqualTo(metricsEndpoint);
}

private void verifySettings(
Expand All @@ -168,7 +175,8 @@ private void verifySettings(
WatchdogProvider watchdogProvider,
Duration watchdogInterval,
boolean enableRoutingCookie,
boolean enableRetryInfo) {
boolean enableRetryInfo,
String metricsEndpoint) {
assertThat(settings.getProjectId()).isEqualTo(projectId);
assertThat(settings.getInstanceId()).isEqualTo(instanceId);
assertThat(settings.getAppProfileId()).isEqualTo(appProfileId);
Expand All @@ -179,6 +187,7 @@ private void verifySettings(
assertThat(settings.getStreamWatchdogCheckInterval()).isEqualTo(watchdogInterval);
assertThat(settings.getEnableRoutingCookie()).isEqualTo(enableRoutingCookie);
assertThat(settings.getEnableRetryInfo()).isEqualTo(enableRetryInfo);
assertThat(settings.getMetricsEndpoint()).isEqualTo(metricsEndpoint);
}

@Test
Expand Down Expand Up @@ -965,6 +974,7 @@ public void enableRetryInfoFalseValueTest() throws IOException {
"pingAndWarmSettings",
"executeQuerySettings",
"metricsProvider",
"metricsEndpoint",
};

@Test
Expand Down

0 comments on commit a341eb8

Please sign in to comment.