Skip to content

Commit

Permalink
Fix precommit errors
Browse files Browse the repository at this point in the history
  • Loading branch information
rohitsinha54 committed Jul 6, 2024
1 parent 2dd784b commit 67ef5bd
Show file tree
Hide file tree
Showing 24 changed files with 214 additions and 120 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -38,13 +38,14 @@ public static StringSetData create(Set<String> stringSet) {
return new AutoValue_StringSetData(stringSet);
}

/** Return a {@link EmptyStringSetData#INSTANCE} representing an empty {@link StringSetData}. */
/** Return a {@link EmptyStringSetData#INSTANCE} representing an empty {@link StringSetData}. */
public static StringSetData empty() {
return EmptyStringSetData.INSTANCE;
}

/** Combines this {@link StringSetData} with other, both original StringSetData are left
* intact. */
/**
* Combines this {@link StringSetData} with other, both original StringSetData are left intact.
*/
public StringSetData combine(StringSetData other) {
// do not merge other on this as this StringSetData might hold an immutable set like in case
// of EmptyStringSetData
Expand All @@ -66,13 +67,13 @@ public static class EmptyStringSetData extends StringSetData {

private EmptyStringSetData() {}

/** Return an immutable empty set. */
/** Return an immutable empty set. */
@Override
public Set<String> stringSet() {
return Collections.emptySet();
}

/** Return a {@link StringSetResult#empty()} which is immutable empty set. */
/** Return a {@link StringSetResult#empty()} which is immutable empty set. */
@Override
public StringSetResult extractResult() {
return StringSetResult.empty();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,8 @@ public class MetricsContainerStepMapTest {
Metrics.distribution(MetricsContainerStepMapTest.class, DISTRIBUTION_NAME);
private static final Gauge gauge = Metrics.gauge(MetricsContainerStepMapTest.class, GAUGE_NAME);

private static final StringSet stringSet = Metrics.stringSet(MetricsContainerStepMapTest.class, STRING_SET_NAME);
private static final StringSet stringSet =
Metrics.stringSet(MetricsContainerStepMapTest.class, STRING_SET_NAME);

private static final MetricsContainerImpl metricsContainer;

Expand Down Expand Up @@ -121,8 +122,12 @@ public void testAttemptedAccumulatedMetricResults() {
false);
assertGauge(GAUGE_NAME, step1res, STEP1, GaugeResult.create(VALUE, Instant.now()), false);

assertStringSet(STRING_SET_NAME, step1res, STEP1,
StringSetResult.create(ImmutableSet.of(FIRST_STRING, SECOND_STRING)), false);
assertStringSet(
STRING_SET_NAME,
step1res,
STEP1,
StringSetResult.create(ImmutableSet.of(FIRST_STRING, SECOND_STRING)),
false);

MetricQueryResults step2res =
metricResults.queryMetrics(MetricsFilter.builder().addStep(STEP2).build());
Expand All @@ -141,8 +146,12 @@ public void testAttemptedAccumulatedMetricResults() {
false);
assertGauge(GAUGE_NAME, step2res, STEP2, GaugeResult.create(VALUE, Instant.now()), false);

assertStringSet(STRING_SET_NAME, step2res, STEP2,
StringSetResult.create(ImmutableSet.of(FIRST_STRING, SECOND_STRING)), false);
assertStringSet(
STRING_SET_NAME,
step2res,
STEP2,
StringSetResult.create(ImmutableSet.of(FIRST_STRING, SECOND_STRING)),
false);

MetricQueryResults allres = metricResults.allMetrics();

Expand Down Expand Up @@ -302,8 +311,12 @@ public void testAttemptedAndCommittedAccumulatedMetricResults() {
DistributionResult.create(VALUE * 3, 2, VALUE, VALUE * 2),
true);
assertGauge(GAUGE_NAME, step1res, STEP1, GaugeResult.create(VALUE, Instant.now()), true);
assertStringSet(STRING_SET_NAME, step1res, STEP1,
StringSetResult.create(ImmutableSet.of(FIRST_STRING, SECOND_STRING)), false);
assertStringSet(
STRING_SET_NAME,
step1res,
STEP1,
StringSetResult.create(ImmutableSet.of(FIRST_STRING, SECOND_STRING)),
false);

MetricQueryResults step2res =
metricResults.queryMetrics(MetricsFilter.builder().addStep(STEP2).build());
Expand All @@ -321,8 +334,12 @@ public void testAttemptedAndCommittedAccumulatedMetricResults() {
DistributionResult.create(VALUE * 9, 6, VALUE, VALUE * 2),
false);
assertGauge(GAUGE_NAME, step2res, STEP2, GaugeResult.create(VALUE, Instant.now()), false);
assertStringSet(STRING_SET_NAME, step2res, STEP2,
StringSetResult.create(ImmutableSet.of(FIRST_STRING, SECOND_STRING)), false);
assertStringSet(
STRING_SET_NAME,
step2res,
STEP2,
StringSetResult.create(ImmutableSet.of(FIRST_STRING, SECOND_STRING)),
false);

assertCounter(COUNTER_NAME, step2res, STEP2, VALUE * 2, true);
assertDistribution(
Expand All @@ -332,8 +349,12 @@ public void testAttemptedAndCommittedAccumulatedMetricResults() {
DistributionResult.create(VALUE * 6, 4, VALUE, VALUE * 2),
true);
assertGauge(GAUGE_NAME, step2res, STEP2, GaugeResult.create(VALUE, Instant.now()), true);
assertStringSet(STRING_SET_NAME, step2res, STEP2,
StringSetResult.create(ImmutableSet.of(FIRST_STRING, SECOND_STRING)), true);
assertStringSet(
STRING_SET_NAME,
step2res,
STEP2,
StringSetResult.create(ImmutableSet.of(FIRST_STRING, SECOND_STRING)),
true);

MetricQueryResults allres = metricResults.queryMetrics(MetricsFilter.builder().build());

Expand Down Expand Up @@ -389,8 +410,12 @@ public void testReset() {
DistributionResult.create(VALUE * 3, 2, VALUE, VALUE * 2),
false);
assertGauge(GAUGE_NAME, allres, STEP1, GaugeResult.create(VALUE, Instant.now()), false);
assertStringSet(STRING_SET_NAME, allres, STEP1,
StringSetResult.create(ImmutableSet.of(FIRST_STRING, SECOND_STRING)), false);
assertStringSet(
STRING_SET_NAME,
allres,
STEP1,
StringSetResult.create(ImmutableSet.of(FIRST_STRING, SECOND_STRING)),
false);

assertCounter(COUNTER_NAME, allres, STEP2, VALUE * 2, false);
assertDistribution(
Expand All @@ -400,8 +425,12 @@ public void testReset() {
DistributionResult.create(VALUE * 6, 4, VALUE, VALUE * 2),
false);
assertGauge(GAUGE_NAME, allres, STEP2, GaugeResult.create(VALUE, Instant.now()), false);
assertStringSet(STRING_SET_NAME, allres, STEP2,
StringSetResult.create(ImmutableSet.of(FIRST_STRING, SECOND_STRING)), false);
assertStringSet(
STRING_SET_NAME,
allres,
STEP2,
StringSetResult.create(ImmutableSet.of(FIRST_STRING, SECOND_STRING)),
false);

attemptedMetrics.reset();
metricResults = asAttemptedOnlyMetricResults(attemptedMetrics);
Expand Down Expand Up @@ -458,6 +487,7 @@ private void assertGauge(
metricQueryResults.getGauges(),
hasItem(metricsResult(NAMESPACE, name, step, expected, isCommitted)));
}

private void assertStringSet(
String name,
MetricQueryResults metricQueryResults,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,6 @@ public void testInt64GaugeEncoding() {
assertEquals(data, decodeInt64Gauge(payload));
}


@Test
public void testStringSetEncoding() {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,19 +35,21 @@ public void testDeltaAndCumulative() {
cell.add("pubsub");
cell.add("bq", "spanner");
assertEquals(cell.getCumulative().stringSet(), ImmutableSet.of("spanner", "pubsub", "bq"));
assertEquals("getCumulative is idempotent", cell.getCumulative().stringSet(),
assertEquals(
"getCumulative is idempotent",
cell.getCumulative().stringSet(),
ImmutableSet.of("spanner", "pubsub", "bq"));

assertThat(cell.getDirty().beforeCommit(), equalTo(true));
cell.getDirty().afterCommit();
assertThat(cell.getDirty().beforeCommit(), equalTo(false));

cell.add("gcs");
assertEquals(cell.getCumulative().stringSet(),
ImmutableSet.of("spanner", "pubsub", "bq", "gcs"));
assertEquals(
cell.getCumulative().stringSet(), ImmutableSet.of("spanner", "pubsub", "bq", "gcs"));

assertThat("Adding a new value made the cell dirty", cell.getDirty().beforeCommit(),
equalTo(true));
assertThat(
"Adding a new value made the cell dirty", cell.getDirty().beforeCommit(), equalTo(true));
}

@Test
Expand Down Expand Up @@ -84,7 +86,8 @@ public void testReset() {
StringSetCell stringSetCell = new StringSetCell(MetricName.named("namespace", "name"));
stringSetCell.add("hello");
Assert.assertNotEquals(stringSetCell.getDirty(), new DirtyState());
assertThat(stringSetCell.getCumulative().stringSet(),
assertThat(
stringSetCell.getCumulative().stringSet(),
equalTo(StringSetData.create(ImmutableSet.of("hello")).stringSet()));

stringSetCell.reset();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@

/** Tests for {@link StringSetData}. */
public class StringSetDataTest {
@Rule
public ExpectedException exception = ExpectedException.none();
@Rule public ExpectedException exception = ExpectedException.none();

@Test
public void testCreate() {
// test empty stringset creation
Expand Down Expand Up @@ -67,6 +67,7 @@ public void testCombineWithEmpty() {
assertTrue(empty.stringSet().isEmpty());
assertEquals(multipleElement.stringSet(), ImmutableSet.of("cd", "ef"));
}

@Test
public void testEmpty() {
StringSetData empty = StringSetData.empty();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,7 @@ public StringSetData combine(Iterable<StringSetData> updates) {
}
return result;
}

@Override
public StringSetResult extract(StringSetData data) {
return data.extractResult();
Expand Down Expand Up @@ -275,12 +276,16 @@ public MetricQueryResults queryMetrics(@Nullable MetricsFilter filter) {
}

ImmutableList.Builder<MetricResult<StringSetResult>> stringSetResult = ImmutableList.builder();
for (Entry<MetricKey, DirectMetric<StringSetData, StringSetResult>> stringSet : stringSet.entries()) {
for (Entry<MetricKey, DirectMetric<StringSetData, StringSetResult>> stringSet :
stringSet.entries()) {
maybeExtractResult(filter, stringSetResult, stringSet);
}

return MetricQueryResults.create(
counterResults.build(), distributionResults.build(), gaugeResults.build(), stringSetResult.build());
counterResults.build(),
distributionResults.build(),
gaugeResults.build(),
stringSetResult.build());
}

private <ResultT> void maybeExtractResult(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,9 @@ public void testApplyCommittedNoFilter() {
ImmutableList.of(
MetricUpdate.create(MetricKey.create("step1", NAME4), GaugeData.create(15L))),
ImmutableList.of(
MetricUpdate.create(MetricKey.create("step1", NAME4), StringSetData.create(ImmutableSet.of("ab"))))));
MetricUpdate.create(
MetricKey.create("step1", NAME4),
StringSetData.create(ImmutableSet.of("ab"))))));
metrics.commitLogical(
bundle1,
MetricUpdates.create(
Expand All @@ -103,7 +105,9 @@ public void testApplyCommittedNoFilter() {
ImmutableList.of(
MetricUpdate.create(MetricKey.create("step1", NAME4), GaugeData.create(27L))),
ImmutableList.of(
MetricUpdate.create(MetricKey.create("step1", NAME4), StringSetData.create(ImmutableSet.of("cd"))))));
MetricUpdate.create(
MetricKey.create("step1", NAME4),
StringSetData.create(ImmutableSet.of("cd"))))));

MetricQueryResults results = metrics.allMetrics();
assertThat(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -163,10 +163,10 @@ public void addMetricResult(
Long value = getCounterValue(committed);
counterResults.add(MetricResult.create(metricKey, !isStreamingJob, value));
} else if (committed.getSet() != null && attempted.getSet() != null) {
// stringset metric
StringSetResult value = getStringSetValue(committed);
stringSetResults.add(MetricResult.create(metricKey, !isStreamingJob, value));
} else {
// stringset metric
StringSetResult value = getStringSetValue(committed);
stringSetResults.add(MetricResult.create(metricKey, !isStreamingJob, value));
} else {
// This is exceptionally unexpected. We expect matching user metrics to only have the
// value types provided by the Metrics API.
LOG.warn(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -257,11 +257,11 @@ public void testSingleStringSetUpdates() throws IOException {
// The parser relies on the fact that one tentative and one committed metric update exist in
// the job metrics results.
MetricUpdate mu1 =
makeStringSetMetricUpdate("counterName", "counterNamespace", "s2",
Arrays.asList("ab", "cd"), false);
makeStringSetMetricUpdate(
"counterName", "counterNamespace", "s2", Arrays.asList("ab", "cd"), false);
MetricUpdate mu1Tentative =
makeStringSetMetricUpdate("counterName", "counterNamespace", "s2",
Arrays.asList("ef", "gh"), true);
makeStringSetMetricUpdate(
"counterName", "counterNamespace", "s2", Arrays.asList("ef", "gh"), true);
jobMetrics.setMetrics(ImmutableList.of(mu1, mu1Tentative));
DataflowClient dataflowClient = mock(DataflowClient.class);
when(dataflowClient.getJobMetrics(JOB_ID)).thenReturn(jobMetrics);
Expand All @@ -271,15 +271,19 @@ public void testSingleStringSetUpdates() throws IOException {
assertThat(
result.getStringSets(),
containsInAnyOrder(
attemptedMetricsResult("counterNamespace", "counterName", "myStepName",
StringSetResult.create(
ImmutableSet.of("ab", "cd")))));
attemptedMetricsResult(
"counterNamespace",
"counterName",
"myStepName",
StringSetResult.create(ImmutableSet.of("ab", "cd")))));
assertThat(
result.getStringSets(),
containsInAnyOrder(
committedMetricsResult("counterNamespace", "counterName", "myStepName",
StringSetResult.create(
ImmutableSet.of("ef", "gh")))));
committedMetricsResult(
"counterNamespace",
"counterName",
"myStepName",
StringSetResult.create(ImmutableSet.of("ef", "gh")))));
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,8 +98,7 @@ public static CounterUpdate fromGauge(
.setIntegerGauge(integerGaugeProto);
}

public static CounterUpdate fromStringSet(
MetricKey key, StringSetData stringSetData) {
public static CounterUpdate fromStringSet(MetricKey key, StringSetData stringSetData) {
CounterStructuredNameAndMetadata name = structuredNameAndMetadata(key, Kind.SET);

StringList stringList = new StringList();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,9 @@ public Histogram getPerWorkerHistogram(
}

public Iterable<CounterUpdate> extractUpdates() {
return counterUpdates().append(distributionUpdates()).append(gaugeUpdates().append(stringSetUpdates()));
return counterUpdates()
.append(distributionUpdates())
.append(gaugeUpdates().append(stringSetUpdates()));
}

private FluentIterable<CounterUpdate> counterUpdates() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -168,9 +168,10 @@ public void extractMetricUpdatesStringSet() {
DataflowOperationContext operationContext =
executionContext.createOperationContext(NameContextsForTests.nameContextForTest());

StringSet stringSet = operationContext
.metricsContainer()
.getStringSet(MetricName.named("namespace", "some-stringset"));
StringSet stringSet =
operationContext
.metricsContainer()
.getStringSet(MetricName.named("namespace", "some-stringset"));
stringSet.add("ab");
stringSet.add("cd");

Expand Down
Loading

0 comments on commit 67ef5bd

Please sign in to comment.