Skip to content

fix(endpoint-enricher): add service name to API entity #251

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Aug 23, 2021
Merged

fix(endpoint-enricher): add service name to API entity #251

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion ...rc/main/java/org/hypertrace/traceenricher/enrichment/enrichers/endpoint/ApiEntityDao.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ public class ApiEntityDao {

private static final String SERVICE_ID_ATTR =
EntityConstants.getValue(ServiceAttribute.SERVICE_ATTRIBUTE_ID);
private static final String SERVICE_NAME_ATTR =
EntityConstants.getValue(ServiceAttribute.SERVICE_ATTRIBUTE_NAME);
private static final String API_URL_PATTERN_ATTR =
EntityConstants.getValue(ApiAttribute.API_ATTRIBUTE_URL_PATTERN);
private static final String API_TYPE_ATTR =
Expand All @@ -45,10 +47,12 @@ public ApiEntityDao(EdsClient client) {
edsClient = client;
}

public Entity upsertApiEntity(String tenantId, String serviceId, String apiType, String apiName) {
public Entity upsertApiEntity(
String tenantId, String serviceId, String serviceName, String apiType, String apiName) {

Preconditions.checkNotNull(tenantId, "tenantId can't be empty");
Preconditions.checkNotNull(serviceId, "serviceId can't be empty");
Preconditions.checkNotNull(serviceName, "serviceName can't be empty");
Preconditions.checkNotNull(apiType, "apiType can't be empty");
Preconditions.checkNotNull(apiName, "apiName can't be empty");

Expand All @@ -62,6 +66,7 @@ public Entity upsertApiEntity(String tenantId, String serviceId, String apiType,
.putIdentifyingAttributes(API_URL_PATTERN_ATTR, createAttributeValue(pattern))
.putIdentifyingAttributes(SERVICE_ID_ATTR, createAttributeValue(serviceId))
.putIdentifyingAttributes(API_TYPE_ATTR, createAttributeValue(apiType))
.putAttributes(SERVICE_NAME_ATTR, createAttributeValue(serviceName))
.putAttributes(API_DISCOVERY_FROM_ATTR, createAttributeValue(DISCOVERED_FROM))
.putAttributes(API_DISCOVERY_STATE_ATTR, createAttributeValue(DISCOVERED_STATE))
.putAttributes(API_NAME_ATTR, createAttributeValue(apiName));
Expand Down
10 changes: 7 additions & 3 deletions ...ain/java/org/hypertrace/traceenricher/enrichment/enrichers/endpoint/EndpointEnricher.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ public void enrichEvent(StructuredTrace trace, Event event) {
// Lookup the service id from the span. If we can't find a service id, we can't really
// associate API details with that span.
String serviceId = EnrichedSpanUtils.getServiceId(event);
String serviceName = EnrichedSpanUtils.getServiceName(event);
String customerId = trace.getCustomerId();
if (serviceId == null) {
LOGGER.warn(
Expand All @@ -84,7 +85,8 @@ public void enrichEvent(StructuredTrace trace, Event event) {
Entity apiEntity = null;
try {
apiEntity =
getOperationNameBasedEndpointDiscoverer(customerId, serviceId).getApiEntity(event);
getOperationNameBasedEndpointDiscoverer(customerId, serviceId, serviceName)
.getApiEntity(event);
} catch (Exception e) {
LOGGER.error(
"Unable to get apiEntity for tenantId {}, serviceId {} and event {}",
Expand Down Expand Up @@ -142,10 +144,12 @@ void setApiEntityDao(ApiEntityDao apiEntityDao) {
}

private OperationNameBasedEndpointDiscoverer getOperationNameBasedEndpointDiscoverer(
String customerId, String serviceId) {
String customerId, String serviceId, String serviceName) {
serviceIdToEndpointDiscoverer.computeIfAbsent(
serviceId,
e -> new OperationNameBasedEndpointDiscoverer(customerId, serviceId, apiEntityDao));
e ->
new OperationNameBasedEndpointDiscoverer(
customerId, serviceId, serviceName, apiEntityDao));
return serviceIdToEndpointDiscoverer.get(serviceId);
}

Expand Down
7 changes: 5 additions & 2 deletions ...ace/traceenricher/enrichment/enrichers/endpoint/OperationNameBasedEndpointDiscoverer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ public class OperationNameBasedEndpointDiscoverer {

private final String customerId;
private final String serviceId;
private final String serviceName;
private final ApiEntityDao apiEntityDao;

private final LoadingCache<String, Entity> patternToApiEntityCache =
Expand All @@ -32,9 +33,10 @@ public Entity load(@Nonnull String pattern) {
});

public OperationNameBasedEndpointDiscoverer(
String customerId, String serviceId, ApiEntityDao apiEntityDao) {
String customerId, String serviceId, String serviceName, ApiEntityDao apiEntityDao) {
this.customerId = customerId;
this.serviceId = serviceId;
this.serviceName = serviceName;
this.apiEntityDao = apiEntityDao;
}

Expand All @@ -44,7 +46,8 @@ public Entity getApiEntity(Event spanEvent) throws ExecutionException {
}

private Entity getEntityForPattern(String pattern) {
return apiEntityDao.upsertApiEntity(customerId, serviceId, ApiEntityDao.API_TYPE, pattern);
return apiEntityDao.upsertApiEntity(
customerId, serviceId, serviceName, ApiEntityDao.API_TYPE, pattern);
}

@VisibleForTesting
Expand Down
31 changes: 20 additions & 11 deletions ...est/java/org/hypertrace/traceenricher/enrichment/enrichers/endpoint/ApiEntityDaoTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,44 +9,53 @@
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;

public class ApiEntityDaoTest {
class ApiEntityDaoTest {

private EdsClient edsClient;
private ApiEntityDao underTest;
private ApiEntityDao apiEntityDao;

@BeforeEach
public void setup() {
void setup() {
edsClient = mock(EdsClient.class);
underTest = new ApiEntityDao(edsClient);
apiEntityDao = new ApiEntityDao(edsClient);
}

@Test
public void whenValidDataIsPassedExpectDaoToBeCalled() {
underTest.upsertApiEntity("tenant-1", "service-1", "OPERATION_NAME", "Driver:;getCustomers");
void whenValidDataIsPassedExpectDaoToBeCalled() {
apiEntityDao.upsertApiEntity(
"tenant-1", "service-1", "service1", "OPERATION_NAME", "Driver:;getCustomers");
verify(edsClient).upsert(any(org.hypertrace.entity.data.service.v1.Entity.class));
}

@Test
public void whenParamsAreNullThenExpectNullPointerException() {
void whenParamsAreNullThenExpectNullPointerException() {
Assertions.assertThrows(
NullPointerException.class,
() -> {
underTest.upsertApiEntity(null, "service-1", "OPERATION_NAME", "Driver:;getCustomers");
apiEntityDao.upsertApiEntity(
null, "service-1", "service1", "OPERATION_NAME", "Driver:;getCustomers");
});
Assertions.assertThrows(
NullPointerException.class,
() -> {
underTest.upsertApiEntity("tenant-1", null, "OPERATION_NAME", "Driver:;getCustomers");
apiEntityDao.upsertApiEntity(
"tenant-1", null, null, "OPERATION_NAME", "Driver:;getCustomers");
});
Assertions.assertThrows(
NullPointerException.class,
() -> {
underTest.upsertApiEntity("tenant-1", "service-1", null, "Driver:;getCustomers");
apiEntityDao.upsertApiEntity("tenant-1", "service-1", null, null, "Driver:;getCustomers");
});
Assertions.assertThrows(
NullPointerException.class,
() -> {
underTest.upsertApiEntity("tenant-1", "service-1", "OPERATION_NAME", null);
apiEntityDao.upsertApiEntity(
"tenant-1", "service-1", "service1", null, "Driver:;getCustomers");
});
Assertions.assertThrows(
NullPointerException.class,
() -> {
apiEntityDao.upsertApiEntity("tenant-1", "service-1", "service1", "OPERATION_NAME", null);
});
}
}
18 changes: 11 additions & 7 deletions ...java/org/hypertrace/traceenricher/enrichment/enrichers/endpoint/EndpointEnricherTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;

public class EndpointEnricherTest extends AbstractAttributeEnricherTest {
class EndpointEnricherTest extends AbstractAttributeEnricherTest {

private static final String API_NAME_ATTR =
EntityConstants.getValue(ApiAttribute.API_ATTRIBUTE_NAME);
Expand All @@ -51,14 +51,14 @@ public class EndpointEnricherTest extends AbstractAttributeEnricherTest {
private ApiEntityDao dao;

@BeforeEach
public void setup() {
void setup() {
dao = mock(ApiEntityDao.class);
endpointEnricher = new EndpointEnricher();
endpointEnricher.setApiEntityDao(dao);
}

@Test
public void whenEnrichedAttributesAreMissing_thenNoEnrichmentWillHappen() {
void whenEnrichedAttributesAreMissing_thenNoEnrichmentWillHappen() {
Event event =
Event.newBuilder()
.setCustomerId(TENANT_ID)
Expand All @@ -70,7 +70,7 @@ public void whenEnrichedAttributesAreMissing_thenNoEnrichmentWillHappen() {
}

@Test
public void whenServiceIdMissing_thenNoEnrichmentWillHappen() {
void whenServiceIdMissing_thenNoEnrichmentWillHappen() {
Event event = createMockApiBoundaryEntryEvent();
StructuredTrace trace = getBigTrace();
endpointEnricher.enrichEvent(trace, event);
Expand All @@ -79,7 +79,7 @@ public void whenServiceIdMissing_thenNoEnrichmentWillHappen() {
}

@Test
public void whenAllAttributesArePresentThenExpectEventToBeEnriched() {
void whenAllAttributesArePresentThenExpectEventToBeEnriched() {
StructuredTrace trace = getBigTrace();
Event event = trace.getEventList().get(0);
Entity entity =
Expand All @@ -99,7 +99,11 @@ public void whenAllAttributesArePresentThenExpectEventToBeEnriched() {
.build();

when(dao.upsertApiEntity(
trace.getCustomerId(), SERVICE_ID, ApiEntityDao.API_TYPE, event.getEventName()))
trace.getCustomerId(),
SERVICE_ID,
SERVICE_NAME,
ApiEntityDao.API_TYPE,
event.getEventName()))
.thenReturn(entity);
endpointEnricher.enrichEvent(trace, event);

Expand All @@ -110,7 +114,7 @@ public void whenAllAttributesArePresentThenExpectEventToBeEnriched() {
}

@Test
public void testApiEnrichmentForIntermediateEvents() {
void testApiEnrichmentForIntermediateEvents() {
StructuredTrace trace = getBigTrace();
Event exitEvent = trace.getEventList().get(2);
endpointEnricher.enrichTrace(trace);
Expand Down
31 changes: 17 additions & 14 deletions ...traceenricher/enrichment/enrichers/endpoint/OperationNameBasedEndpointDiscovererTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,52 +14,55 @@

public class OperationNameBasedEndpointDiscovererTest {

private OperationNameBasedEndpointDiscoverer underTest;
private ApiEntityDao dao;
private OperationNameBasedEndpointDiscoverer endpointDiscoverer;
private ApiEntityDao apiEntityDao;

@BeforeEach
public void setup() {
dao = mock(ApiEntityDao.class);
underTest = new OperationNameBasedEndpointDiscoverer("tenant-1", "service-1", dao);
apiEntityDao = mock(ApiEntityDao.class);
endpointDiscoverer =
new OperationNameBasedEndpointDiscoverer("tenant-1", "service-1", "service1", apiEntityDao);
}

@Test
public void whenCacheIsEmptyExpectCacheToLoadAndReturnEntity() throws Exception {
Entity entity =
Entity.newBuilder().setEntityId("entity-id").setEntityName("entity-name").build();
when(dao.upsertApiEntity(
"tenant-1", "service-1", ApiEntityDao.API_TYPE, "Driver::getCustomers"))
when(apiEntityDao.upsertApiEntity(
"tenant-1", "service-1", "service1", ApiEntityDao.API_TYPE, "Driver::getCustomers"))
.thenReturn(entity);
Event event =
Event.newBuilder()
.setEventId(ByteBuffer.wrap("event-id".getBytes()))
.setEventName("Driver::getCustomers")
.setCustomerId("tenant-1")
.build();
Entity expected = underTest.getApiEntity(event);
assertEquals(expected, underTest.getPatternToApiEntityCache().get(event.getEventName()));
Entity expected = endpointDiscoverer.getApiEntity(event);
assertEquals(
expected, endpointDiscoverer.getPatternToApiEntityCache().get(event.getEventName()));
}

@Test
public void whenCacheIsNotEmptyExpectCacheToReturnCachedEntity() throws Exception {
Entity entity =
Entity.newBuilder().setEntityId("entity-id").setEntityName("entity-name").build();
when(dao.upsertApiEntity(
"tenant-1", "service-1", ApiEntityDao.API_TYPE, "Driver::getCustomers"))
when(apiEntityDao.upsertApiEntity(
"tenant-1", "service-1", "service1", ApiEntityDao.API_TYPE, "Driver::getCustomers"))
.thenReturn(entity);
Event event =
Event.newBuilder()
.setEventId(ByteBuffer.wrap("event-id".getBytes()))
.setEventName("Driver::getCustomers")
.setCustomerId("tenant-1")
.build();
Entity expected = underTest.getApiEntity(event);
Entity expected = endpointDiscoverer.getApiEntity(event);

// query again
underTest.getApiEntity(event);
endpointDiscoverer.getApiEntity(event);
// make sure cache didn't trigger a load again. dao should have been called only once during
// setup above
verify(dao, times(1))
.upsertApiEntity("tenant-1", "service-1", ApiEntityDao.API_TYPE, event.getEventName());
verify(apiEntityDao, times(1))
.upsertApiEntity(
"tenant-1", "service-1", "service1", ApiEntityDao.API_TYPE, event.getEventName());
}
}