Skip to content

Commit 496971b

Browse files
yidongnanyidongnan
authored
Merge pull request #492 from yidongnan/fix/metrics-test
Fix failing metrics test
2 parents 89fd01b + 37ac4d6 commit 496971b

File tree

4 files changed

+167
-11
lines changed

4 files changed

+167
-11
lines changed

‎tests/src/test/java/net/devh/boot/grpc/test/config/AwaitableServerClientCallConfiguration.java

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717

1818
package net.devh.boot.grpc.test.config;
1919

20+
import static net.devh.boot.grpc.common.util.InterceptorOrder.ORDER_FIRST;
2021
import static net.devh.boot.grpc.common.util.InterceptorOrder.ORDER_LAST;
2122

2223
import java.util.concurrent.CountDownLatch;
@@ -29,7 +30,7 @@
2930
import io.grpc.ClientInterceptor;
3031
import io.grpc.ForwardingClientCall.SimpleForwardingClientCall;
3132
import io.grpc.ForwardingClientCallListener.SimpleForwardingClientCallListener;
32-
import io.grpc.ForwardingServerCall.SimpleForwardingServerCall;
33+
import io.grpc.ForwardingServerCallListener.SimpleForwardingServerCallListener;
3334
import io.grpc.Metadata;
3435
import io.grpc.MethodDescriptor;
3536
import io.grpc.ServerCall;
@@ -73,19 +74,25 @@ public <ReqT, RespT> Listener<ReqT> interceptCall(
7374
return next.startCall(call, headers);
7475
} else {
7576
final CountDownLatch thatCounter = serverCounter;
76-
return next.startCall(new SimpleForwardingServerCall<ReqT, RespT>(call) {
77+
return new SimpleForwardingServerCallListener<ReqT>(next.startCall(call, headers)) {
7778

7879
@Override
79-
public void close(final Status status, final Metadata trailers) {
80-
super.close(status, trailers);
80+
public void onComplete() {
81+
super.onComplete();
8182
thatCounter.countDown();
8283
}
8384

84-
}, headers);
85+
@Override
86+
public void onCancel() {
87+
super.onCancel();
88+
thatCounter.countDown();
89+
}
90+
91+
};
8592
}
8693
}
8794

88-
}, ORDER_LAST);
95+
}, ORDER_FIRST);
8996
}
9097

