Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: support override monitoring endpoint #2364

Merged
merged 7 commits into from
Oct 1, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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,
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) {
igorbernstein2 marked this conversation as resolved.
Show resolved Hide resolved
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() {
igorbernstein2 marked this conversation as resolved.
Show resolved Hide resolved
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,11 @@ 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.
// 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.
private static final String MONITORING_ENDPOINT =
mutianf marked this conversation as resolved.
Show resolved Hide resolved
MoreObjects.firstNonNull(
System.getProperty("bigtable.test-monitoring-endpoint"),
MetricServiceSettings.getDefaultEndpoint());
System.getProperty("bigtable.test-monitoring-endpoint");

private static final String APPLICATION_RESOURCE_PROJECT_ID = "project_id";

Expand Down Expand Up @@ -126,14 +125,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 != null) {
logger.warning(
"Setting the monitoring endpoint through system variable will be removed in future versions");
settingsBuilder.setEndpoint(MONITORING_ENDPOINT);
}
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 @@ -17,6 +17,7 @@

import com.google.auth.Credentials;
import com.google.auth.oauth2.GoogleCredentials;
import com.google.cloud.monitoring.v3.MetricServiceSettings;
import io.opentelemetry.sdk.metrics.InstrumentSelector;
import io.opentelemetry.sdk.metrics.SdkMeterProviderBuilder;
import io.opentelemetry.sdk.metrics.View;
Expand All @@ -37,19 +38,37 @@ 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, MetricServiceSettings.getDefaultEndpoint());
mutianf marked this conversation as resolved.
Show resolved Hide resolved
}

/**
* 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
Loading