Skip to content
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

Jump to bottom

Minor gRPC fixes (2.0) #1951

Merged
merged 2 commits into from
Jun 9, 2020
Merged

Minor gRPC fixes (2.0) #1951

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
915f6a5
Publish snapshots to Synoki Nexus
aseovic May 20, 2020
2943651
gRPC 2.0 fixes
aseovic Jun 9, 2020
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
19 changes: 14 additions & 5 deletions grpc/metrics/src/main/java/io/helidon/grpc/metrics/GrpcMetrics.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
import org.eclipse.microprofile.metrics.Meter;
import org.eclipse.microprofile.metrics.MetricRegistry;
import org.eclipse.microprofile.metrics.MetricType;
import org.eclipse.microprofile.metrics.MetricUnits;
import org.eclipse.microprofile.metrics.Tag;
import org.eclipse.microprofile.metrics.Timer;

Expand All @@ -53,11 +54,6 @@
public class GrpcMetrics
implements ServerInterceptor, ServiceDescriptor.Configurer, MethodDescriptor.Configurer {

static {
HelidonFeatures.register("gRPC Server", "Metrics");
HelidonFeatures.register("gRPC Client", "Metrics");
}

/**
* The registry of vendor metrics.
*/
Expand All @@ -70,6 +66,19 @@ public class GrpcMetrics
private static final MetricRegistry APP_REGISTRY =
RegistryFactory.getInstance().getRegistry(MetricRegistry.Type.APPLICATION);

static {
HelidonFeatures.register("gRPC Server", "Metrics");
HelidonFeatures.register("gRPC Client", "Metrics");

VENDOR_REGISTRY.meter(org.eclipse.microprofile.metrics.Metadata.builder()
.withName("grpc.requests.meter")
.withDisplayName("Meter for overall gRPC requests")
.withDescription("Each gRPC request will mark the meter to see overall throughput")
.withType(MetricType.METERED)
.withUnit(MetricUnits.NONE)
.build());
Comment on lines +73 to +79
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see that this takes the place of the meter that used to be registered in MetricsSupport. There is no replacement here for the counter that MetricsSupport used to register. Is that intentional?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes -- meter already has a counter, so having a separate counter is a bit pointless.

}

/**
* The context key name to use to obtain rules to use when applying metrics.
*/
Expand Down
16 changes: 0 additions & 16 deletions metrics/metrics/src/main/java/io/helidon/metrics/MetricsSupport.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -367,22 +367,6 @@ public void configureVendorMetrics(String routingName,
.withUnit(MetricUnits.NONE)
.build());

vendor.counter(Metadata.builder()
.withName("grpc.requests.count")
.withDisplayName("Total number of gRPC requests")
.withDescription("Each gRPC request (regardless of the method) will increase this counter")
.withType(MetricType.COUNTER)
.withUnit(MetricUnits.NONE)
.build());

vendor.meter(Metadata.builder()
.withName("grpc.requests.meter")
.withDisplayName("Meter for overall gRPC requests")
.withDescription("Each gRPC request will mark the meter to see overall throughput")
.withType(MetricType.METERED)
.withUnit(MetricUnits.NONE)
.build());

rules.any((req, res) -> {
totalCount.inc();
totalMeter.mark();
Expand Down
37 changes: 13 additions & 24 deletions ...file/grpc/client/src/main/java/io/helidon/microprofile/grpc/client/GrpcProxyProducer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,28 +41,7 @@ private GrpcProxyProducer() {
/**
* A CDI producer method that produces a client proxy for a gRPC service that
* will connect to the server using the channel specified via {@link GrpcChannel}
* annotation on the proxy interface, or the default {@link Channel}.
* <p>
* This is not a real producer method but is used as a stub by the gRPC client
* CDI extension to create real producers as injection points are discovered.
*
* @param injectionPoint the injection point where the client proxy is to be injected
* @return a gRPC client proxy
*/
@GrpcProxy
static Object proxyUsingDefaultChannel(InjectionPoint injectionPoint, ChannelProducer producer) {
Class<?> type = ModelHelper.getGenericType(injectionPoint.getType());
String channelName = type.isAnnotationPresent(GrpcChannel.class)
? type.getAnnotation(GrpcChannel.class).name()
: GrpcChannelsProvider.DEFAULT_CHANNEL_NAME;
Channel channel = producer.findChannel(channelName);
GrpcProxyBuilder<?> builder = GrpcProxyBuilder.create(channel, type);
return builder.build();
}

/**
* A CDI producer method that produces a client proxy for a gRPC service that
* will connect to the server using a named {@link Channel}.
* annotation on the proxy interface or injection point, or the default {@link Channel}.
* <p>
* This is not a real producer method but is used as a stub by the gRPC client
* CDI extension to create real producers as injection points are discovered.
Expand All @@ -74,9 +53,19 @@ static Object proxyUsingDefaultChannel(InjectionPoint injectionPoint, ChannelPro
@GrpcChannel(name = GrpcChannelsProvider.DEFAULT_CHANNEL_NAME)
static Object proxyUsingNamedChannel(InjectionPoint injectionPoint, ChannelProducer producer) {
Class<?> type = ModelHelper.getGenericType(injectionPoint.getType());
GrpcChannel channelName = injectionPoint.getAnnotated().getAnnotation(GrpcChannel.class);
Channel channel = producer.findChannel(channelName.name());

String channelName;
if (injectionPoint.getAnnotated().isAnnotationPresent(GrpcChannel.class)) {
channelName = injectionPoint.getAnnotated().getAnnotation(GrpcChannel.class).name();
} else {
channelName = type.isAnnotationPresent(GrpcChannel.class)
? type.getAnnotation(GrpcChannel.class).name()
: GrpcChannelsProvider.DEFAULT_CHANNEL_NAME;
}

Channel channel = producer.findChannel(channelName);
GrpcProxyBuilder<?> builder = GrpcProxyBuilder.create(channel, type);

return builder.build();
}

Expand Down