9198
/**

‎tests/src/test/java/net/devh/boot/grpc/test/metric/MetricCollectingInterceptorTest.java

Lines changed: 141 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,9 @@
1818
package net.devh.boot.grpc.test.metric;
1919

2020
import static io.grpc.Status.Code.CANCELLED;
21+
import static io.grpc.Status.Code.INTERNAL;
2122
import static io.grpc.Status.Code.UNIMPLEMENTED;
23+
import static io.grpc.Status.Code.UNKNOWN;
2224
import static net.devh.boot.grpc.common.metric.MetricConstants.METRIC_NAME_CLIENT_PROCESSING_DURATION;
2325
import static net.devh.boot.grpc.common.metric.MetricConstants.METRIC_NAME_CLIENT_REQUESTS_SENT;
2426
import static net.devh.boot.grpc.common.metric.MetricConstants.METRIC_NAME_CLIENT_RESPONSES_RECEIVED;
@@ -411,12 +413,12 @@ private synchronized void setError(final Throwable t) {
411413
}
412414

413415
/**
414-
* Test failing call.
416+
* Test unimplemented call.
415417
*/
416418
@Test
417419
@DirtiesContext
418-
void testMetricsFailingCall() {
419-
log.info("--- Starting tests with failing call ---");
420+
void testMetricsUniplementedCall() {
421+
log.info("--- Starting tests with unimplemented call ---");
420422

421423
final CountDownLatch counter = awaitNextServerAndClientCallCloses(1);
422424

@@ -479,4 +481,140 @@ void testMetricsFailingCall() {
479481
log.info("--- Test completed ---");
480482
}
481483

484+
/**
485+
* Test failed call.
486+
*/
487+
@Test
488+
@DirtiesContext
489+
void testMetricsFailedCall() {
490+
log.info("--- Starting tests with failing call ---");
491+
492+
final CountDownLatch counter = awaitNextServerAndClientCallCloses(1);
493+
494+
// Invoke
495+
assertThrows(StatusRuntimeException.class, () -> this.testService.secure(EMPTY));
496+
497+
assertTimeoutPreemptively(Duration.ofSeconds(1), (Executable) counter::await);
498+
499+
// Test-Client
500+
final Counter requestSentCounter = this.meterRegistry
501+
.find(METRIC_NAME_CLIENT_REQUESTS_SENT)
502+
.tag(MetricConstants.TAG_METHOD_NAME, "secure")
503+
.counter();
504+
assertNotNull(requestSentCounter);
505+
assertEquals(1, requestSentCounter.count());
506+
507+
final Counter responseReceivedCounter = this.meterRegistry
508+
.find(METRIC_NAME_CLIENT_RESPONSES_RECEIVED)
509+
.tag(MetricConstants.TAG_METHOD_NAME, "secure")
510+
.counter();
511+
assertNotNull(responseReceivedCounter);
512+
assertEquals(0, responseReceivedCounter.count());
513+
514+
final Timer clientTimer = this.meterRegistry
515+
.find(METRIC_NAME_CLIENT_PROCESSING_DURATION)
516+
.tag(MetricConstants.TAG_METHOD_NAME, "secure")
517+
.tag(TAG_STATUS_CODE, UNKNOWN.name())
518+
.timer();
519+
assertNotNull(clientTimer);
520+
assertEquals(1, clientTimer.count());
521+
assertTrue(clientTimer.max(TimeUnit.SECONDS) < 1);
522+
523+
// Test-Server
524+
final Counter requestsReceivedCounter = this.meterRegistry
525+
.find(METRIC_NAME_SERVER_REQUESTS_RECEIVED)
526+
.tag(MetricConstants.TAG_METHOD_NAME, "secure")
527+
.counter();
528+
assertNotNull(requestsReceivedCounter);
529+
assertEquals(1, requestsReceivedCounter.count());
530+
531+
final Counter responsesSentCounter = this.meterRegistry
532+
.find(METRIC_NAME_SERVER_RESPONSES_SENT)
533+
.tag(MetricConstants.TAG_METHOD_NAME, "secure")
534+
.counter();
535+
assertNotNull(responsesSentCounter);
536+
assertEquals(0, responsesSentCounter.count());
537+
538+
final Timer serverTimer = this.meterRegistry
539+
.find(METRIC_NAME_SERVER_PROCESSING_DURATION)
540+
.tag(MetricConstants.TAG_METHOD_NAME, "secure")
541+
.tag(TAG_STATUS_CODE, UNKNOWN.name())
542+
.timer();
543+
assertNotNull(serverTimer);
544+
assertEquals(1, serverTimer.count());
545+
assertTrue(serverTimer.max(TimeUnit.SECONDS) < 1);
546+
547+
// Client has network overhead so it has to be slower
548+
assertTrue(serverTimer.max(TimeUnit.SECONDS) <= clientTimer.max(TimeUnit.SECONDS));
549+
log.info("--- Test completed ---");
550+
}
551+
552+
/**
553+
* Test error call.
554+
*/
555+
@Test
556+
@DirtiesContext
557+
void testMetricsErrorCall() {
558+
log.info("--- Starting tests with error status call ---");
559+
560+
final CountDownLatch counter = awaitNextServerAndClientCallCloses(1);
561+
562+
// Invoke
563+
assertThrows(StatusRuntimeException.class, () -> this.testService.error(EMPTY));
564+
565+
assertTimeoutPreemptively(Duration.ofSeconds(1), (Executable) counter::await);
566+
567+
// Test-Client
568+
final Counter requestSentCounter = this.meterRegistry
569+
.find(METRIC_NAME_CLIENT_REQUESTS_SENT)
570+
.tag(MetricConstants.TAG_METHOD_NAME, "error")
571+
.counter();
572+
assertNotNull(requestSentCounter);
573+
assertEquals(1, requestSentCounter.count());
574+
575+
final Counter responseReceivedCounter = this.meterRegistry
576+
.find(METRIC_NAME_CLIENT_RESPONSES_RECEIVED)
577+
.tag(MetricConstants.TAG_METHOD_NAME, "error")
578+
.counter();
579+
assertNotNull(responseReceivedCounter);
580+
assertEquals(0, responseReceivedCounter.count());
581+
582+
final Timer clientTimer = this.meterRegistry
583+
.find(METRIC_NAME_CLIENT_PROCESSING_DURATION)
584+
.tag(MetricConstants.TAG_METHOD_NAME, "error")
585+
.tag(TAG_STATUS_CODE, INTERNAL.name())
586+
.timer();
587+
assertNotNull(clientTimer);
588+
assertEquals(1, clientTimer.count());
589+
assertTrue(clientTimer.max(TimeUnit.SECONDS) < 1);
590+
591+
// Test-Server
592+
final Counter requestsReceivedCounter = this.meterRegistry
593+
.find(METRIC_NAME_SERVER_REQUESTS_RECEIVED)
594+
.tag(MetricConstants.TAG_METHOD_NAME, "error")
595+
.counter();
596+
assertNotNull(requestsReceivedCounter);
597+
assertEquals(1, requestsReceivedCounter.count());
598+
599+
final Counter responsesSentCounter = this.meterRegistry
600+
.find(METRIC_NAME_SERVER_RESPONSES_SENT)
601+
.tag(MetricConstants.TAG_METHOD_NAME, "error")
602+
.counter();
603+
assertNotNull(responsesSentCounter);
604+
assertEquals(0, responsesSentCounter.count());
605+
606+
final Timer serverTimer = this.meterRegistry
607+
.find(METRIC_NAME_SERVER_PROCESSING_DURATION)
608+
.tag(MetricConstants.TAG_METHOD_NAME, "error")
609+
.tag(TAG_STATUS_CODE, INTERNAL.name())
610+
.timer();
611+
assertNotNull(serverTimer);
612+
assertEquals(1, serverTimer.count());
613+
assertTrue(serverTimer.max(TimeUnit.SECONDS) < 1);
614+
615+
// Client has network overhead so it has to be slower
616+
assertTrue(serverTimer.max(TimeUnit.SECONDS) <= clientTimer.max(TimeUnit.SECONDS));
617+
log.info("--- Test completed ---");
618+
}
619+
482620
}

‎tests/src/test/java/net/devh/boot/grpc/test/server/TestServiceImpl.java

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
import com.google.protobuf.Empty;
3131

3232
import io.grpc.Context;
33+
import io.grpc.Status;
3334
import io.grpc.stub.StreamObserver;
3435
import lombok.extern.slf4j.Slf4j;
3536
import net.devh.boot.grpc.server.security.interceptors.AuthenticatingServerInterceptor;
@@ -41,7 +42,7 @@
4142
@GrpcService
4243
public class TestServiceImpl extends TestServiceImplBase {
4344

44-
public static final int METHOD_COUNT = 7;
45+
public static final int METHOD_COUNT = 8;
4546

4647
public TestServiceImpl() {
4748
log.info("Created TestServiceImpl");
@@ -63,7 +64,14 @@ public void unimplemented(final Empty request, final StreamObserver<SomeType> re
6364
}
6465

6566
@Override
66-
public StreamObserver<SomeType> echo(StreamObserver<SomeType> responseObserver) {
67+
public void error(final Empty request, final StreamObserver<Empty> responseObserver) {
68+
log.debug("error");
69+
responseObserver.onError(Status.INTERNAL.asRuntimeException());
70+
}
71+
72+
@Override
73+
public StreamObserver<SomeType> echo(final StreamObserver<SomeType> responseObserver) {
74+
log.debug("echo");
6775
return responseObserver;
6876
}
6977

‎tests/src/test/proto/TestService.proto

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,9 @@ service TestService {
1010

1111
// Returns version information
1212
rpc normal(google.protobuf.Empty) returns (SomeType) {}
13+
14+
// Fails with an error
15+
rpc error(google.protobuf.Empty) returns (google.protobuf.Empty) {}
1316

1417
// Unimplemented method
1518
rpc unimplemented(google.protobuf.Empty) returns (SomeType) {}

0 commit comments

Comments
 (0)