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

feat: add internal "deadline remaining" client side metric #2341 #2370

Merged
Merged
Show file tree
Hide file tree
Changes from 37 commits
Commits
Show all changes
42 commits
Select commit Hold shift + click to select a range
7fe9933
Rebase on HEAD
djyau Oct 16, 2024
bd36596
Add override to CompositeTracer
djyau Oct 4, 2024
965e4bd
Remove blank lines
djyau Oct 4, 2024
2068604
Fix missing imports
djyau Oct 4, 2024
affb852
Only record remaining deadline if it is not null
djyau Oct 7, 2024
67fe252
Add remaining deadline to list of metrics in BigtableCloudMonitoringE…
djyau Oct 7, 2024
64908fb
Export remainingDeadline histogram
djyau Oct 7, 2024
804d223
Fix unit test
djyau Oct 8, 2024
585c64c
Remove print statement
djyau Oct 8, 2024
ff7c311
Fix merge conflict
djyau Oct 16, 2024
a336fa4
Use deadline from callContext, not callOptions
djyau Oct 16, 2024
3f3d273
Fixing merge conflicts
djyau Oct 16, 2024
afb4d50
Fix more merge conflicts
djyau Oct 16, 2024
bf789ab
Fix merge conflicts...
djyau Oct 16, 2024
501e029
Fixing more merge conflicts
djyau Oct 16, 2024
42930a8
Fix more merge conflicts
djyau Oct 16, 2024
0191ab0
Use operation timer
djyau Oct 17, 2024
ec13bd8
Run auto formatter
djyau Oct 21, 2024
1322f44
Address PR comment
djyau Oct 21, 2024
2235f20
Remove print statement, fix fn coment
djyau Oct 21, 2024
b8046a6
Trying to plumb operation timeout thru
djyau Oct 25, 2024
68d2216
Set remaining deadline in tracer callables
djyau Oct 25, 2024
235e73a
Set remaining deadline in tracer unary callable as well
djyau Oct 25, 2024
b878e0b
Remove unused lines
djyau Oct 25, 2024
e9b4b12
Remove unused code
djyau Oct 25, 2024
74b9ae5
Set the operation timeout on the call context for each callable
djyau Oct 25, 2024
7e78857
Remove unnecessary long conversion, format code
djyau Oct 25, 2024
1a66d9e
Make the deadline key a public member of Bigtable Tracer
djyau Oct 25, 2024
439e081
Fix settings
djyau Oct 25, 2024
18ffd59
Missed one setting
djyau Oct 25, 2024
b04b8ca
Address PR comments
djyau Oct 28, 2024
d63d640
Rename variable to make consistent
djyau Oct 28, 2024
f6b6c70
Add nonzero check for remaining deadline
djyau Oct 28, 2024
0fef2b8
Set remaining operation timeout in attemptStart
djyau Oct 30, 2024
4a9ef83
Add clarifying comment
djyau Oct 30, 2024
f409ea7
Format code
djyau Oct 30, 2024
83f0df1
Use only java.time.Duration in BuiltinMetricsTracerTest
djyau Nov 4, 2024
cc88293
Loosen test assertions
djyau Nov 4, 2024
2c54095
Merge branch 'main' into feature-deadline-remaining-metric
mutianf Nov 6, 2024
549fb9a
Use callSettings in UnaryCallable
djyau Nov 6, 2024
4e66bc2
Format code
djyau Nov 6, 2024
bc1dde2
Address PR comments
djyau Nov 6, 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
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@
import com.google.cloud.bigtable.data.v2.stub.changestream.GenerateInitialChangeStreamPartitionsUserCallable;
import com.google.cloud.bigtable.data.v2.stub.changestream.ReadChangeStreamResumptionStrategy;
import com.google.cloud.bigtable.data.v2.stub.changestream.ReadChangeStreamUserCallable;
import com.google.cloud.bigtable.data.v2.stub.metrics.BigtableTracer;
import com.google.cloud.bigtable.data.v2.stub.metrics.BigtableTracerStreamingCallable;
import com.google.cloud.bigtable.data.v2.stub.metrics.BigtableTracerUnaryCallable;
import com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsTracerFactory;
Expand Down Expand Up @@ -539,7 +540,12 @@ public <RowT> ServerStreamingCallable<Query, RowT> createReadRowsCallable(
new TracedServerStreamingCallable<>(
readRowsUserCallable, clientContext.getTracerFactory(), span);

return traced.withDefaultCallContext(clientContext.getDefaultCallContext());
return traced.withDefaultCallContext(
djyau marked this conversation as resolved.
Show resolved Hide resolved
clientContext
.getDefaultCallContext()
.withOption(
BigtableTracer.OPERATION_TIMEOUT_KEY,
settings.readRowsSettings().getRetrySettings().getTotalTimeout()));
}

