Skip to content

Commit e839a5f

Browse files
Xylusemeroad
authored andcommitted
[pinpoint-apm#2939] Fix rpc hint checks for WAS to WAS filtering
1 parent ab1beb0 commit e839a5f

File tree

3 files changed

+78
-7
lines changed

3 files changed

+78
-7
lines changed

‎web/src/main/java/com/navercorp/pinpoint/web/filter/FilterHint.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -45,14 +45,14 @@ public FilterHint(List<RpcHint> rpcHintList) {
4545
}
4646

4747

48-
public List<RpcHint> getRpcHintList(String sourceApplicationName) {
49-
if (sourceApplicationName == null) {
50-
throw new NullPointerException("sourceApplicationName must not be null");
48+
public List<RpcHint> getRpcHintList(String targetApplicationName) {
49+
if (targetApplicationName == null) {
50+
throw new NullPointerException("targetApplicationName must not be null");
5151
}
5252
final List<RpcHint> findRpcHintList = new ArrayList<>();
5353
for (RpcHint rpcHint : rpcHintList) {
5454
// TODO miss serviceType
55-
if (rpcHint.getApplicationName().equals(sourceApplicationName)) {
55+
if (rpcHint.getApplicationName().equals(targetApplicationName)) {
5656
findRpcHintList.add(rpcHint);
5757
}
5858
}

‎web/src/main/java/com/navercorp/pinpoint/web/filter/LinkFilter.java‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,8 @@ public LinkFilter(FilterDescriptor filterDescriptor, FilterHint filterHint, Serv
122122
this.filterType = getFilterType();
123123
logger.info("filterType:{}", filterType);
124124

125-
this.rpcHintList = this.filterHint.getRpcHintList(fromApplicationName);
125+
this.rpcHintList = this.filterHint.getRpcHintList(toApplicationName);
126+
126127
// TODO fix : fromSpan base rpccall filter
127128
this.acceptURLFilter = createAcceptUrlFilter(filterDescriptor);
128129
this.rpcUrlFilter = createRpcUrlFilter(filterDescriptor);
@@ -359,9 +360,11 @@ private boolean wasToWasFilter(List<SpanBo> transaction) {
359360
return false;
360361
}
361362

362-
if (!rpcHintList.isEmpty()) {
363+
// Check for url pattern should now be done on the caller side (from spans) as to spans are missing at this point
364+
if (!rpcUrlFilter.accept(fromSpanList)) {
363365
return false;
364366
}
367+
365368
// if agent filter is FromAgentFilter or AcceptAgentFilter(agent filter is not selected), url filtering is available.
366369
return fromBaseFilter(fromSpanList);
367370
}
@@ -395,20 +398,27 @@ private boolean fromBaseFilter(List<SpanBo> fromSpanList) {
395398
// exceptional case
396399
// 1. remote call fail
397400
// 2. span packet lost.
401+
if (rpcHintList.isEmpty()) {
402+
// fast skip. There is nothing more we can do if rpcHintList is empty.
403+
return false;
404+
}
398405
for (SpanBo fromSpan : fromSpanList) {
399406
final List<SpanEventBo> eventBoList = fromSpan.getSpanEventBoList();
400407
if (eventBoList == null) {
401408
continue;
402409
}
403410
for (SpanEventBo event : eventBoList) {
404411
final ServiceType eventServiceType = serviceTypeRegistryService.findServiceType(event.getServiceType());
405-
if (!eventServiceType.isRpcClient() || !eventServiceType.isQueue()) {
412+
if (!eventServiceType.isRpcClient() && !eventServiceType.isQueue()) {
406413
continue;
407414
}
408415
if (!eventServiceType.isRecordStatistics()) {
409416
continue;
410417
}
411418
// check rpc call fail
419+
// There are also cases where multiple applications receiving the same request from the caller node
420+
// but not all of them have agents installed. RpcHint is used for such cases as acceptUrlFilter will
421+
// reject these transactions.
412422
for (RpcHint rpcHint : rpcHintList) {
413423
for (RpcType rpcType : rpcHint.getRpcTypeList()) {
414424
if (rpcType.isMatched(event.getDestinationId(), eventServiceType.getCode())) {

‎web/src/test/java/com/navercorp/pinpoint/web/filter/LinkFilterTest.java‎

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -313,6 +313,67 @@ public void wasToWasFilter_noMatch() {
313313
Assert.assertFalse(linkFilter.include(Arrays.asList(user_appA, appA_appC)));
314314
}
315315

316+
@Test
317+
public void wasToWasFilter_noMatch_missingReceivingSpan() {
318+
final ServiceType tomcat = serviceTypeRegistryService.findServiceTypeByName(TOMCAT_TYPE_NAME);
319+
320+
final String rpcHost = "some.domain.name";
321+
final String rpcUrl = "http://" + rpcHost + "/some/test/path";
322+
323+
FilterDescriptor descriptor = new FilterDescriptor();
324+
descriptor.setFromApplicationName("APP_A");
325+
descriptor.setFromServiceType(tomcat.getName());
326+
descriptor.setToApplicationName("APP_B");
327+
descriptor.setToServiceType(tomcat.getName());
328+
329+
FilterHint emptyHint = new FilterHint(Collections.emptyList());
330+
FilterHint unmatchingHint = new FilterHint(Collections.singletonList(
331+
new RpcHint("APP_B", Collections.singletonList(
332+
new RpcType("different.domain.name", RPC_TYPE_CODE)))));
333+
FilterHint matchingHint = new FilterHint(Collections.singletonList(
334+
new RpcHint("APP_B", Collections.singletonList(
335+
new RpcType(rpcHost, RPC_TYPE_CODE)))));
336+
337+
LinkFilter emptyHintLinkFilter = new LinkFilter(descriptor, emptyHint, serviceTypeRegistryService, annotationKeyRegistryService);
338+
LinkFilter unmatchingHintLinkFilter = new LinkFilter(descriptor, unmatchingHint, serviceTypeRegistryService, annotationKeyRegistryService);
339+
LinkFilter matchingHintLinkFilter = new LinkFilter(descriptor, matchingHint, serviceTypeRegistryService, annotationKeyRegistryService);
340+
logger.debug("emptyHintLinkFilter : {}", emptyHintLinkFilter.toString());
341+
logger.debug("unmatchingHintLinkFilter : {}", unmatchingHintLinkFilter.toString());
342+
logger.debug("matchingHintLinkFilter : {}", matchingHintLinkFilter.toString());
343+
344+
SpanBo fromSpan = new SpanBo();
345+
fromSpan.setSpanId(1);
346+
fromSpan.setParentSpanId(-1);
347+
fromSpan.setApplicationId("APP_A");
348+
fromSpan.setApplicationServiceType(tomcat.getCode());
349+
AnnotationBo rpcAnnotation = new AnnotationBo();
350+
rpcAnnotation.setKey(RPC_ANNOTATION_CODE);
351+
rpcAnnotation.setValue(rpcUrl);
352+
SpanEventBo rpcSpanEvent = new SpanEventBo();
353+
rpcSpanEvent.setServiceType(RPC_TYPE_CODE);
354+
rpcSpanEvent.setDestinationId(rpcHost);
355+
rpcSpanEvent.setAnnotationBoList(Collections.singletonList(rpcAnnotation));
356+
fromSpan.addSpanEvent(rpcSpanEvent);
357+
// Reject - filter hint empty
358+
Assert.assertFalse(emptyHintLinkFilter.include(Collections.singletonList(fromSpan)));
359+
// Reject - filter hint does not match
360+
Assert.assertFalse(unmatchingHintLinkFilter.include(Collections.singletonList(fromSpan)));
361+
// Accept - filter hint matches
362+
Assert.assertTrue(matchingHintLinkFilter.include(Collections.singletonList(fromSpan)));
363+
364+
// Check rpc url as well
365+
final String unmatchingUrlPattern = "/other/test/**";
366+
final String matchingUrlPattern = "/some/test/**";
367+
// Reject - url pattern does not match
368+
descriptor.setUrl(unmatchingUrlPattern);
369+
LinkFilter matchingHintLinkFilterWithUnmatchingUrlPattern = new LinkFilter(descriptor, matchingHint, serviceTypeRegistryService, annotationKeyRegistryService);
370+
Assert.assertFalse(matchingHintLinkFilterWithUnmatchingUrlPattern.include(Collections.singletonList(fromSpan)));
371+
// Accept - url pattern matches
372+
descriptor.setUrl(encodeUrl(matchingUrlPattern));
373+
LinkFilter matchingHintLinkFilterWithMatchingUrlPattern = new LinkFilter(descriptor, matchingHint, serviceTypeRegistryService, annotationKeyRegistryService);
374+
Assert.assertTrue(matchingHintLinkFilterWithMatchingUrlPattern.include(Collections.singletonList(fromSpan)));
375+
}
376+
316377
@Test
317378
public void wasToBackendFilter() {
318379
final ServiceType tomcat = serviceTypeRegistryService.findServiceTypeByName(TOMCAT_TYPE_NAME);

0 commit comments

Comments
 (0)