Skip to content
This repository was archived by the owner on Dec 23, 2023. It is now read-only.

Commit 4475dd9

Browse files
songy23songy23
authored
Exporter/Stackdriver: Fail fast on invalid project ID. (#1846)
* Exporter/Stackdriver: Fail fast on invalid project ID. Also require a few configurations to be non-null. * Handle null project ID in SD trace exporter. Fixes #1547.
1 parent 9b2ac2b commit 4475dd9

8 files changed

Lines changed: 268 additions & 71 deletions

File tree

‎exporters/stats/stackdriver/src/main/java/io/opencensus/exporter/stats/stackdriver/StackdriverStatsConfiguration.java‎

Lines changed: 34 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,14 @@
2121
import com.google.api.MonitoredResource;
2222
import com.google.auth.Credentials;
2323
import com.google.auto.value.AutoValue;
24+
import com.google.cloud.ServiceOptions;
25+
import com.google.common.base.Preconditions;
26+
import com.google.common.base.Strings;
2427
import io.opencensus.common.Duration;
2528
import io.opencensus.metrics.LabelKey;
2629
import io.opencensus.metrics.LabelValue;
30+
import java.util.Collections;
31+
import java.util.LinkedHashMap;
2732
import java.util.Map;
2833
import javax.annotation.Nullable;
2934
import javax.annotation.concurrent.Immutable;
@@ -37,6 +42,11 @@
3742
@Immutable
3843
public abstract class StackdriverStatsConfiguration {
3944

45+
static final Duration DEFAULT_INTERVAL = Duration.create(60, 0);
46+
static final MonitoredResource DEFAULT_RESOURCE = StackdriverExportUtils.getDefaultResource();
47+
static final String DEFAULT_PROJECT_ID =
48+
Strings.nullToEmpty(ServiceOptions.getDefaultProjectId());
49+
4050
StackdriverStatsConfiguration() {}
4151

4252
/**
@@ -54,7 +64,6 @@ public abstract class StackdriverStatsConfiguration {
5464
* @return the project id.
5565
* @since 0.11
5666
*/
57-
@Nullable
5867
public abstract String getProjectId();
5968

6069
/**
@@ -63,7 +72,6 @@ public abstract class StackdriverStatsConfiguration {
6372
* @return the export interval.
6473
* @since 0.11
6574
*/
66-
@Nullable
6775
public abstract Duration getExportInterval();
6876

6977
/**
@@ -72,7 +80,6 @@ public abstract class StackdriverStatsConfiguration {
7280
* @return the {@code MonitoredResource}.
7381
* @since 0.11
7482
*/
75-
@Nullable
7683
public abstract MonitoredResource getMonitoredResource();
7784

7885
/**
@@ -100,7 +107,10 @@ public abstract class StackdriverStatsConfiguration {
100107
*/
101108
public static Builder builder() {
102109
return new AutoValue_StackdriverStatsConfiguration.Builder()
103-
.setConstantLabels(DEFAULT_CONSTANT_LABELS);
110+
.setProjectId(DEFAULT_PROJECT_ID)
111+
.setConstantLabels(DEFAULT_CONSTANT_LABELS)
112+
.setExportInterval(DEFAULT_INTERVAL)
113+
.setMonitoredResource(DEFAULT_RESOURCE);
104114
}
105115

106116
/**
@@ -182,12 +192,31 @@ public abstract static class Builder {
182192
*/
183193
public abstract Builder setConstantLabels(Map<LabelKey, LabelValue> constantLabels);
184194

195+
abstract String getProjectId();
196+
197+
abstract Map<LabelKey, LabelValue> getConstantLabels();
198+
199+
abstract StackdriverStatsConfiguration autoBuild();
200+
185201
/**
186202
* Builds a new {@link StackdriverStatsConfiguration} with current settings.
187203
*
188204
* @return a {@code StackdriverStatsConfiguration}.
189205
* @since 0.11
190206
*/
191-
public abstract StackdriverStatsConfiguration build();
207+
public StackdriverStatsConfiguration build() {
208+
// Make a defensive copy of constant labels.
209+
setConstantLabels(
210+
Collections.unmodifiableMap(
211+
new LinkedHashMap<LabelKey, LabelValue>(getConstantLabels())));
212+
Preconditions.checkArgument(
213+
!Strings.isNullOrEmpty(getProjectId()),
214+
"Cannot find a project ID from either configurations or application default.");
215+
for (Map.Entry<LabelKey, LabelValue> constantLabel : getConstantLabels().entrySet()) {
216+
Preconditions.checkNotNull(constantLabel.getKey(), "constant label key");
217+
Preconditions.checkNotNull(constantLabel.getValue(), "constant label value");
218+
}
219+
return autoBuild();
220+
}
192221
}
193222
}