/**
Expand Down Expand Up @@ -575,7 +581,12 @@ public <RowT> UnaryCallable<Query, RowT> createReadRowCallable(RowAdapter<RowT>
new TracedUnaryCallable<>(
firstRow, clientContext.getTracerFactory(), getSpanName("ReadRow"));

return traced.withDefaultCallContext(clientContext.getDefaultCallContext());
return traced.withDefaultCallContext(
clientContext
.getDefaultCallContext()
.withOption(
BigtableTracer.OPERATION_TIMEOUT_KEY,
settings.readRowSettings().getRetrySettings().getTotalTimeout()));
}

/**
Expand Down Expand Up @@ -694,7 +705,12 @@ private <RowT> UnaryCallable<Query, List<RowT>> createBulkReadRowsCallable(
UnaryCallable<Query, List<RowT>> traced =
new TracedUnaryCallable<>(tracedBatcher, clientContext.getTracerFactory(), span);

return traced.withDefaultCallContext(clientContext.getDefaultCallContext());
return traced.withDefaultCallContext(
clientContext
.getDefaultCallContext()
.withOption(
BigtableTracer.OPERATION_TIMEOUT_KEY,
settings.bulkReadRowsSettings().getRetrySettings().getTotalTimeout()));
}

/**
Expand Down Expand Up @@ -766,7 +782,14 @@ private UnaryCallable<String, List<KeyOffset>> createSampleRowKeysCallable() {
UnaryCallable<com.google.bigtable.v2.SampleRowKeysRequest, List<SampleRowKeysResponse>>
baseCallable = createSampleRowKeysBaseCallable();
return createUserFacingUnaryCallable(
methodName, new SampleRowKeysCallable(baseCallable, requestContext));
methodName,
new SampleRowKeysCallable(baseCallable, requestContext)
.withDefaultCallContext(
clientContext
.getDefaultCallContext()
.withOption(
BigtableTracer.OPERATION_TIMEOUT_KEY,
settings.sampleRowKeysSettings().getRetrySettings().getTotalTimeout())));
}

/**
Expand All @@ -789,7 +812,14 @@ private UnaryCallable<String, List<KeyOffset>> createSampleRowKeysCallable() {
UnaryCallable<com.google.bigtable.v2.SampleRowKeysRequest, List<SampleRowKeysResponse>>
baseCallable = createSampleRowKeysBaseCallable();
return createUserFacingUnaryCallable(
methodName, new SampleRowKeysCallableWithRequest(baseCallable, requestContext));
methodName,
new SampleRowKeysCallableWithRequest(baseCallable, requestContext)
.withDefaultCallContext(
clientContext
.getDefaultCallContext()
.withOption(
BigtableTracer.OPERATION_TIMEOUT_KEY,
settings.sampleRowKeysSettings().getRetrySettings().getTotalTimeout())));
}

/**
Expand Down Expand Up @@ -836,7 +866,14 @@ public Map<String, String> extract(MutateRowRequest mutateRowRequest) {
withRetries(withBigtableTracer, settings.mutateRowSettings());

return createUserFacingUnaryCallable(
methodName, new MutateRowCallable(retrying, requestContext));
methodName,
new MutateRowCallable(retrying, requestContext)
.withDefaultCallContext(
clientContext
.getDefaultCallContext()
.withOption(
BigtableTracer.OPERATION_TIMEOUT_KEY,
settings.mutateRowSettings().getRetrySettings().getTotalTimeout())));
}

/**
Expand Down Expand Up @@ -953,7 +990,12 @@ public Map<String, String> extract(MutateRowsRequest mutateRowsRequest) {
new TracedUnaryCallable<>(
tracedBatcherUnaryCallable, clientContext.getTracerFactory(), spanName);

return traced.withDefaultCallContext(clientContext.getDefaultCallContext());
return traced.withDefaultCallContext(
clientContext
.getDefaultCallContext()
.withOption(
BigtableTracer.OPERATION_TIMEOUT_KEY,
settings.bulkMutateRowsSettings().getRetrySettings().getTotalTimeout()));
}

/**
Expand Down Expand Up @@ -1093,7 +1135,17 @@ public Map<String, String> extract(
withRetries(withBigtableTracer, settings.checkAndMutateRowSettings());

return createUserFacingUnaryCallable(
methodName, new CheckAndMutateRowCallable(retrying, requestContext));
methodName,
new CheckAndMutateRowCallable(retrying, requestContext)
.withDefaultCallContext(
clientContext
.getDefaultCallContext()
.withOption(
BigtableTracer.OPERATION_TIMEOUT_KEY,
settings
.checkAndMutateRowSettings()
.getRetrySettings()
.getTotalTimeout())));
}

/**
Expand Down Expand Up @@ -1139,7 +1191,17 @@ public Map<String, String> extract(ReadModifyWriteRowRequest request) {
withRetries(withBigtableTracer, settings.readModifyWriteRowSettings());

return createUserFacingUnaryCallable(
methodName, new ReadModifyWriteRowCallable(retrying, requestContext));
methodName,
new ReadModifyWriteRowCallable(retrying, requestContext)
.withDefaultCallContext(
clientContext
.getDefaultCallContext()
.withOption(
BigtableTracer.OPERATION_TIMEOUT_KEY,
settings
.readModifyWriteRowSettings()
.getRetrySettings()
.getTotalTimeout())));
}

/**
Expand Down Expand Up @@ -1222,7 +1284,15 @@ public Map<String, String> extract(
ServerStreamingCallable<String, ByteStringRange> traced =
new TracedServerStreamingCallable<>(retrying, clientContext.getTracerFactory(), span);

return traced.withDefaultCallContext(clientContext.getDefaultCallContext());
return traced.withDefaultCallContext(
clientContext
.getDefaultCallContext()
.withOption(
BigtableTracer.OPERATION_TIMEOUT_KEY,
settings
.generateInitialChangeStreamPartitionsSettings()
.getRetrySettings()
.getTotalTimeout()));
}

/**
Expand Down Expand Up @@ -1302,7 +1372,12 @@ public Map<String, String> extract(
new TracedServerStreamingCallable<>(
readChangeStreamUserCallable, clientContext.getTracerFactory(), span);

return traced.withDefaultCallContext(clientContext.getDefaultCallContext());
return traced.withDefaultCallContext(
clientContext
.getDefaultCallContext()
.withOption(
BigtableTracer.OPERATION_TIMEOUT_KEY,
settings.readChangeStreamSettings().getRetrySettings().getTotalTimeout()));
}

/**
Expand Down Expand Up @@ -1388,7 +1463,13 @@ public Map<String, String> extract(ExecuteQueryRequest executeQueryRequest) {
new TracedServerStreamingCallable<>(retries, clientContext.getTracerFactory(), span);

return new ExecuteQueryCallable(
traced.withDefaultCallContext(clientContext.getDefaultCallContext()), requestContext);
traced.withDefaultCallContext(
clientContext
.getDefaultCallContext()
.withOption(
BigtableTracer.OPERATION_TIMEOUT_KEY,
settings.executeQuerySettings().getRetrySettings().getTotalTimeout())),
requestContext);
}

/**
Expand Down Expand Up @@ -1420,7 +1501,12 @@ public Map<String, String> extract(PingAndWarmRequest request) {
})
.build(),
Collections.emptySet());
return pingAndWarm.withDefaultCallContext(clientContext.getDefaultCallContext());
return pingAndWarm.withDefaultCallContext(
clientContext
.getDefaultCallContext()
.withOption(
BigtableTracer.OPERATION_TIMEOUT_KEY,
settings.pingAndWarmSettings().getRetrySettings().getTotalTimeout()));
}

private <RequestT, ResponseT> UnaryCallable<RequestT, ResponseT> withRetries(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import static com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsConstants.METER_NAME;
import static com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsConstants.OPERATION_LATENCIES_NAME;
import static com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsConstants.PER_CONNECTION_ERROR_COUNT_NAME;
import static com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsConstants.REMAINING_DEADLINE_NAME;
import static com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsConstants.RETRY_COUNT_NAME;
import static com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsConstants.SERVER_LATENCIES_NAME;

Expand Down Expand Up @@ -115,7 +116,8 @@ public final class BigtableCloudMonitoringExporter implements MetricExporter {
CLIENT_BLOCKING_LATENCIES_NAME,
APPLICATION_BLOCKING_LATENCIES_NAME,
RETRY_COUNT_NAME,
CONNECTIVITY_ERROR_COUNT_NAME)
CONNECTIVITY_ERROR_COUNT_NAME,
REMAINING_DEADLINE_NAME)
.stream()
.map(m -> METER_NAME + m)
.collect(ImmutableList.toImmutableList());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,12 @@
package com.google.cloud.bigtable.data.v2.stub.metrics;

import com.google.api.core.BetaApi;
import com.google.api.core.InternalApi;
import com.google.api.gax.rpc.ApiCallContext;
import com.google.api.gax.tracing.ApiTracer;
import com.google.api.gax.tracing.BaseApiTracer;
import javax.annotation.Nullable;
import org.threeten.bp.Duration;

/**
* A Bigtable specific {@link ApiTracer} that includes additional contexts. This class is a base
Expand All @@ -30,6 +32,10 @@ public class BigtableTracer extends BaseApiTracer {

private volatile int attempt = 0;

@InternalApi("for internal use only")
public static final ApiCallContext.Key<Duration> OPERATION_TIMEOUT_KEY =
ApiCallContext.Key.create("OPERATION_TIMEOUT");

@Override
public void attemptStarted(int attemptNumber) {
this.attempt = attemptNumber;
Expand Down Expand Up @@ -93,4 +99,12 @@ public void grpcChannelQueuedLatencies(long queuedTimeMs) {
public void grpcMessageSent() {
// noop
}

/**
* Record the operation timeout from user settings for calculating remaining deadline. This will
* be called in BuiltinMetricsTracer.
*/
public void setOperationTimeout(Duration operationTimeout) {
// noop
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
package com.google.cloud.bigtable.data.v2.stub.metrics;

import com.google.api.core.InternalApi;
import com.google.api.gax.grpc.GrpcCallContext;
import com.google.api.gax.grpc.GrpcResponseMetadata;
import com.google.api.gax.rpc.ApiCallContext;
import com.google.api.gax.rpc.ResponseObserver;
Expand All @@ -26,6 +27,7 @@
import com.google.common.base.Stopwatch;
import java.util.concurrent.TimeUnit;
import javax.annotation.Nonnull;
import org.threeten.bp.Duration;

/**
* This callable will
Expand Down Expand Up @@ -62,6 +64,9 @@ public void call(
BigtableTracerResponseObserver<ResponseT> innerObserver =
new BigtableTracerResponseObserver<>(
responseObserver, (BigtableTracer) context.getTracer(), responseMetadata);
GrpcCallContext callContext = (GrpcCallContext) context;
Duration deadline = callContext.getOption(BigtableTracer.OPERATION_TIMEOUT_KEY);
djyau marked this conversation as resolved.
Show resolved Hide resolved
((BigtableTracer) context.getTracer()).setOperationTimeout(deadline);
innerCallable.call(
request,
innerObserver,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,14 @@
import com.google.api.core.ApiFutureCallback;
import com.google.api.core.ApiFutures;
import com.google.api.core.InternalApi;
import com.google.api.gax.grpc.GrpcCallContext;
import com.google.api.gax.grpc.GrpcResponseMetadata;
import com.google.api.gax.rpc.ApiCallContext;
import com.google.api.gax.rpc.UnaryCallable;
import com.google.common.base.Preconditions;
import com.google.common.util.concurrent.MoreExecutors;
import javax.annotation.Nonnull;
import org.threeten.bp.Duration;

/**
* This callable will:
Expand Down Expand Up @@ -58,6 +60,9 @@ public ApiFuture<ResponseT> futureCall(RequestT request, ApiCallContext context)
BigtableTracerUnaryCallback<ResponseT> callback =
new BigtableTracerUnaryCallback<ResponseT>(
(BigtableTracer) context.getTracer(), responseMetadata);
GrpcCallContext callContext = (GrpcCallContext) context;
Duration deadline = callContext.getOption(BigtableTracer.OPERATION_TIMEOUT_KEY);
djyau marked this conversation as resolved.
Show resolved Hide resolved
((BigtableTracer) context.getTracer()).setOperationTimeout(deadline);
ApiFuture<ResponseT> future =
innerCallable.futureCall(
request,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ public class BuiltinMetricsConstants {
static final String SERVER_LATENCIES_NAME = "server_latencies";
static final String FIRST_RESPONSE_LATENCIES_NAME = "first_response_latencies";
static final String APPLICATION_BLOCKING_LATENCIES_NAME = "application_latencies";
static final String REMAINING_DEADLINE_NAME = "remaining_deadline";
static final String CLIENT_BLOCKING_LATENCIES_NAME = "throttling_latencies";
static final String PER_CONNECTION_ERROR_COUNT_NAME = "per_connection_error_count";

Expand Down Expand Up @@ -214,6 +215,16 @@ public static Map<InstrumentSelector, View> getAllViews() {
ImmutableSet.<AttributeKey>builder()
.add(BIGTABLE_PROJECT_ID_KEY, INSTANCE_ID_KEY, APP_PROFILE_KEY, CLIENT_NAME_KEY)
.build());
defineView(
views,
REMAINING_DEADLINE_NAME,
AGGREGATION_WITH_MILLIS_HISTOGRAM,
InstrumentType.HISTOGRAM,
"ms",
ImmutableSet.<AttributeKey>builder()
.addAll(COMMON_ATTRIBUTES)
.add(STREAMING_KEY, STATUS_KEY)
.build());

return views.build();
}
Expand Down
Loading
Loading