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.

Sign up for GitHub

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

chore: client metrics #3125

Merged
merged 9 commits into from
Sep 19, 2024
Merged

chore: client metrics #3125

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
3b64f56
feat: client metrics
surbhigarg92 Jun 19, 2024
cedbb61
Review comments
surbhigarg92 Jul 23, 2024
a7aeb57
Few issues and code optimisations
surbhigarg92 Aug 14, 2024
a6532a5
review comments
surbhigarg92 Aug 16, 2024
eeed7d3
directpath_used attribute
surbhigarg92 Aug 21, 2024
e8a587f
removed directpath enabled attribute
surbhigarg92 Sep 4, 2024
b2f9361
removed directpath enabled attribute
surbhigarg92 Sep 4, 2024
cf8e92d
lint
surbhigarg92 Sep 19, 2024
8259d03
bucket boundaries
surbhigarg92 Sep 19, 2024
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
Prev Previous commit
Next Next commit
Few issues and code optimisations
  • Loading branch information
@surbhigarg92
surbhigarg92 committed Sep 17, 2024
commit a7aeb5766a64f875e82b2b093e56954ad8500e6e
19 changes: 11 additions & 8 deletions ...d-spanner/src/main/java/com/google/cloud/spanner/BuiltInOpenTelemetryMetricsProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,16 @@

package com.google.cloud.spanner;

import static com.google.cloud.opentelemetry.detection.GCPPlatformDetector.SupportedPlatform.GOOGLE_KUBERNETES_ENGINE;
import static com.google.cloud.spanner.BuiltInMetricsConstant.CLIENT_NAME_KEY;
import static com.google.cloud.spanner.BuiltInMetricsConstant.CLIENT_UID_KEY;
import static com.google.cloud.spanner.BuiltInMetricsConstant.DIRECT_PATH_ENABLED_KEY;
import static com.google.cloud.spanner.BuiltInMetricsConstant.INSTANCE_CONFIG_ID_KEY;
import static com.google.cloud.spanner.BuiltInMetricsConstant.LOCATION_ID_KEY;
import static com.google.cloud.spanner.BuiltInMetricsConstant.PROJECT_ID_KEY;

import com.google.api.gax.core.GaxProperties;
import com.google.auth.Credentials;
import com.google.cloud.opentelemetry.detection.AttributeKeys;
import com.google.cloud.opentelemetry.detection.DetectedPlatform;
import com.google.cloud.opentelemetry.detection.GCPPlatformDetector;
import io.opentelemetry.api.OpenTelemetry;
Expand Down Expand Up @@ -76,26 +77,28 @@ OpenTelemetry getOrCreateOpenTelemetry(String projectId, @Nullable Credentials c
}
}

