Skip to content

Commit 2fdd9d1

Browse files
committed
fix(api-tracing): handle corner case for historic
1 parent 2989175 commit 2fdd9d1

File tree

2 files changed

+62
-6
lines changed

2 files changed

+62
-6
lines changed

metadata-io/src/main/java/com/linkedin/metadata/trace/TraceServiceImpl.java

+15-6
Original file line numberDiff line numberDiff line change
@@ -331,24 +331,33 @@ private Map<Urn, LinkedHashMap<String, TraceStorageStatus>> traceSearchInParalle
331331
primaryStatuses.getOrDefault(urn, new LinkedHashMap<>());
332332

333333
for (String aspectName : entry.getValue()) {
334-
TraceWriteStatus status = primaryStatus.get(aspectName).getWriteStatus();
335-
if (status == TraceWriteStatus.PENDING) {
334+
TraceWriteStatus primaryStorageStatus = primaryStatus.get(aspectName).getWriteStatus();
335+
if (primaryStorageStatus == TraceWriteStatus.PENDING) {
336+
// If the primary storage write hasn't happened, then we don't expect the search write
336337
finalResponse.put(
337338
aspectName,
338339
TraceStorageStatus.ok(TraceWriteStatus.PENDING, "Pending primary storage write."));
339-
} else if (status == TraceWriteStatus.NO_OP) {
340+
} else if (primaryStorageStatus == TraceWriteStatus.NO_OP) {
340341
if (entitySpec.getAspectSpec(aspectName).isTimeseries()) {
341342
finalResponse.put(
342343
aspectName, TraceStorageStatus.ok(TraceWriteStatus.TRACE_NOT_IMPLEMENTED));
343344
} else {
345+
// If the primary write is a no-op, then the search write is as well
344346
finalResponse.put(aspectName, TraceStorageStatus.NO_OP);
345347
}
346-
} else if (status == TraceWriteStatus.ERROR) {
348+
} else if (primaryStorageStatus == TraceWriteStatus.HISTORIC_STATE) {
349+
// If primary storage is historic then we assume the search write should also be historic
350+
// If the search state hasn't been overwritten, then this "write" didn't fail
351+
finalResponse.put(aspectName, TraceStorageStatus.ok(TraceWriteStatus.HISTORIC_STATE));
352+
} else if (primaryStorageStatus == TraceWriteStatus.ERROR) {
353+
// If primary write fails, then search write never happened
347354
finalResponse.put(
348355
aspectName,
349356
TraceStorageStatus.fail(TraceWriteStatus.ERROR, "Primary storage write failed."));
350-
} else if (status == TraceWriteStatus.TRACE_NOT_IMPLEMENTED
351-
|| status == TraceWriteStatus.UNKNOWN) {
357+
} else if (primaryStorageStatus == TraceWriteStatus.TRACE_NOT_IMPLEMENTED
358+
|| primaryStorageStatus == TraceWriteStatus.UNKNOWN) {
359+
// If we don't know what happened with primary storage, then we can't know what should
360+
// have happend to search
352361
finalResponse.put(
353362
aspectName,
354363
TraceStorageStatus.ok(

metadata-io/src/test/java/com/linkedin/metadata/trace/TraceServiceImplTest.java

+47
Original file line numberDiff line numberDiff line change
@@ -392,4 +392,51 @@ public void testTraceWithNoOpState() throws Exception {
392392
assertEquals(status.getSearchStorage().getWriteStatus(), TraceWriteStatus.NO_OP);
393393
assertTrue(status.isSuccess());
394394
}
395+
396+
@Test
397+
public void testTraceHistoricStateSearchPropagation() throws Exception {
398+
// This test verifies that when primary storage has HISTORIC_STATE,
399+
// the search storage is also set to HISTORIC_STATE based on this line:
400+
// finalResponse.put(aspectName, TraceStorageStatus.ok(TraceWriteStatus.HISTORIC_STATE));
401+
402+
// Arrange - create request with one URN and one aspect
403+
Map<Urn, List<String>> aspectNames =
404+
Collections.singletonMap(TEST_URN, Collections.singletonList(ASPECT_NAME));
405+
406+
// Create a primary storage response with HISTORIC_STATE
407+
Map<String, TraceStorageStatus> primaryStatus = new LinkedHashMap<>();
408+
primaryStatus.put(ASPECT_NAME, TraceStorageStatus.ok(TraceWriteStatus.HISTORIC_STATE));
409+
410+
// Mock entityService to return empty to skip that branch
411+
when(entityService.getEntitiesV2(any(), anyString(), anySet(), anySet(), anyBoolean()))
412+
.thenReturn(Collections.emptyMap());
413+
414+
// Mock mcpTraceReader to return our primary status with HISTORIC_STATE
415+
when(mcpTraceReader.tracePendingStatuses(any(), eq(TEST_TRACE_ID), any(), anyBoolean()))
416+
.thenReturn(Collections.singletonMap(TEST_URN, primaryStatus));
417+
418+
// Mock systemMetadataService to return empty list to ensure
419+
// we don't get a pre-existing search status
420+
when(systemMetadataService.findAspectsByUrn(eq(TEST_URN), anyList(), eq(true)))
421+
.thenReturn(Collections.emptyList());
422+
423+
// Act
424+
Map<Urn, Map<String, TraceStatus>> result =
425+
traceService.trace(operationContext, TEST_TRACE_ID, aspectNames, false, false);
426+
427+
// Assert
428+
assertNotNull(result);
429+
assertTrue(result.containsKey(TEST_URN));
430+
Map<String, TraceStatus> urnStatus = result.get(TEST_URN);
431+
assertTrue(urnStatus.containsKey(ASPECT_NAME));
432+
433+
// The key assertion - if primary is HISTORIC_STATE, search should also be HISTORIC_STATE
434+
TraceStatus status = urnStatus.get(ASPECT_NAME);
435+
assertEquals(status.getPrimaryStorage().getWriteStatus(), TraceWriteStatus.HISTORIC_STATE);
436+
assertEquals(
437+
status.getSearchStorage().getWriteStatus(),
438+
TraceWriteStatus.HISTORIC_STATE,
439+
"When primary storage is HISTORIC_STATE, search storage should also be HISTORIC_STATE");
440+
assertTrue(status.isSuccess());
441+
}
395442
}

0 commit comments

Comments
 (0)