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: send attempt and timestamp in headers #935
Changes from 6 commits
2e0fe77
e6e53f8
49142dc
73cecdd
ec37f18
857ebef
875840c
3d94f6e
492cdab
22a0f62
98d6043
69bdbba
bee61ed
6ec8f12
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,46 @@ | ||
/* | ||
* Copyright 2021 Google LLC | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"); | ||
* you may not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* https://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
|
||
package com.google.cloud.bigtable.data.v2.stub.metrics; | ||
|
||
import com.google.api.core.InternalApi; | ||
import com.google.api.gax.rpc.ApiCallContext; | ||
import com.google.api.gax.rpc.ResponseObserver; | ||
import com.google.api.gax.rpc.ServerStreamingCallable; | ||
|
||
/** | ||
* A callable that injects client timestamp and current attempt number to request headers. Attempt | ||
* number starts from 0. | ||
*/ | ||
@InternalApi("For internal use only") | ||
public final class ExtraHeadersSeverStreamingCallable<RequestT, ResponseT> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. typo: missing r in server There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe rename it to StatsHeader.... |
||
extends ServerStreamingCallable<RequestT, ResponseT> { | ||
private final ServerStreamingCallable innerCallable; | ||
|
||
public ExtraHeadersSeverStreamingCallable(ServerStreamingCallable innerCallable) { | ||
this.innerCallable = innerCallable; | ||
} | ||
|
||
@Override | ||
public void call( | ||
RequestT request, | ||
ResponseObserver<ResponseT> responseObserver, | ||
ApiCallContext apiCallContext) { | ||
ApiCallContext newCallContext = | ||
apiCallContext.withExtraHeaders(Util.createExtraHeaders(apiCallContext)); | ||
innerCallable.call(request, responseObserver, newCallContext); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,43 @@ | ||
/* | ||
* Copyright 2021 Google LLC | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"); | ||
* you may not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* https://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
|
||
package com.google.cloud.bigtable.data.v2.stub.metrics; | ||
|
||
import com.google.api.core.ApiFuture; | ||
import com.google.api.core.InternalApi; | ||
import com.google.api.gax.rpc.ApiCallContext; | ||
import com.google.api.gax.rpc.UnaryCallable; | ||
|
||
/** | ||
* A callable that injects client timestamp and current attempt number to request headers. Attempt | ||
* number starts from 0. | ||
*/ | ||
@InternalApi("For internal use only") | ||
public final class ExtraHeadersUnaryCallable<RequestT, ResponseT> | ||
extends UnaryCallable<RequestT, ResponseT> { | ||
private final UnaryCallable innerCallable; | ||
|
||
public ExtraHeadersUnaryCallable(UnaryCallable innerCallable) { | ||
this.innerCallable = innerCallable; | ||
} | ||
|
||
@Override | ||
public ApiFuture futureCall(RequestT request, ApiCallContext apiCallContext) { | ||
ApiCallContext newCallContext = | ||
apiCallContext.withExtraHeaders(Util.createExtraHeaders(apiCallContext)); | ||
return innerCallable.futureCall(request, newCallContext); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,20 +15,31 @@ | |
*/ | ||
package com.google.cloud.bigtable.data.v2.stub.metrics; | ||
|
||
import com.google.api.gax.rpc.ApiCallContext; | ||
import com.google.api.gax.rpc.ApiException; | ||
import com.google.api.gax.rpc.StatusCode; | ||
import com.google.api.gax.rpc.StatusCode.Code; | ||
import com.google.common.collect.ImmutableMap; | ||
import io.grpc.Metadata; | ||
import io.grpc.Status; | ||
import io.grpc.StatusException; | ||
import io.grpc.StatusRuntimeException; | ||
import io.opencensus.tags.TagValue; | ||
import java.util.Arrays; | ||
import java.util.List; | ||
import java.util.Map; | ||
import java.util.concurrent.CancellationException; | ||
import java.util.concurrent.ExecutionException; | ||
import java.util.concurrent.Future; | ||
import javax.annotation.Nullable; | ||
|
||
/** Utilities to help integrating with OpenCensus. */ | ||
class Util { | ||
static final Metadata.Key<String> ATTEMPT_HEADER_KEY = | ||
Metadata.Key.of("bigtable-attempt", Metadata.ASCII_STRING_MARSHALLER); | ||
static final Metadata.Key<String> ATTEMPT_EPOCH_KEY = | ||
Metadata.Key.of("bigtable-client-attempt-epoch", Metadata.ASCII_STRING_MARSHALLER); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think you can drop There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure if its better, but have you considered sending the timestamp as binary? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will keep the naming and timestamp for since the change on the server side depends on this and is already released. We can update them later if we see any problems. |
||
|
||
private static final TagValue OK_STATUS = TagValue.create(StatusCode.Code.OK.toString()); | ||
|
||
/** Convert an exception into a value that can be used as an OpenCensus tag value. */ | ||
|
@@ -71,4 +82,20 @@ static TagValue extractStatus(Future<?> future) { | |
} | ||
return extractStatus(error); | ||
} | ||
|
||
/** | ||
* Create extra headers with attempt number and client timestamp from api call context. Attempt | ||
* number starts from 0. | ||
*/ | ||
static Map<String, List<String>> createExtraHeaders(ApiCallContext apiCallContext) { | ||
ImmutableMap.Builder headers = ImmutableMap.<String, List<String>>builder(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. please out type params on the variable type |
||
headers.put( | ||
ATTEMPT_EPOCH_KEY.name(), Arrays.asList(String.valueOf(System.currentTimeMillis()))); | ||
|
||
if (apiCallContext.getTracer() instanceof BigtableTracer) { | ||
mutianf marked this conversation as resolved.
Show resolved
Hide resolved
|
||
int attemptCount = ((BigtableTracer) apiCallContext.getTracer()).getAttempt(); | ||
headers.put(ATTEMPT_HEADER_KEY.name(), Arrays.asList(String.valueOf(attemptCount))); | ||
} | ||
return headers.build(); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should probably happen first