Map<String, String> getClientAttributes(String projectId, boolean isDirectPathChannelCreated) {
Map<String, String> getClientAttributes(
String projectId, boolean isDirectPathChannelCreated, String client_name) {
Map<String, String> clientAttributes = new HashMap<>();
clientAttributes.put(LOCATION_ID_KEY.getKey(), detectClientLocation());
clientAttributes.put(PROJECT_ID_KEY.getKey(), projectId);
// TODO: Replace this with real value.
clientAttributes.put(INSTANCE_CONFIG_ID_KEY.getKey(), "unknown");
clientAttributes.put(
DIRECT_PATH_ENABLED_KEY.getKey(), String.valueOf(isDirectPathChannelCreated));
clientAttributes.put(
CLIENT_NAME_KEY.getKey(),
"spanner-java/"
+ GaxProperties.getLibraryVersion(SpannerCloudMonitoringExporterUtils.class));
clientAttributes.put(CLIENT_NAME_KEY.getKey(), client_name);
clientAttributes.put(CLIENT_UID_KEY.getKey(), getDefaultTaskValue());
return clientAttributes;
}

private String detectClientLocation() {
static String detectClientLocation() {
GCPPlatformDetector detector = GCPPlatformDetector.DEFAULT_INSTANCE;
DetectedPlatform detectedPlatform = detector.detectPlatform();
String region = detectedPlatform.getAttributes().get("cloud.region");
// All platform except GKE uses "cloud_region" for region attribute.
String region = detectedPlatform.getAttributes().get("cloud_region");
if (detectedPlatform.getSupportedPlatform() == GOOGLE_KUBERNETES_ENGINE) {
region = detectedPlatform.getAttributes().get(AttributeKeys.GKE_LOCATION_TYPE_REGION);
}
return region == null ? "global" : region;
}

Expand Down
17 changes: 10 additions & 7 deletions google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1653,23 +1653,24 @@ public OpenTelemetry getOpenTelemetry() {

@Override
public ApiTracerFactory getApiTracerFactory() {
return createApiTracerFactory(false, false);
return createApiTracerFactory(false, false, false);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A get-method should preferably not call something that creates a new instance every time it is called. The original implementation of this did create a new list every time it was called, but it did not create any of the elements in the list. Can we make sure that this method still does not create a new instance of ApiTracerFactory each time it is called?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, even the metricstracerfactory getting added is only created once. We are using the static instance of BuiltInOpenTelemetryMetricsProvider to call getOrCreateOpenTelemetry . We have a check in this if openTelemetry is already created, then we do not create it again.

It will create a new MetricsTracerFactory object with the same openTelemetry object similar to OpenTelemetryApiTracerFactory

}

public ApiTracerFactory getApiTracerFactory(
boolean isDirectPathChannelCreated, boolean isAdminClient) {
return createApiTracerFactory(isDirectPathChannelCreated, isAdminClient);
boolean isDirectPathChannelCreated, boolean isAdminClient, boolean isEmulatorEnabled) {
return createApiTracerFactory(isDirectPathChannelCreated, isAdminClient, isEmulatorEnabled);
}

private ApiTracerFactory createApiTracerFactory(
boolean isDirectPathChannelCreated, boolean isAdminClient) {
boolean isDirectPathChannelCreated, boolean isAdminClient, boolean isEmulatorEnabled) {
List<ApiTracerFactory> apiTracerFactories = new ArrayList<>();
// Prefer any direct ApiTracerFactory that might have been set on the builder.
apiTracerFactories.add(
MoreObjects.firstNonNull(super.getApiTracerFactory(), getDefaultApiTracerFactory()));

// Add Metrics Tracer factory if enabled and if data client
if (isEnableBuiltInMetrics() && !isAdminClient) {
// Add Metrics Tracer factory if built in metrics are enabled and if the client is data client
// and if emulator is not enabled.
if (isEnableBuiltInMetrics() && !isAdminClient && !isEmulatorEnabled) {
ApiTracerFactory metricsTracerFactory =
getMetricsApiTracerFactory(isDirectPathChannelCreated);
if (metricsTracerFactory != null) {
Expand Down Expand Up @@ -1705,7 +1706,9 @@ private ApiTracerFactory getMetricsApiTracerFactory(boolean isDirectPathChannelC
? new MetricsTracerFactory(
new OpenTelemetryMetricsRecorder(openTelemetry, BuiltInMetricsConstant.METER_NAME),
builtInOpenTelemetryMetricsProvider.getClientAttributes(
getDefaultProjectId(), isDirectPathChannelCreated))
getDefaultProjectId(),
isDirectPathChannelCreated,
"spanner-java/" + GaxProperties.getLibraryVersion(getClass())))
: null;
}

Expand Down
40 changes: 28 additions & 12 deletions google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpc.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -395,6 +395,8 @@ public GapicSpannerRpc(final SpannerOptions options) {
.withCheckInterval(checkInterval)
.withClock(NanoClock.getDefaultClock());

final String emulatorHost = System.getenv("SPANNER_EMULATOR_HOST");

try {
this.spannerStub =
GrpcSpannerStub.create(
Expand All @@ -405,7 +407,10 @@ public GapicSpannerRpc(final SpannerOptions options) {
.setCredentialsProvider(credentialsProvider)
.setStreamWatchdogProvider(watchdogProvider)
.setTracerFactory(
options.getApiTracerFactory(isDirectPathChannelCreated, false))
options.getApiTracerFactory(
isDirectPathChannelCreated,
false,
isEmulatorEnabled(options, emulatorHost)))
.build());
this.readRetrySettings =
options.getSpannerStubSettings().streamingReadSettings().getRetrySettings();
Expand Down Expand Up @@ -433,7 +438,9 @@ public GapicSpannerRpc(final SpannerOptions options) {
.setTransportChannelProvider(channelProvider)
.setCredentialsProvider(credentialsProvider)
.setStreamWatchdogProvider(watchdogProvider)
.setTracerFactory(options.getApiTracerFactory(isDirectPathChannelCreated, false))
.setTracerFactory(
options.getApiTracerFactory(
isDirectPathChannelCreated, false, isEmulatorEnabled(options, emulatorHost)))
.executeSqlSettings()
.setRetrySettings(partitionedDmlRetrySettings);
pdmlSettings.executeStreamingSqlSettings().setRetrySettings(partitionedDmlRetrySettings);
Expand All @@ -460,7 +467,9 @@ public GapicSpannerRpc(final SpannerOptions options) {
.setTransportChannelProvider(channelProvider)
.setCredentialsProvider(credentialsProvider)
.setStreamWatchdogProvider(watchdogProvider)
.setTracerFactory(options.getApiTracerFactory(isDirectPathChannelCreated, true))
.setTracerFactory(
options.getApiTracerFactory(
isDirectPathChannelCreated, true, isEmulatorEnabled(options, emulatorHost)))
.build();
this.instanceAdminStub = GrpcInstanceAdminStub.create(instanceAdminStubSettings);

Expand All @@ -471,7 +480,9 @@ public GapicSpannerRpc(final SpannerOptions options) {
.setTransportChannelProvider(channelProvider)
.setCredentialsProvider(credentialsProvider)
.setStreamWatchdogProvider(watchdogProvider)
.setTracerFactory(options.getApiTracerFactory(isDirectPathChannelCreated, true))
.setTracerFactory(
options.getApiTracerFactory(
isDirectPathChannelCreated, true, isEmulatorEnabled(options, emulatorHost)))
.build();

// Automatically retry RESOURCE_EXHAUSTED for GetOperation if auto-throttling of
Expand Down Expand Up @@ -515,7 +526,7 @@ public <RequestT, ResponseT> UnaryCallable<RequestT, ResponseT> createUnaryCalla

// Check whether the SPANNER_EMULATOR_HOST env var has been set, and if so, if the emulator
// is actually running.
checkEmulatorConnection(options, channelProvider, credentialsProvider);
checkEmulatorConnection(options, channelProvider, credentialsProvider, emulatorHost);
} catch (Exception e) {
throw newSpannerException(e);
}
Expand Down Expand Up @@ -614,15 +625,11 @@ private static HeaderProvider headerProviderWithUserAgentFrom(HeaderProvider hea
private static void checkEmulatorConnection(
SpannerOptions options,
TransportChannelProvider channelProvider,
CredentialsProvider credentialsProvider)
CredentialsProvider credentialsProvider,
String emulatorHost)
throws IOException {
final String emulatorHost = System.getenv("SPANNER_EMULATOR_HOST");
// Only do the check if the emulator environment variable has been set to localhost.
if (options.getChannelProvider() == null
&& emulatorHost != null
&& options.getHost() != null
&& options.getHost().startsWith("http://localhost")
&& options.getHost().endsWith(emulatorHost)) {
if (isEmulatorEnabled(options, emulatorHost)) {
// Do a quick check to see if the emulator is actually running.
try {
InstanceAdminStubSettings.Builder testEmulatorSettings =
Expand Down Expand Up @@ -655,6 +662,15 @@ private static void checkEmulatorConnection(
}
}

private static boolean isEmulatorEnabled(SpannerOptions options, String emulatorHost) {
// Only do the check if the emulator environment variable has been set to localhost.
return options.getChannelProvider() == null
&& emulatorHost != null
&& options.getHost() != null
&& options.getHost().startsWith("http://localhost")
&& options.getHost().endsWith(emulatorHost);
}

private static final RetrySettings ADMIN_REQUESTS_LIMIT_EXCEEDED_RETRY_SETTINGS =
RetrySettings.newBuilder()
.setInitialRetryDelay(Duration.ofSeconds(5L))
Expand Down
5 changes: 3 additions & 2 deletions google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/HeaderInterceptor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ class HeaderInterceptor implements ClientInterceptor {
@Override
public <ReqT, RespT> ClientCall<ReqT, RespT> interceptCall(
MethodDescriptor<ReqT, RespT> method, CallOptions callOptions, Channel next) {
Object tracer = callOptions.getOption(TRACER_KEY);
return new SimpleForwardingClientCall<ReqT, RespT>(next.newCall(method, callOptions)) {
@Override
public void start(Listener<RespT> responseListener, Metadata headers) {
Expand All @@ -99,8 +100,8 @@ public void start(Listener<RespT> responseListener, Metadata headers) {
DatabaseName databaseName = extractDatabaseName(headers);
String key = databaseName + method.getFullMethodName();
TagContext tagContext = getTagContext(key, method.getFullMethodName(), databaseName);
if (callOptions.getOption(TRACER_KEY) instanceof CompositeTracer) {
CompositeTracer compositeTracer = (CompositeTracer) callOptions.getOption(TRACER_KEY);
if (tracer instanceof CompositeTracer) {
CompositeTracer compositeTracer = (CompositeTracer) tracer;
addBuiltInMetricAttributes(compositeTracer, databaseName);
}
Attributes attributes =
Expand Down
11 changes: 5 additions & 6 deletions ...spanner/src/test/java/com/google/cloud/spanner/OpenTelemetryBuiltInMetricsTracerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;

import com.google.api.gax.core.GaxProperties;
import com.google.api.gax.longrunning.OperationTimedPollAlgorithm;
import com.google.api.gax.retrying.RetrySettings;
import com.google.api.gax.tracing.ApiTracerFactory;
Expand Down Expand Up @@ -87,19 +86,19 @@ public static void setup() {

BuiltInMetricsConstant.getAllViews().forEach(meterProvider::registerView);

String client_name = "spanner-java/";
openTelemetry = OpenTelemetrySdk.builder().setMeterProvider(meterProvider.build()).build();
attributes = provider.getClientAttributes("test-project", true);
attributes = provider.getClientAttributes("test-project", true, client_name);

expectedBaseAttributes =
Attributes.builder()
.put(BuiltInMetricsConstant.PROJECT_ID_KEY, "test-project")
.put(BuiltInMetricsConstant.INSTANCE_CONFIG_ID_KEY, "unknown")
.put(BuiltInMetricsConstant.DIRECT_PATH_ENABLED_KEY, "true")
.put(BuiltInMetricsConstant.LOCATION_ID_KEY, "global")
.put(
BuiltInMetricsConstant.CLIENT_NAME_KEY,
"spanner-java/"
+ GaxProperties.getLibraryVersion(SpannerCloudMonitoringExporterUtils.class))
BuiltInMetricsConstant.LOCATION_ID_KEY,
BuiltInOpenTelemetryMetricsProvider.detectClientLocation())
.put(BuiltInMetricsConstant.CLIENT_NAME_KEY, client_name)
.put(BuiltInMetricsConstant.CLIENT_UID_KEY, attributes.get("client_uid"))
.build();
}
Expand Down