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

Jump to bottom

feat: migrate to OTEL and enable metrics by default #2166

Merged
merged 35 commits into from
Apr 12, 2024
Merged

feat: migrate to OTEL and enable metrics by default #2166

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
043d551
feat: migrate to OTEL exporter (#1788)
mutianf Jan 23, 2024
beb0219
chore: refactor factory class (#2081)
mutianf Feb 5, 2024
b8fd4d5
chore: refactor TracerFactory creation (#2102)
mutianf Feb 16, 2024
08e1719
feat: migrate built in metrics to OTEL (#1796)
mutianf Feb 21, 2024
5e4eb51
Merge branch 'main' into otel
mutianf Feb 23, 2024
65493e7
merge from main
mutianf Feb 23, 2024
cd3467c
feat: migrate per connection error count metric to otel (#2133)
mutianf Mar 4, 2024
eb46d54
feat: migrate per connection error exporter to otel (#2152)
mutianf Mar 11, 2024
15fb42c
chore: clean up stats package (#2163)
mutianf Mar 12, 2024
9d5eb08
🦉 Updates from OwlBot post-processor
gcf-owl-bot[bot] Mar 12, 2024
b35d956
Merge branch 'main' into otel
mutianf Mar 13, 2024
da09fae
fix clirr and integration test
mutianf Mar 13, 2024
15c28ee
fix clirr and integration test
mutianf Mar 13, 2024
618ba7f
🦉 Updates from OwlBot post-processor
gcf-owl-bot[bot] Mar 14, 2024
f383e33
Merge branch 'otel' of https://github.com/googleapis/java-bigtable in…
gcf-owl-bot[bot] Mar 14, 2024
f0b458b
log more information on test failures
mutianf Mar 15, 2024
f07a64b
Merge branch 'main' into otel
mutianf Mar 15, 2024
452571e
address comments
mutianf Mar 18, 2024
970f4f7
🦉 Updates from OwlBot post-processor
gcf-owl-bot[bot] Mar 18, 2024
29cd9ae
include version
mutianf Mar 19, 2024
98aef25
fix flaky test
mutianf Mar 19, 2024
2e26e20
Merge branch 'main' into otel
mutianf Mar 25, 2024
377d385
remove otel dependencies since they're added to shared dependencies
mutianf Mar 25, 2024
388cd71
Merge branch 'main' into otel
mutianf Mar 28, 2024
b816dda
backport new tests since 2.37
mutianf Mar 28, 2024
21cad39
set feature flag when metrics is enabled
mutianf Mar 28, 2024
1f291fb
record metrics whenever it's registered
mutianf Mar 29, 2024
56d55cb
record metrics whenever it's registered
mutianf Mar 29, 2024
59a05f4
🦉 Updates from OwlBot post-processor
gcf-owl-bot[bot] Mar 29, 2024
b890843
Merge branch 'otel' of https://github.com/googleapis/java-bigtable in…
gcf-owl-bot[bot] Mar 29, 2024
5349c2b
🦉 Updates from OwlBot post-processor
gcf-owl-bot[bot] Mar 29, 2024
a6c0d91
Merge branch 'otel' of https://github.com/googleapis/java-bigtable in…
gcf-owl-bot[bot] Mar 29, 2024
f00e838
remove stale reference
mutianf Apr 2, 2024
b330948
clean up
mutianf Apr 2, 2024
a2423ec
Merge branch 'main' into otel
mutianf Apr 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Prev Previous commit
Next Next commit
address comments
  • Loading branch information
@mutianf
mutianf committed Mar 18, 2024
commit 452571e7e9532fc3584b03ecdb2d807f8374515a
85 changes: 35 additions & 50 deletions google-cloud-bigtable/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,41 @@
<artifactId>threetenbp</artifactId>
</dependency>

<!-- metrics dependencies -->
<dependency>
<groupId>io.opentelemetry</groupId>
<artifactId>opentelemetry-api</artifactId>
</dependency>
<dependency>
<groupId>io.opentelemetry</groupId>
<artifactId>opentelemetry-sdk</artifactId>
</dependency>
<dependency>
<groupId>io.opentelemetry</groupId>
<artifactId>opentelemetry-sdk-metrics</artifactId>
</dependency>
<dependency>
<groupId>io.opentelemetry</groupId>
<artifactId>opentelemetry-sdk-common</artifactId>
</dependency>
<dependency>
<groupId>com.google.cloud.opentelemetry</groupId>
<artifactId>detector-resources-support</artifactId>
</dependency>
<dependency>
<groupId>io.opentelemetry</groupId>
<artifactId>opentelemetry-sdk-testing</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>com.google.cloud</groupId>
<artifactId>google-cloud-monitoring</artifactId>
</dependency>
<dependency>
<groupId>com.google.api.grpc</groupId>
<artifactId>proto-google-cloud-monitoring-v3</artifactId>
</dependency>

<!-- Test dependencies -->
<dependency>
<groupId>com.google.api</groupId>
Expand Down Expand Up @@ -259,23 +294,6 @@
</exclusion>
</exclusions>
</dependency>
<dependency>
<groupId>com.google.cloud</groupId>
<artifactId>google-cloud-monitoring</artifactId>
<exclusions>
<exclusion>
<!-- using the perfmark version in opencensus -->
<groupId>io.perfmark</groupId>
<artifactId>perfmark-api</artifactId>
</exclusion>
</exclusions>
<scope>runtime</scope>
</dependency>
<dependency>
<groupId>com.google.api.grpc</groupId>
<artifactId>proto-google-cloud-monitoring-v3</artifactId>
<scope>runtime</scope>
</dependency>
<dependency>
<groupId>com.google.truth</groupId>
<artifactId>truth</artifactId>
Expand Down Expand Up @@ -322,39 +340,6 @@
<artifactId>mockito-core</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>io.opentelemetry</groupId>
<artifactId>opentelemetry-api</artifactId>
</dependency>
<dependency>
<groupId>io.opentelemetry</groupId>
<artifactId>opentelemetry-sdk</artifactId>
</dependency>
<dependency>
<groupId>io.opentelemetry</groupId>
<artifactId>opentelemetry-sdk-metrics</artifactId>
</dependency>
<dependency>
<groupId>io.opentelemetry</groupId>
<artifactId>opentelemetry-sdk-common</artifactId>
</dependency>
<dependency>
<groupId>com.google.cloud.opentelemetry</groupId>
<artifactId>detector-resources-support</artifactId>
</dependency>
<dependency>
<groupId>io.opentelemetry</groupId>
<artifactId>opentelemetry-sdk-testing</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>com.google.cloud</groupId>
<artifactId>google-cloud-monitoring</artifactId>
</dependency>
<dependency>
<groupId>com.google.api.grpc</groupId>
<artifactId>proto-google-cloud-monitoring-v3</artifactId>
</dependency>
</dependencies>

<profiles>
Expand Down
9 changes: 4 additions & 5 deletions .../java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableCloudMonitoringExporter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@
import java.util.Arrays;
import java.util.Collection;
import java.util.List;
import java.util.Set;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.logging.Level;
import java.util.logging.Logger;
Expand Down Expand Up @@ -96,7 +95,7 @@ public final class BigtableCloudMonitoringExporter implements MetricExporter {

private CompletableResultCode lastExportCode;

private static final Set<String> BIGTABLE_TABLE_METRICS =
private static final ImmutableList<String> BIGTABLE_TABLE_METRICS =
ImmutableSet.of(
OPERATION_LATENCIES_NAME,
ATTEMPT_LATENCIES_NAME,
Expand All @@ -108,12 +107,12 @@ public final class BigtableCloudMonitoringExporter implements MetricExporter {
CONNECTIVITY_ERROR_COUNT_NAME)
.stream()
.map(m -> METER_NAME + m)
.collect(Collectors.toSet());
.collect(ImmutableList.toImmutableList());

private static final Set<String> APPLICATION_METRICS =
private static final ImmutableList<String> APPLICATION_METRICS =
ImmutableSet.of(PER_CONNECTION_ERROR_COUNT_NAME).stream()
.map(m -> METER_NAME + m)
.collect(Collectors.toSet());
.collect(ImmutableList.toImmutableList());

public static BigtableCloudMonitoringExporter create(
String projectId, @Nullable Credentials credentials) throws IOException {
Expand Down
3 changes: 2 additions & 1 deletion ...va/com/google/cloud/bigtable/data/v2/stub/metrics/CustomOpenTelemetryMetricsProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/
package com.google.cloud.bigtable.data.v2.stub.metrics;

import com.google.common.base.MoreObjects;
import io.opentelemetry.api.OpenTelemetry;

/**
Expand Down Expand Up @@ -64,6 +65,6 @@ public OpenTelemetry getOpenTelemetry() {

@Override
public String toString() {
return otel.toString();
return MoreObjects.toStringHelper(this).add("openTelemetry", otel).toString();
}
}
6 changes: 5 additions & 1 deletion .../src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/DefaultMetricsProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

import com.google.api.core.InternalApi;
import com.google.auth.Credentials;
import com.google.common.base.MoreObjects;
import io.opentelemetry.api.OpenTelemetry;
import io.opentelemetry.sdk.OpenTelemetrySdk;
import io.opentelemetry.sdk.metrics.SdkMeterProvider;
Expand Down Expand Up @@ -54,6 +55,9 @@ public OpenTelemetry getOpenTelemetry(String projectId, @Nullable Credentials cr

@Override
public String toString() {
return "DefaultMetricsProvider: projectId=" + projectId + " " + openTelemetry;
return MoreObjects.toStringHelper(this)
.add("projectId", projectId)
.add("openTelemetry", openTelemetry)
.toString();
}
}
4 changes: 3 additions & 1 deletion ...ble/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/NoopMetricsProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
*/
package com.google.cloud.bigtable.data.v2.stub.metrics;

import com.google.common.base.MoreObjects;

/**
* Set {@link
* com.google.cloud.bigtable.data.v2.BigtableDataSettings.Builder#setMetricsProvider(MetricsProvider)},
Expand All @@ -29,6 +31,6 @@ private NoopMetricsProvider() {}

@Override
public String toString() {
return "NoopMetricsProvider";
return MoreObjects.toStringHelper(this).toString();
}
}
37 changes: 37 additions & 0 deletions google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/it/MetricsITUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
/*
* 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.it;

import com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsConstants;
import com.google.common.truth.Correspondence;
import io.opentelemetry.sdk.metrics.data.MetricData;
import io.opentelemetry.sdk.metrics.data.PointData;

public class MetricsITUtils {

static final Correspondence<MetricData, String> METRIC_DATA_NAME_CONTAINS =
Correspondence.from((md, s) -> md.getName().contains(s), "contains name");

static final Correspondence<PointData, String> POINT_DATA_CLUSTER_ID_CONTAINS =
Correspondence.from(
(pd, s) -> pd.getAttributes().get(BuiltinMetricsConstants.CLUSTER_ID_KEY).contains(s),
"contains attributes");

static final Correspondence<PointData, String> POINT_DATA_ZONE_ID_CONTAINS =
Correspondence.from(
(pd, s) -> pd.getAttributes().get(BuiltinMetricsConstants.ZONE_ID_KEY).contains(s),
"contains attributes");
}
35 changes: 13 additions & 22 deletions ...gtable/src/test/java/com/google/cloud/bigtable/data/v2/it/StreamingMetricsMetadataIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@
*/
package com.google.cloud.bigtable.data.v2.it;

import static com.google.cloud.bigtable.data.v2.it.MetricsITUtils.METRIC_DATA_NAME_CONTAINS;
import static com.google.cloud.bigtable.data.v2.it.MetricsITUtils.POINT_DATA_CLUSTER_ID_CONTAINS;
import static com.google.cloud.bigtable.data.v2.it.MetricsITUtils.POINT_DATA_ZONE_ID_CONTAINS;
import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.TruthJUnit.assume;

Expand All @@ -31,7 +34,6 @@
import com.google.cloud.bigtable.test_helpers.env.EmulatorEnv;
import com.google.cloud.bigtable.test_helpers.env.TestEnvRule;
import com.google.common.collect.Lists;
import com.google.common.truth.Correspondence;
import io.opentelemetry.api.OpenTelemetry;
import io.opentelemetry.sdk.OpenTelemetrySdk;
import io.opentelemetry.sdk.metrics.SdkMeterProvider;
Expand All @@ -57,19 +59,6 @@ public class StreamingMetricsMetadataIT {
private BigtableDataClient client;
private InMemoryMetricReader metricReader;

private static final Correspondence<MetricData, String> METRIC_DATA_CONTAINS =
Correspondence.from((md, s) -> md.getName().contains(s), "contains name");

private static final Correspondence<PointData, String> POINT_DATA_CLUSTER_ID_IS =
Correspondence.from(
(pd, s) -> pd.getAttributes().get(BuiltinMetricsConstants.CLUSTER_ID_KEY).contains(s),
"contains attributes");

private static final Correspondence<PointData, String> POINT_DATA_ZONE_ID_IS =
Correspondence.from(
(pd, s) -> pd.getAttributes().get(BuiltinMetricsConstants.ZONE_ID_KEY).contains(s),
"contains attributes");

@Before
public void setup() throws IOException {
assume()
Expand Down Expand Up @@ -121,9 +110,9 @@ public void testSuccess() throws Exception {
.collect(Collectors.toList());

assertThat(allMetricData)
.comparingElementsUsing(METRIC_DATA_CONTAINS)
.comparingElementsUsing(METRIC_DATA_NAME_CONTAINS)
.contains(BuiltinMetricsConstants.OPERATION_LATENCIES_NAME);
assertThat(metrics.size()).isEqualTo(1);
assertThat(metrics).hasSize(1);

MetricData metricData = metrics.get(0);
List<PointData> pointData = new ArrayList<>(metricData.getData().getPoints());
Expand All @@ -137,10 +126,10 @@ public void testSuccess() throws Exception {
.collect(Collectors.toList());

assertThat(pointData)
.comparingElementsUsing(POINT_DATA_CLUSTER_ID_IS)
.comparingElementsUsing(POINT_DATA_CLUSTER_ID_CONTAINS)
.contains(clusters.get(0).getId());
assertThat(pointData)
.comparingElementsUsing(POINT_DATA_ZONE_ID_IS)
.comparingElementsUsing(POINT_DATA_ZONE_ID_CONTAINS)
.contains(clusters.get(0).getZone());
assertThat(clusterAttributes).contains(clusters.get(0).getId());
assertThat(zoneAttributes).contains(clusters.get(0).getZone());
Expand All @@ -161,9 +150,9 @@ public void testFailure() {
.collect(Collectors.toList());

assertThat(allMetricData)
.comparingElementsUsing(METRIC_DATA_CONTAINS)
.comparingElementsUsing(METRIC_DATA_NAME_CONTAINS)
.contains(BuiltinMetricsConstants.OPERATION_LATENCIES_NAME);
assertThat(metrics.size()).isEqualTo(1);
assertThat(metrics).hasSize(1);

MetricData metricData = metrics.get(0);
List<PointData> pointData = new ArrayList<>(metricData.getData().getPoints());
Expand All @@ -176,8 +165,10 @@ public void testFailure() {
.map(pd -> pd.getAttributes().get(BuiltinMetricsConstants.ZONE_ID_KEY))
.collect(Collectors.toList());

assertThat(pointData).comparingElementsUsing(POINT_DATA_CLUSTER_ID_IS).contains("unspecified");
assertThat(pointData).comparingElementsUsing(POINT_DATA_ZONE_ID_IS).contains("global");
assertThat(pointData)
.comparingElementsUsing(POINT_DATA_CLUSTER_ID_CONTAINS)
.contains("unspecified");
assertThat(pointData).comparingElementsUsing(POINT_DATA_ZONE_ID_CONTAINS).contains("global");
assertThat(clusterAttributes).contains("unspecified");
assertThat(zoneAttributes).contains("global");
}
Expand Down
31 changes: 11 additions & 20 deletions ...d-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/it/UnaryMetricsMetadataIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@
*/
package com.google.cloud.bigtable.data.v2.it;

import static com.google.cloud.bigtable.data.v2.it.MetricsITUtils.METRIC_DATA_NAME_CONTAINS;
import static com.google.cloud.bigtable.data.v2.it.MetricsITUtils.POINT_DATA_CLUSTER_ID_CONTAINS;
import static com.google.cloud.bigtable.data.v2.it.MetricsITUtils.POINT_DATA_ZONE_ID_CONTAINS;
import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.TruthJUnit.assume;

Expand All @@ -29,7 +32,6 @@
import com.google.cloud.bigtable.data.v2.stub.metrics.CustomOpenTelemetryMetricsProvider;
import com.google.cloud.bigtable.test_helpers.env.EmulatorEnv;
import com.google.cloud.bigtable.test_helpers.env.TestEnvRule;
import com.google.common.truth.Correspondence;
import io.opentelemetry.api.OpenTelemetry;
import io.opentelemetry.sdk.OpenTelemetrySdk;
import io.opentelemetry.sdk.metrics.SdkMeterProvider;
Expand All @@ -56,19 +58,6 @@ public class UnaryMetricsMetadataIT {
private BigtableDataClient client;
private InMemoryMetricReader metricReader;

private static final Correspondence<MetricData, String> METRIC_DATA_CONTAINS =
Correspondence.from((md, s) -> md.getName().contains(s), "contains name");

private static final Correspondence<PointData, String> POINT_DATA_CLUSTER_ID_IS =
Correspondence.from(
(pd, s) -> pd.getAttributes().get(BuiltinMetricsConstants.CLUSTER_ID_KEY).contains(s),
"contains attributes");

private static final Correspondence<PointData, String> POINT_DATA_ZONE_ID_IS =
Correspondence.from(
(pd, s) -> pd.getAttributes().get(BuiltinMetricsConstants.ZONE_ID_KEY).contains(s),
"contains attributes");

@Before
public void setup() throws IOException {
assume()
Expand Down Expand Up @@ -126,7 +115,7 @@ public void testSuccess() throws Exception {
.collect(Collectors.toList());

assertThat(allMetricData)
.comparingElementsUsing(METRIC_DATA_CONTAINS)
.comparingElementsUsing(METRIC_DATA_NAME_CONTAINS)
.contains(BuiltinMetricsConstants.OPERATION_LATENCIES_NAME);
assertThat(metrics).hasSize(1);
mutianf marked this conversation as resolved.
Show resolved Hide resolved

Expand All @@ -142,10 +131,10 @@ public void testSuccess() throws Exception {
.collect(Collectors.toList());

assertThat(pointData)
.comparingElementsUsing(POINT_DATA_CLUSTER_ID_IS)
.comparingElementsUsing(POINT_DATA_CLUSTER_ID_CONTAINS)
.contains(clusters.get(0).getId());
assertThat(pointData)
.comparingElementsUsing(POINT_DATA_ZONE_ID_IS)
.comparingElementsUsing(POINT_DATA_ZONE_ID_CONTAINS)
.contains(clusters.get(0).getZone());
assertThat(clusterAttributes).contains(clusters.get(0).getId());
assertThat(zoneAttributes).contains(clusters.get(0).getZone());
Expand Down Expand Up @@ -185,14 +174,16 @@ public void testFailure() throws Exception {
}

assertThat(allMetricData)
.comparingElementsUsing(METRIC_DATA_CONTAINS)
.comparingElementsUsing(METRIC_DATA_NAME_CONTAINS)
.contains(BuiltinMetricsConstants.OPERATION_LATENCIES_NAME);
assertThat(metricData).isNotNull();

List<PointData> pointData = new ArrayList<>(metricData.getData().getPoints());

assertThat(pointData).comparingElementsUsing(POINT_DATA_CLUSTER_ID_IS).contains("unspecified");
assertThat(pointData).comparingElementsUsing(POINT_DATA_ZONE_ID_IS).contains("global");
assertThat(pointData)
.comparingElementsUsing(POINT_DATA_CLUSTER_ID_CONTAINS)
.contains("unspecified");
assertThat(pointData).comparingElementsUsing(POINT_DATA_ZONE_ID_CONTAINS).contains("global");
List<String> clusterAttributes =
pointData.stream()
.map(pd -> pd.getAttributes().get(BuiltinMetricsConstants.CLUSTER_ID_KEY))
Expand Down