‎exporters/stats/stackdriver/src/main/java/io/opencensus/exporter/stats/stackdriver/StackdriverStatsExporter.java‎

Lines changed: 22 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,12 @@
1616

1717
package io.opencensus.exporter.stats.stackdriver;
1818

19+
import static com.google.common.base.Preconditions.checkArgument;
1920
import static com.google.common.base.Preconditions.checkNotNull;
2021
import static com.google.common.base.Preconditions.checkState;
2122
import static io.opencensus.exporter.stats.stackdriver.StackdriverExportUtils.DEFAULT_CONSTANT_LABELS;
23+
import static io.opencensus.exporter.stats.stackdriver.StackdriverStatsConfiguration.DEFAULT_PROJECT_ID;
24+
import static io.opencensus.exporter.stats.stackdriver.StackdriverStatsConfiguration.DEFAULT_RESOURCE;
2225

2326
import com.google.api.MonitoredResource;
2427
import com.google.api.gax.core.FixedCredentialsProvider;
@@ -78,10 +81,6 @@ public final class StackdriverStatsExporter {
7881
"opencensus-java/" + OpenCensusLibraryInformation.VERSION;
7982
private static final HeaderProvider OPENCENSUS_USER_AGENT_HEADER_PROVIDER =
8083
FixedHeaderProvider.create(USER_AGENT_KEY, USER_AGENT);
81-
@VisibleForTesting static final Duration DEFAULT_INTERVAL = Duration.create(60, 0);
82-
83-
private static final MonitoredResource DEFAULT_RESOURCE =
84-
StackdriverExportUtils.getDefaultResource();
8584

8685
private final IntervalMetricReader intervalMetricReader;
8786

@@ -94,9 +93,7 @@ private StackdriverStatsExporter(
9493
Map<LabelKey, LabelValue> constantLabels) {
9594
IntervalMetricReader.Options.Builder intervalMetricReaderOptionsBuilder =
9695
IntervalMetricReader.Options.builder();
97-
if (exportInterval != null) {
98-
intervalMetricReaderOptionsBuilder.setExportInterval(exportInterval);
99-
}
96+
intervalMetricReaderOptionsBuilder.setExportInterval(exportInterval);
10097
intervalMetricReader =
10198
IntervalMetricReader.create(
10299
new CreateMetricDescriptorExporter(
@@ -138,7 +135,8 @@ public static void createAndRegisterWithCredentialsAndProjectId(
138135
checkNotNull(credentials, "credentials");
139136
checkNotNull(projectId, "projectId");
140137
checkNotNull(exportInterval, "exportInterval");
141-
createInternal(credentials, projectId, exportInterval, null, null, DEFAULT_CONSTANT_LABELS);
138+
createInternal(
139+
credentials, projectId, exportInterval, DEFAULT_RESOURCE, null, DEFAULT_CONSTANT_LABELS);
142140
}
143141

144142
/**
@@ -168,7 +166,8 @@ public static void createAndRegisterWithProjectId(String projectId, Duration exp
168166
throws IOException {
169167
checkNotNull(projectId, "projectId");
170168
checkNotNull(exportInterval, "exportInterval");
171-
createInternal(null, projectId, exportInterval, null, null, DEFAULT_CONSTANT_LABELS);
169+
createInternal(
170+
null, projectId, exportInterval, DEFAULT_RESOURCE, null, DEFAULT_CONSTANT_LABELS);
172171
}
173172

174173
/**
@@ -237,7 +236,7 @@ public static void createAndRegister(StackdriverStatsConfiguration configuration
237236
* @since 0.11.0
238237
*/
239238
public static void createAndRegister() throws IOException {
240-
createInternal(null, null, null, null, null, DEFAULT_CONSTANT_LABELS);
239+
createAndRegister(StackdriverStatsConfiguration.builder().build());
241240
}
242241

243242
/**
@@ -264,7 +263,10 @@ public static void createAndRegister() throws IOException {
264263
@Deprecated
265264
public static void createAndRegister(Duration exportInterval) throws IOException {
266265
checkNotNull(exportInterval, "exportInterval");
267-
createInternal(null, null, exportInterval, null, null, DEFAULT_CONSTANT_LABELS);
266+
checkArgument(
267+
!DEFAULT_PROJECT_ID.isEmpty(), "Cannot find a project ID from application default.");
268+
createInternal(
269+
null, DEFAULT_PROJECT_ID, exportInterval, DEFAULT_RESOURCE, null, DEFAULT_CONSTANT_LABELS);
268270
}
269271

270272
/**
@@ -321,42 +323,34 @@ public static void createAndRegisterWithMonitoredResource(
321323
Duration exportInterval, MonitoredResource monitoredResource) throws IOException {
322324
checkNotNull(exportInterval, "exportInterval");
323325
checkNotNull(monitoredResource, "monitoredResource");
324-
createInternal(null, null, exportInterval, monitoredResource, null, DEFAULT_CONSTANT_LABELS);
326+
checkArgument(
327+
!DEFAULT_PROJECT_ID.isEmpty(), "Cannot find a project ID from application default.");
328+
createInternal(
329+
null, DEFAULT_PROJECT_ID, exportInterval, monitoredResource, null, DEFAULT_CONSTANT_LABELS);
325330
}
326331

327332
// Use createInternal() (instead of constructor) to enforce singleton.
328333
private static void createInternal(
329334
@Nullable Credentials credentials,
330-
@Nullable String projectId,
331-
@Nullable Duration exportInterval,
332-
@Nullable MonitoredResource monitoredResource,
335+
String projectId,
336+
Duration exportInterval,
337+
MonitoredResource monitoredResource,
333338
@Nullable String metricNamePrefix,
334339
Map<LabelKey, LabelValue> constantLabels)
335340
throws IOException {
336-
projectId =
337-
projectId == null
338-
// TODO(sebright): Handle null default project ID.
339-
? castNonNull(ServiceOptions.getDefaultProjectId())
340-
: projectId;
341341
synchronized (monitor) {
342342
checkState(instance == null, "Stackdriver stats exporter is already created.");
343343
instance =
344344
new StackdriverStatsExporter(
345345
projectId,
346346
createMetricServiceClient(credentials),
347-
exportInterval == null ? DEFAULT_INTERVAL : exportInterval,
348-
monitoredResource == null ? DEFAULT_RESOURCE : monitoredResource,
347+
exportInterval,
348+
monitoredResource,
349349
metricNamePrefix,
350350
constantLabels);
351351
}
352352
}
353353

354-
// TODO(sebright): Remove this method.
355-
@SuppressWarnings("nullness")
356-
private static <T> T castNonNull(@javax.annotation.Nullable T arg) {
357-
return arg;
358-
}
359-
360354
// Initialize MetricServiceClient inside lock to avoid creating multiple clients.
361355
@GuardedBy("monitor")
362356
@VisibleForTesting

‎exporters/stats/stackdriver/src/test/java/io/opencensus/exporter/stats/stackdriver/StackdriverStatsConfigurationTest.java‎

Lines changed: 100 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,17 +18,22 @@
1818

1919
import static com.google.common.truth.Truth.assertThat;
2020
import static io.opencensus.exporter.stats.stackdriver.StackdriverExportUtils.DEFAULT_CONSTANT_LABELS;
21+
import static io.opencensus.exporter.stats.stackdriver.StackdriverStatsConfiguration.DEFAULT_INTERVAL;
2122

2223
import com.google.api.MonitoredResource;
2324
import com.google.auth.Credentials;
2425
import com.google.auth.oauth2.AccessToken;
2526
import com.google.auth.oauth2.GoogleCredentials;
27+
import com.google.cloud.ServiceOptions;
2628
import io.opencensus.common.Duration;
2729
import io.opencensus.metrics.LabelKey;
2830
import io.opencensus.metrics.LabelValue;
2931
import java.util.Collections;
3032
import java.util.Date;
33+
import java.util.Map;
34+
import org.junit.Rule;
3135
import org.junit.Test;
36+
import org.junit.rules.ExpectedException;
3237
import org.junit.runner.RunWith;
3338
import org.junit.runners.JUnit4;
3439

@@ -47,6 +52,13 @@ public class StackdriverStatsConfigurationTest {
4752
.build();
4853
private static final String CUSTOM_PREFIX = "myorg";
4954

55+
@Rule public final ExpectedException thrown = ExpectedException.none();
56+
57+
@Test
58+
public void testConstants() {
59+
assertThat(DEFAULT_INTERVAL).isEqualTo(Duration.create(60, 0));
60+
}
61+
5062
@Test
5163
public void testBuild() {
5264
StackdriverStatsConfiguration configuration =
@@ -68,12 +80,96 @@ public void testBuild() {
6880

6981
@Test
7082
public void testBuild_Default() {
71-
StackdriverStatsConfiguration configuration = StackdriverStatsConfiguration.builder().build();
83+
StackdriverStatsConfiguration configuration;
84+
try {
85+
configuration = StackdriverStatsConfiguration.builder().build();
86+
} catch (Exception e) {
87+
// Some test hosts may not have cloud project ID set up.
88+
configuration = StackdriverStatsConfiguration.builder().setProjectId("test").build();
89+
}
7290
assertThat(configuration.getCredentials()).isNull();
73-
assertThat(configuration.getProjectId()).isNull();
74-
assertThat(configuration.getExportInterval()).isNull();
75-
assertThat(configuration.getMonitoredResource()).isNull();
91+
assertThat(configuration.getProjectId()).isNotNull();
92+
assertThat(configuration.getExportInterval()).isEqualTo(DEFAULT_INTERVAL);
93+
assertThat(configuration.getMonitoredResource()).isNotNull();
7694
assertThat(configuration.getMetricNamePrefix()).isNull();
7795
assertThat(configuration.getConstantLabels()).isEqualTo(DEFAULT_CONSTANT_LABELS);
7896
}
97+
98+
@Test
99+
public void disallowNullProjectId() {
100+
StackdriverStatsConfiguration.Builder builder = StackdriverStatsConfiguration.builder();
101+
thrown.expect(NullPointerException.class);
102+
builder.setProjectId(null);
103+
}
104+
105+
@Test
106+
public void disallowEmptyProjectId() {
107+
StackdriverStatsConfiguration.Builder builder = StackdriverStatsConfiguration.builder();
108+
builder.setProjectId("");
109+
thrown.expect(IllegalArgumentException.class);
110+
builder.build();
111+
}
112+
113+
@Test
114+
public void allowToUseDefaultProjectId() {
115+
String defaultProjectId = ServiceOptions.getDefaultProjectId();
116+
if (defaultProjectId != null) {
117+
StackdriverStatsConfiguration configuration = StackdriverStatsConfiguration.builder().build();
118+
assertThat(configuration.getProjectId()).isEqualTo(defaultProjectId);
119+
}
120+
}
121+
122+
@Test
123+
public void disallowNullResource() {
124+
StackdriverStatsConfiguration.Builder builder = StackdriverStatsConfiguration.builder();
125+
thrown.expect(NullPointerException.class);
126+
builder.setMonitoredResource(null);
127+
}
128+
129+
@Test
130+
public void disallowNullExportInterval() {
131+
StackdriverStatsConfiguration.Builder builder = StackdriverStatsConfiguration.builder();
132+
thrown.expect(NullPointerException.class);
133+
builder.setExportInterval(null);
134+
}
135+
136+
@Test
137+
public void disallowNullConstantLabels() {
138+
StackdriverStatsConfiguration.Builder builder = StackdriverStatsConfiguration.builder();
139+
thrown.expect(NullPointerException.class);
140+
builder.setConstantLabels(null);
141+
}
142+
143+
@Test
144+
public void disallowNullConstantLabelKey() {
145+
StackdriverStatsConfiguration.Builder builder = StackdriverStatsConfiguration.builder();
146+
Map<LabelKey, LabelValue> labels = Collections.singletonMap(null, LabelValue.create("val"));
147+
builder.setConstantLabels(labels);
148+
thrown.expect(NullPointerException.class);
149+
builder.build();
150+
}
151+
152+
@Test
153+
public void disallowNullConstantLabelValue() {
154+
StackdriverStatsConfiguration.Builder builder = StackdriverStatsConfiguration.builder();
155+
Map<LabelKey, LabelValue> labels =
156+
Collections.singletonMap(LabelKey.create("key", "desc"), null);
157+
builder.setConstantLabels(labels);
158+
thrown.expect(NullPointerException.class);
159+
builder.build();
160+
}
161+
162+
@Test
163+
public void allowNullCredentials() {
164+
StackdriverStatsConfiguration configuration =
165+
StackdriverStatsConfiguration.builder().setCredentials(null).build();
166+
assertThat(configuration.getCredentials()).isNull();
167+
}
168+
169+
@Test
170+
public void allowNullMetricPrefix() {
171+
StackdriverStatsConfiguration configuration =
172+
StackdriverStatsConfiguration.builder().setMetricNamePrefix(null).build();
173+
assertThat(configuration.getMetricNamePrefix()).isNull();
174+
}
79175
}

‎exporters/stats/stackdriver/src/test/java/io/opencensus/exporter/stats/stackdriver/StackdriverStatsExporterTest.java‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -50,11 +50,6 @@ public class StackdriverStatsExporterTest {
5050

5151
@Rule public final ExpectedException thrown = ExpectedException.none();
5252

53-
@Test
54-
public void testConstants() {
55-
assertThat(StackdriverStatsExporter.DEFAULT_INTERVAL).isEqualTo(Duration.create(60, 0));
56-
}
57-
5853
@Test
5954
public void createWithNullStackdriverStatsConfiguration() throws IOException {
6055
thrown.expect(NullPointerException.class);

0 commit comments

Comments
 (0)