Skip to content

Commit b50f823

Browse files
ehsannasehsannas
committed
feat: Drop the setTracingEnabled flag in OpenTelemetryOptions (Beta api change).
1 parent d59ac1b commit b50f823

File tree

8 files changed

+112
-186
lines changed

8 files changed

+112
-186
lines changed

‎google-cloud-firestore/src/main/java/com/google/cloud/firestore/FirestoreOpenTelemetryOptions.java

-35
Original file line numberDiff line numberDiff line change
@@ -27,25 +27,12 @@
2727
*/
2828
@BetaApi
2929
public class FirestoreOpenTelemetryOptions {
30-
private final boolean tracingEnabled;
3130
private final @Nullable OpenTelemetry openTelemetry;
3231

3332
FirestoreOpenTelemetryOptions(Builder builder) {
34-
this.tracingEnabled = builder.tracingEnabled;
3533
this.openTelemetry = builder.openTelemetry;
3634
}
3735

38-
/**
39-
* @deprecated This method will be removed in the next minor version update. Please use a no-op
40-
* TracerProvider or set the environment variable `FIRESTORE_ENABLE_TRACING=OFF` to disable
41-
* tracing. If the GlobalOpenTelemetry or the OpenTelemetry instance passed to Firestore
42-
* contain a valid TracerProvider, the Firestore client will generate spans by utilizing it.
43-
*/
44-
@Deprecated
45-
public boolean isTracingEnabled() {
46-
return tracingEnabled;
47-
}
48-
4936
public OpenTelemetry getOpenTelemetry() {
5037
return openTelemetry;
5138
}
@@ -61,18 +48,13 @@ public static FirestoreOpenTelemetryOptions.Builder newBuilder() {
6148
}
6249

6350
public static class Builder {
64-
65-
private boolean tracingEnabled;
66-
6751
@Nullable private OpenTelemetry openTelemetry;
6852

6953
private Builder() {
70-
tracingEnabled = false;
7154
openTelemetry = null;
7255
}
7356

7457
private Builder(FirestoreOpenTelemetryOptions options) {
75-
this.tracingEnabled = options.tracingEnabled;
7658
this.openTelemetry = options.openTelemetry;
7759
}
7860

@@ -81,23 +63,6 @@ public FirestoreOpenTelemetryOptions build() {
8163
return new FirestoreOpenTelemetryOptions(this);
8264
}
8365

84-
/**
85-
* Sets whether tracing should be enabled.
86-
*
87-
* @param tracingEnabled Whether tracing should be enabled.
88-
* @deprecated This method will be removed in the next minor version update. Please use a no-op
89-
* TracerProvider or set the environment variable `FIRESTORE_ENABLE_TRACING=OFF` to disable
90-
* tracing. If the GlobalOpenTelemetry or the OpenTelemetry instance passed to Firestore
91-
* contains a valid TracerProvider, the Firestore client will generate spans by utilizing
92-
* it.
93-
*/
94-
@Deprecated
95-
@Nonnull
96-
public FirestoreOpenTelemetryOptions.Builder setTracingEnabled(boolean tracingEnabled) {
97-
this.tracingEnabled = tracingEnabled;
98-
return this;
99-
}
100-
10166
/**
10267
* Sets the {@link OpenTelemetry} to use with this Firestore instance. If telemetry collection
10368
* is enabled, but an `OpenTelemetry` is not provided, the Firestore SDK will attempt to use the

‎google-cloud-firestore/src/main/java/com/google/cloud/firestore/telemetry/EnabledTraceUtil.java

+6
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
import io.opentelemetry.api.trace.SpanKind;
3232
import io.opentelemetry.api.trace.StatusCode;
3333
import io.opentelemetry.api.trace.Tracer;
34+
import io.opentelemetry.api.trace.TracerProvider;
3435
import io.opentelemetry.instrumentation.grpc.v1_6.GrpcTelemetry;
3536
import java.util.Map;
3637
import javax.annotation.Nonnull;
@@ -84,6 +85,11 @@ public ManagedChannelBuilder apply(ManagedChannelBuilder managedChannelBuilder)
8485
@Override
8586
@Nullable
8687
public ApiFunction<ManagedChannelBuilder, ManagedChannelBuilder> getChannelConfigurator() {
88+
// Note: using `==` rather than `.equals` since OpenTelemetry has only 1 static instance of
89+
// `TracerProvider.noop`.
90+
if (openTelemetry.getTracerProvider() == TracerProvider.noop()) {
91+
return null;
92+
}
8793
return new OpenTelemetryGrpcChannelConfigurator();
8894
}
8995

‎google-cloud-firestore/src/main/java/com/google/cloud/firestore/telemetry/TraceUtil.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ public interface TraceUtil {
7575
* @return An instance of the TraceUtil class.
7676
*/
7777
static TraceUtil getInstance(@Nonnull FirestoreOptions firestoreOptions) {
78-
boolean createEnabledInstance = firestoreOptions.getOpenTelemetryOptions().isTracingEnabled();
78+
boolean createEnabledInstance = true;
7979

8080
// The environment variable can override options to enable/disable telemetry collection.
8181
String enableTracingEnvVar = System.getenv(ENABLE_TRACING_ENV_VAR);

‎google-cloud-firestore/src/test/java/com/google/cloud/firestore/OpenTelemetryOptionsTest.java

+36-80
Original file line numberDiff line numberDiff line change
@@ -18,11 +18,12 @@
1818

1919
import static com.google.common.truth.Truth.assertThat;
2020

21-
import com.google.cloud.firestore.telemetry.DisabledTraceUtil;
2221
import com.google.cloud.firestore.telemetry.EnabledTraceUtil;
2322
import io.opentelemetry.api.GlobalOpenTelemetry;
24-
import io.opentelemetry.api.OpenTelemetry;
23+
import io.opentelemetry.api.trace.TracerProvider;
2524
import io.opentelemetry.sdk.OpenTelemetrySdk;
25+
import io.opentelemetry.sdk.resources.Resource;
26+
import io.opentelemetry.sdk.trace.SdkTracerProvider;
2627
import javax.annotation.Nullable;
2728
import org.junit.After;
2829
import org.junit.Before;
@@ -49,112 +50,67 @@ FirestoreOptions.Builder getBaseOptions() {
4950
}
5051

5152
@Test
52-
public void defaultOptionsDisablesTelemetryCollection() {
53+
public void defaultOptionsUsesEnabledTraceUtilWithNoopOpenTelemetry() {
5354
FirestoreOptions firestoreOptions = getBaseOptions().build();
5455
firestore = firestoreOptions.getService();
55-
assertThat(firestore.getOptions().getOpenTelemetryOptions().isTracingEnabled()).isFalse();
5656
assertThat(firestore.getOptions().getOpenTelemetryOptions().getOpenTelemetry()).isNull();
5757
assertThat(firestore.getOptions().getTraceUtil()).isNotNull();
58-
assertThat(firestore.getOptions().getTraceUtil() instanceof DisabledTraceUtil).isTrue();
59-
}
60-
61-
@Test
62-
public void canEnableTelemetryCollectionWithoutOpenTelemetryInstance() {
63-
FirestoreOptions firestoreOptions =
64-
getBaseOptions()
65-
.setOpenTelemetryOptions(
66-
FirestoreOpenTelemetryOptions.newBuilder().setTracingEnabled(true).build())
67-
.build();
68-
firestore = firestoreOptions.getService();
69-
assertThat(firestore.getOptions().getOpenTelemetryOptions().isTracingEnabled()).isTrue();
70-
assertThat(firestore.getOptions().getOpenTelemetryOptions().getOpenTelemetry()).isNull();
71-
assertThat(firestore.getOptions().getTraceUtil()).isNotNull();
72-
assertThat(firestore.getOptions().getTraceUtil() instanceof EnabledTraceUtil).isTrue();
73-
}
74-
75-
@Test
76-
public void canEnableTelemetryCollectionWithOpenTelemetryInstance() {
77-
OpenTelemetry openTelemetry = GlobalOpenTelemetry.get();
78-
FirestoreOptions firestoreOptions =
79-
getBaseOptions()
80-
.setOpenTelemetryOptions(
81-
FirestoreOpenTelemetryOptions.newBuilder()
82-
.setTracingEnabled(true)
83-
.setOpenTelemetry(openTelemetry)
84-
.build())
85-
.build();
86-
firestore = firestoreOptions.getService();
87-
assertThat(firestore.getOptions().getOpenTelemetryOptions().isTracingEnabled()).isTrue();
88-
assertThat(firestore.getOptions().getOpenTelemetryOptions().getOpenTelemetry())
89-
.isEqualTo(openTelemetry);
90-
assertThat(firestore.getOptions().getTraceUtil()).isNotNull();
9158
assertThat(firestore.getOptions().getTraceUtil() instanceof EnabledTraceUtil).isTrue();
59+
EnabledTraceUtil enabledTraceUtil = (EnabledTraceUtil) firestore.getOptions().getTraceUtil();
60+
// Assert that a Noop tracer provider is used by default.
61+
assertThat(enabledTraceUtil.getOpenTelemetry().getTracerProvider())
62+
.isSameInstanceAs(TracerProvider.noop());
9263
}
9364

9465
@Test
95-
public void canDisableTelemetryCollectionWhileOpenTelemetryInstanceIsNotNull() {
96-
OpenTelemetry openTelemetry = GlobalOpenTelemetry.get();
97-
FirestoreOptions firestoreOptions =
98-
getBaseOptions()
99-
.setOpenTelemetryOptions(
100-
FirestoreOpenTelemetryOptions.newBuilder()
101-
.setTracingEnabled(false)
102-
.setOpenTelemetry(openTelemetry)
103-
.build())
104-
.build();
105-
firestore = firestoreOptions.getService();
106-
assertThat(firestore.getOptions().getOpenTelemetryOptions().isTracingEnabled()).isFalse();
107-
assertThat(firestore.getOptions().getOpenTelemetryOptions().getOpenTelemetry())
108-
.isEqualTo(openTelemetry);
109-
assertThat(firestore.getOptions().getTraceUtil()).isNotNull();
110-
assertThat(firestore.getOptions().getTraceUtil() instanceof DisabledTraceUtil).isTrue();
111-
}
66+
public void existenceOfGlobalOpenTelemetryEnablesTracingWithTheGlobalTracerProvider() {
67+
// Make a custom TracerProvider.
68+
Resource resource =
69+
Resource.getDefault().merge(Resource.builder().put("service.name", "test").build());
70+
SdkTracerProvider myTracerProvider = SdkTracerProvider.builder().setResource(resource).build();
11271

113-
@Test
114-
public void existenceOfGlobalOpenTelemetryDoesNotEnableTracing() {
115-
// Register a global OpenTelemetry SDK.
116-
OpenTelemetrySdk.builder().buildAndRegisterGlobal();
72+
// Register a GlobalOpenTelemetry with the custom tracer provider.
73+
OpenTelemetrySdk.builder().setTracerProvider(myTracerProvider).buildAndRegisterGlobal();
11774

118-
// Make sure Firestore does not use GlobalOpenTelemetry by default.
75+
// Do NOT pass an OpenTelemetry instance to Firestore.
11976
FirestoreOptions firestoreOptions = getBaseOptions().build();
12077
firestore = firestoreOptions.getService();
121-
assertThat(firestore.getOptions().getOpenTelemetryOptions().isTracingEnabled()).isFalse();
78+
79+
// An OpenTelemetry instance has not been set in options.
12280
assertThat(firestore.getOptions().getOpenTelemetryOptions().getOpenTelemetry()).isNull();
81+
82+
// Assert that tracing is enabled and is using the custom tracer provider from the
83+
// GlobalOpenTelemetry.
12384
assertThat(firestore.getOptions().getTraceUtil()).isNotNull();
124-
assertThat(firestore.getOptions().getTraceUtil() instanceof DisabledTraceUtil).isTrue();
85+
assertThat(firestore.getOptions().getTraceUtil() instanceof EnabledTraceUtil).isTrue();
86+
EnabledTraceUtil enabledTraceUtil = (EnabledTraceUtil) firestore.getOptions().getTraceUtil();
87+
assertThat(enabledTraceUtil.getOpenTelemetry().getTracerProvider().equals(myTracerProvider));
12588
}
12689

12790
@Test
12891
public void canPassOpenTelemetrySdkInstanceToFirestore() {
129-
OpenTelemetrySdk myOpenTelemetrySdk = OpenTelemetrySdk.builder().build();
92+
// Make a custom TracerProvider and make an OpenTelemetry instance with it.
93+
Resource resource =
94+
Resource.getDefault().merge(Resource.builder().put("service.name", "test").build());
95+
SdkTracerProvider myTracerProvider = SdkTracerProvider.builder().setResource(resource).build();
96+
OpenTelemetrySdk myOpenTelemetrySdk =
97+
OpenTelemetrySdk.builder().setTracerProvider(myTracerProvider).build();
98+
99+
// Pass it to Firestore.
130100
FirestoreOptions firestoreOptions =
131101
getBaseOptions()
132102
.setOpenTelemetryOptions(
133103
FirestoreOpenTelemetryOptions.newBuilder()
134-
.setTracingEnabled(true)
135104
.setOpenTelemetry(myOpenTelemetrySdk)
136105
.build())
137106
.build();
138107
firestore = firestoreOptions.getService();
108+
assertThat(firestore.getOptions().getOpenTelemetryOptions().getOpenTelemetry())
109+
.isEqualTo(myOpenTelemetrySdk);
110+
assertThat(firestore.getOptions().getTraceUtil() instanceof EnabledTraceUtil).isTrue();
139111
EnabledTraceUtil enabledTraceUtil = (EnabledTraceUtil) firestore.getOptions().getTraceUtil();
140112
assertThat(enabledTraceUtil).isNotNull();
141113
assertThat(enabledTraceUtil.getOpenTelemetry()).isEqualTo(myOpenTelemetrySdk);
142-
}
143-
144-
@Test
145-
public void usesGlobalOpenTelemetryIfOpenTelemetryNotProvidedInOptions() {
146-
// Register a global OpenTelemetry SDK.
147-
OpenTelemetrySdk.builder().buildAndRegisterGlobal();
148-
149-
// Do _not_ pass it to FirestoreOptions.
150-
FirestoreOptions firestoreOptions =
151-
getBaseOptions()
152-
.setOpenTelemetryOptions(
153-
FirestoreOpenTelemetryOptions.newBuilder().setTracingEnabled(true).build())
154-
.build();
155-
firestore = firestoreOptions.getService();
156-
EnabledTraceUtil enabledTraceUtil = (EnabledTraceUtil) firestore.getOptions().getTraceUtil();
157-
assertThat(enabledTraceUtil).isNotNull();
158-
assertThat(enabledTraceUtil.getOpenTelemetry()).isEqualTo(GlobalOpenTelemetry.get());
114+
assertThat(enabledTraceUtil.getOpenTelemetry().getTracerProvider().equals(myTracerProvider));
159115
}
160116
}

‎google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITE2ETracingTest.java

+6-13
Original file line numberDiff line numberDiff line change
@@ -319,20 +319,13 @@ public void before() throws Exception {
319319
// Initialize the Firestore DB w/ the OTel SDK. Ideally we'd do this is the @BeforeAll method
320320
// but because gRPC traces need to be deterministically force-flushed, firestore.shutdown()
321321
// must be called in @After for each test.
322-
FirestoreOptions.Builder optionsBuilder;
323-
if (isUsingGlobalOpenTelemetrySDK()) {
324-
optionsBuilder =
325-
FirestoreOptions.newBuilder()
326-
.setOpenTelemetryOptions(
327-
FirestoreOpenTelemetryOptions.newBuilder().setTracingEnabled(true).build());
328-
} else {
322+
FirestoreOptions.Builder optionsBuilder = FirestoreOptions.newBuilder();
323+
if (!isUsingGlobalOpenTelemetrySDK()) {
329324
optionsBuilder =
330-
FirestoreOptions.newBuilder()
331-
.setOpenTelemetryOptions(
332-
FirestoreOpenTelemetryOptions.newBuilder()
333-
.setOpenTelemetry(openTelemetrySdk)
334-
.setTracingEnabled(true)
335-
.build());
325+
optionsBuilder.setOpenTelemetryOptions(
326+
FirestoreOpenTelemetryOptions.newBuilder()
327+
.setOpenTelemetry(openTelemetrySdk)
328+
.build());
336329
}
337330

338331
String namedDb = System.getProperty("FIRESTORE_NAMED_DATABASE");

‎google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITTracingTest.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -125,11 +125,11 @@ public void before() {
125125
if (isUsingGlobalOpenTelemetrySDK()) {
126126
GlobalOpenTelemetry.resetForTest();
127127
openTelemetrySdk = openTelemetrySdkBuilder.buildAndRegisterGlobal();
128-
optionsBuilder.setOpenTelemetryOptions(otelOptionsBuilder.setTracingEnabled(true).build());
128+
optionsBuilder.setOpenTelemetryOptions(otelOptionsBuilder.build());
129129
} else {
130130
openTelemetrySdk = openTelemetrySdkBuilder.build();
131131
optionsBuilder.setOpenTelemetryOptions(
132-
otelOptionsBuilder.setTracingEnabled(true).setOpenTelemetry(openTelemetrySdk).build());
132+
otelOptionsBuilder.setOpenTelemetry(openTelemetrySdk).build());
133133
}
134134

135135
String namedDb = System.getProperty("FIRESTORE_NAMED_DATABASE");

0 commit comments

Comments
 (0)