-
Notifications
You must be signed in to change notification settings - Fork 93
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
feat: create the backbone of counting errors per connection each minu…
…te. (#2094) * Create the backbone of counting errors per connection each minute. * Clean up creating new interceptors and StatsRecorderWrapper ctor. * Rename setting background task and fix imports. * Temporarily skip exporting per connection metrics to fix test failures. * Temporarily share the tests for debugging purposes * Temporarily add the test for debugging. * Remove the new ExecutorProvider and fix integration test failures. * Update unit tests to reflect the new setup. * Clean up and add tests. * Clean comments and add a TODO. * Improve tests and comments. * Address comments and refactor by defining new classes. * Fix code formatting. * Refactor classes and move to better packages. * Clean up classes and address comments. * Update the scheduler object. * Apply cleanups. * Fix unit tests and avoid hanging when getting error in close(). * Fix code formatting. * Improve error handling in the close() method. * Improve exception logging. * Fix code formatting. * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md --------- Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
- Loading branch information
1 parent
2607fff
commit 7d27816
Showing
10 changed files
with
543 additions
and
17 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -42,3 +42,4 @@ api_key | |
| artman-genfiles | ||
|
|
||
| .flattened-pom.xml | ||
| dependency-reduced-pom.xml | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
57 changes: 57 additions & 0 deletions
57
...tats/src/main/java/com/google/cloud/bigtable/stats/StatsRecorderWrapperForConnection.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,57 @@ | ||
| /* | ||
| * Copyright 2024 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.stats; | ||
|
|
||
| import com.google.api.core.InternalApi; | ||
| import io.opencensus.stats.MeasureMap; | ||
| import io.opencensus.stats.StatsRecorder; | ||
| import io.opencensus.tags.TagContext; | ||
| import io.opencensus.tags.TagContextBuilder; | ||
| import io.opencensus.tags.TagKey; | ||
| import io.opencensus.tags.TagValue; | ||
| import io.opencensus.tags.Tagger; | ||
| import io.opencensus.tags.Tags; | ||
| import java.util.Map; | ||
|
|
||
| /** A wrapper to record built-in metrics for connection metrics not tied to operations/RPCs. */ | ||
| @InternalApi("For internal use only") | ||
| public class StatsRecorderWrapperForConnection { | ||
| private final StatsRecorder statsRecorder; | ||
| private final TagContext tagContext; | ||
| private MeasureMap perConnectionErrorCountMeasureMap; | ||
|
|
||
| public StatsRecorderWrapperForConnection( | ||
| Map<String, String> statsAttributes, StatsRecorder statsRecorder) { | ||
| this.statsRecorder = statsRecorder; | ||
|
|
||
| this.perConnectionErrorCountMeasureMap = statsRecorder.newMeasureMap(); | ||
|
|
||
| Tagger tagger = Tags.getTagger(); | ||
| TagContextBuilder tagContextBuilder = tagger.toBuilder(tagger.getCurrentTagContext()); | ||
| for (Map.Entry<String, String> entry : statsAttributes.entrySet()) { | ||
| tagContextBuilder.putLocal(TagKey.create(entry.getKey()), TagValue.create(entry.getValue())); | ||
| } | ||
| this.tagContext = tagContextBuilder.build(); | ||
| } | ||
|
|
||
| public void putAndRecordPerConnectionErrorCount(long errorCount) { | ||
| perConnectionErrorCountMeasureMap.put( | ||
| BuiltinMeasureConstants.PER_CONNECTION_ERROR_COUNT, errorCount); | ||
|
|
||
| perConnectionErrorCountMeasureMap.record(tagContext); | ||
| perConnectionErrorCountMeasureMap = statsRecorder.newMeasureMap(); | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
89 changes: 89 additions & 0 deletions
89
.../java/com/google/cloud/bigtable/data/v2/stub/metrics/ConnectionErrorCountInterceptor.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,89 @@ | ||
| /* | ||
| * Copyright 2024 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 io.grpc.CallOptions; | ||
| import io.grpc.Channel; | ||
| import io.grpc.ClientCall; | ||
| import io.grpc.ClientInterceptor; | ||
| import io.grpc.ForwardingClientCall; | ||
| import io.grpc.ForwardingClientCallListener; | ||
| import io.grpc.Metadata; | ||
| import io.grpc.MethodDescriptor; | ||
| import io.grpc.Status; | ||
| import java.util.concurrent.atomic.LongAdder; | ||
| import java.util.logging.Level; | ||
| import java.util.logging.Logger; | ||
|
|
||
| /** An interceptor which counts the number of failed responses for a channel. */ | ||
| class ConnectionErrorCountInterceptor implements ClientInterceptor { | ||
| private static final Logger LOG = | ||
| Logger.getLogger(ConnectionErrorCountInterceptor.class.toString()); | ||
| private final LongAdder numOfErrors; | ||
| private final LongAdder numOfSuccesses; | ||
|
|
||
| ConnectionErrorCountInterceptor() { | ||
| numOfErrors = new LongAdder(); | ||
| numOfSuccesses = new LongAdder(); | ||
| } | ||
|
|
||
| @Override | ||
| public <ReqT, RespT> ClientCall<ReqT, RespT> interceptCall( | ||
| MethodDescriptor<ReqT, RespT> methodDescriptor, CallOptions callOptions, Channel channel) { | ||
| return new ForwardingClientCall.SimpleForwardingClientCall<ReqT, RespT>( | ||
| channel.newCall(methodDescriptor, callOptions)) { | ||
| @Override | ||
| public void start(Listener<RespT> responseListener, Metadata headers) { | ||
| super.start( | ||
| new ForwardingClientCallListener.SimpleForwardingClientCallListener<RespT>( | ||
| responseListener) { | ||
| @Override | ||
| public void onClose(Status status, Metadata trailers) { | ||
| // Connection accounting is non-critical, so we log the exception, but let normal | ||
| // processing proceed. | ||
| try { | ||
| handleOnCloseUnsafe(status); | ||
| } catch (Throwable t) { | ||
| if (t instanceof InterruptedException) { | ||
| Thread.currentThread().interrupt(); | ||
| } | ||
| LOG.log( | ||
| Level.WARNING, "Unexpected error while updating connection error stats", t); | ||
| } | ||
| super.onClose(status, trailers); | ||
| } | ||
|
|
||
| private void handleOnCloseUnsafe(Status status) { | ||
| if (status.isOk()) { | ||
| numOfSuccesses.increment(); | ||
| } else { | ||
| numOfErrors.increment(); | ||
| } | ||
| } | ||
| }, | ||
| headers); | ||
| } | ||
| }; | ||
| } | ||
|
|
||
| long getAndResetNumOfErrors() { | ||
| return numOfErrors.sumThenReset(); | ||
| } | ||
|
|
||
| long getAndResetNumOfSuccesses() { | ||
| return numOfSuccesses.sumThenReset(); | ||
| } | ||
| } |
83 changes: 83 additions & 0 deletions
83
.../com/google/cloud/bigtable/data/v2/stub/metrics/ErrorCountPerConnectionMetricTracker.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,83 @@ | ||
| /* | ||
| * Copyright 2024 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.cloud.bigtable.stats.StatsRecorderWrapperForConnection; | ||
| import com.google.cloud.bigtable.stats.StatsWrapper; | ||
| import com.google.common.annotations.VisibleForTesting; | ||
| import com.google.common.collect.ImmutableMap; | ||
| import io.grpc.ClientInterceptor; | ||
| import java.util.Collections; | ||
| import java.util.Set; | ||
| import java.util.WeakHashMap; | ||
| import java.util.concurrent.ScheduledExecutorService; | ||
| import java.util.concurrent.TimeUnit; | ||
|
|
||
| /* Background task that goes through all connections and updates the errors_per_connection metric. */ | ||
| @InternalApi("For internal use only") | ||
| public class ErrorCountPerConnectionMetricTracker implements Runnable { | ||
| private static final Integer PER_CONNECTION_ERROR_COUNT_PERIOD_SECONDS = 60; | ||
| private final Set<ConnectionErrorCountInterceptor> connectionErrorCountInterceptors; | ||
| private final Object interceptorsLock = new Object(); | ||
| // This is not final so that it can be updated and mocked during testing. | ||
| private StatsRecorderWrapperForConnection statsRecorderWrapperForConnection; | ||
|
|
||
| @VisibleForTesting | ||
| void setStatsRecorderWrapperForConnection( | ||
| StatsRecorderWrapperForConnection statsRecorderWrapperForConnection) { | ||
| this.statsRecorderWrapperForConnection = statsRecorderWrapperForConnection; | ||
| } | ||
|
|
||
| public ErrorCountPerConnectionMetricTracker(ImmutableMap<String, String> builtinAttributes) { | ||
| connectionErrorCountInterceptors = | ||
| Collections.synchronizedSet(Collections.newSetFromMap(new WeakHashMap<>())); | ||
|
|
||
| this.statsRecorderWrapperForConnection = | ||
| StatsWrapper.createRecorderForConnection(builtinAttributes); | ||
| } | ||
|
|
||
| public void startConnectionErrorCountTracker(ScheduledExecutorService scheduler) { | ||
| scheduler.scheduleAtFixedRate( | ||
| this, 0, PER_CONNECTION_ERROR_COUNT_PERIOD_SECONDS, TimeUnit.SECONDS); | ||
| } | ||
|
|
||
| public ClientInterceptor getInterceptor() { | ||
| ConnectionErrorCountInterceptor connectionErrorCountInterceptor = | ||
| new ConnectionErrorCountInterceptor(); | ||
| synchronized (interceptorsLock) { | ||
| connectionErrorCountInterceptors.add(connectionErrorCountInterceptor); | ||
| } | ||
| return connectionErrorCountInterceptor; | ||
| } | ||
|
|
||
| @Override | ||
| public void run() { | ||
| synchronized (interceptorsLock) { | ||
| for (ConnectionErrorCountInterceptor interceptor : connectionErrorCountInterceptors) { | ||
| long errors = interceptor.getAndResetNumOfErrors(); | ||
| long successes = interceptor.getAndResetNumOfSuccesses(); | ||
| // We avoid keeping track of inactive connections (i.e., without any failed or successful | ||
| // requests). | ||
| if (errors > 0 || successes > 0) { | ||
| // TODO: add a metric to also keep track of the number of successful requests per each | ||
| // connection. | ||
| statsRecorderWrapperForConnection.putAndRecordPerConnectionErrorCount(errors); | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } |
Oops, something went wrong.