Skip to content

Commit a8086f4

Browse files
committed
address review commetns
1 parent 2818990 commit a8086f4

1 file changed

Lines changed: 56 additions & 2 deletions

File tree

clients/da-vinci-client/src/test/java/com/linkedin/davinci/stats/AggVersionedDaVinciRecordTransformerStatsOtelTest.java

Lines changed: 56 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,9 @@
1919
import com.linkedin.venice.utils.OpenTelemetryDataTestUtils;
2020
import io.opentelemetry.api.common.Attributes;
2121
import io.opentelemetry.sdk.testing.exporter.InMemoryMetricReader;
22+
import io.tehuti.metrics.MetricConfig;
2223
import io.tehuti.metrics.MetricsRepository;
24+
import io.tehuti.metrics.stats.AsyncGauge;
2325
import java.util.Collections;
2426
import org.testng.annotations.AfterMethod;
2527
import org.testng.annotations.BeforeMethod;
@@ -34,21 +36,28 @@ public class AggVersionedDaVinciRecordTransformerStatsOtelTest {
3436
private InMemoryMetricReader inMemoryMetricReader;
3537
private VeniceMetricsRepository metricsRepository;
3638
private AggVersionedDaVinciRecordTransformerStats aggStats;
39+
// Dedicated executor avoids shutting down Tehuti's static DEFAULT_ASYNC_GAUGE_EXECUTOR singleton when this
40+
// repository is closed in tearDown(). Closing the static executor would break AsyncGauge measurements
41+
// for any subsequent test running in the same JVM.
42+
private AsyncGauge.AsyncGaugeExecutor asyncGaugeExecutor;
3743

3844
@BeforeMethod
3945
public void setUp() {
4046
inMemoryMetricReader = InMemoryMetricReader.create();
47+
asyncGaugeExecutor = new AsyncGauge.AsyncGaugeExecutor.Builder().build();
4148
metricsRepository = new VeniceMetricsRepository(
4249
new VeniceMetricsConfig.Builder().setMetricPrefix(TEST_METRIC_PREFIX)
4350
.setMetricEntities(SERVER_METRIC_ENTITIES)
4451
.setEmitOtelMetrics(true)
4552
.setOtelAdditionalMetricsReader(inMemoryMetricReader)
53+
.setTehutiMetricConfig(new MetricConfig(asyncGaugeExecutor))
4654
.build());
4755
aggStats = createAggStats(metricsRepository);
4856
}
4957

5058
@AfterMethod
5159
public void tearDown() {
60+
// Closes only the dedicated executor; the JVM-wide static executor stays alive for other tests.
5261
if (metricsRepository != null) {
5362
metricsRepository.close();
5463
}
@@ -136,12 +145,53 @@ public void testOperationDimensionIsolation() {
136145
TEST_METRIC_PREFIX);
137146
}
138147

148+
@Test
149+
public void testMultiStoreIsolation() {
150+
String storeA = "store-a";
151+
String storeB = "store-b";
152+
long timestamp = System.currentTimeMillis();
153+
aggStats.recordPutError(storeA, 1, timestamp);
154+
aggStats.recordPutError(storeA, 1, timestamp);
155+
aggStats.recordPutError(storeB, 1, timestamp);
156+
157+
OpenTelemetryDataTestUtils.validateLongPointDataFromCounter(
158+
inMemoryMetricReader,
159+
2,
160+
buildAttributes(storeA, VeniceRecordTransformerOperation.PUT),
161+
RECORD_TRANSFORMER_ERROR_COUNT.getMetricEntity().getMetricName(),
162+
TEST_METRIC_PREFIX);
163+
164+
OpenTelemetryDataTestUtils.validateLongPointDataFromCounter(
165+
inMemoryMetricReader,
166+
1,
167+
buildAttributes(storeB, VeniceRecordTransformerOperation.PUT),
168+
RECORD_TRANSFORMER_ERROR_COUNT.getMetricEntity().getMetricName(),
169+
TEST_METRIC_PREFIX);
170+
}
171+
172+
@Test
173+
public void testHandleStoreDeletedClearsPerStoreEntries() {
174+
long timestamp = System.currentTimeMillis();
175+
aggStats.recordPutLatency(TEST_STORE_NAME, 1, 10.0, timestamp);
176+
aggStats.recordPutError(TEST_STORE_NAME, 1, timestamp);
177+
178+
aggStats.handleStoreDeleted(TEST_STORE_NAME);
179+
180+
// Recording after deletion must not NPE; per-store entries should be re-created lazily.
181+
aggStats.recordPutLatency(TEST_STORE_NAME, 1, 25.0, timestamp);
182+
aggStats.recordPutError(TEST_STORE_NAME, 1, timestamp);
183+
}
184+
139185
// --- NPE prevention tests ---
140186

141187
@Test
142188
public void testNoNpeWhenOtelDisabled() {
189+
AsyncGauge.AsyncGaugeExecutor localExecutor = new AsyncGauge.AsyncGaugeExecutor.Builder().build();
143190
try (VeniceMetricsRepository disabledRepo = new VeniceMetricsRepository(
144-
new VeniceMetricsConfig.Builder().setMetricPrefix(TEST_METRIC_PREFIX).setEmitOtelMetrics(false).build())) {
191+
new VeniceMetricsConfig.Builder().setMetricPrefix(TEST_METRIC_PREFIX)
192+
.setEmitOtelMetrics(false)
193+
.setTehutiMetricConfig(new MetricConfig(localExecutor))
194+
.build())) {
145195
exerciseAllRecordingPaths(disabledRepo);
146196
}
147197
}
@@ -178,9 +228,13 @@ private static AggVersionedDaVinciRecordTransformerStats createAggStats(MetricsR
178228
}
179229

180230
private Attributes buildAttributes(VeniceRecordTransformerOperation operation) {
231+
return buildAttributes(TEST_STORE_NAME, operation);
232+
}
233+
234+
private Attributes buildAttributes(String storeName, VeniceRecordTransformerOperation operation) {
181235
return Attributes.builder()
182236
.put(VENICE_CLUSTER_NAME.getDimensionNameInDefaultFormat(), TEST_CLUSTER_NAME)
183-
.put(VENICE_STORE_NAME.getDimensionNameInDefaultFormat(), TEST_STORE_NAME)
237+
.put(VENICE_STORE_NAME.getDimensionNameInDefaultFormat(), storeName)
184238
.put(VENICE_RECORD_TRANSFORMER_OPERATION.getDimensionNameInDefaultFormat(), operation.getDimensionValue())
185239
.build();
186240
}

0 commit comments

Comments
 (0)