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.

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

Add gRPC service registration feature in spring-boot-autoconfigure #1749

Conversation

matsumana
Copy link
Member

Hello, I've added gRPC service registration feature in spring-boot-autoconfigure

Motivation:

  • Makes it easy to configure gRPC server in Spring Boot applications

Modifications:

  • Add GrpcServiceRegistrationBean.java
  • Add configureGrpcServices method in ArmeriaConfigurationUtil.java then call it in ArmeriaAutoConfiguration.java and ArmeriaReactiveWebServerFactory.java

Result:

  • It can be configured as follows
@Bean
public GrpcServiceRegistrationBean helloService() {
    return new GrpcServiceRegistrationBean()
            .setServiceName("helloService")
            .setService(new GrpcServiceBuilder()
                                .addService(new HelloService())
                                .supportedSerializationFormats(
                                        GrpcSerializationFormats.values())
                                .enableUnframedRequests(true)
                                .build())
            .setDecorators(LoggingService.newDecorator())
            .setExampleRequests(List.of(ExampleRequest.of(HelloServiceGrpc.SERVICE_NAME,
                                                          "Hello",
                                                          HelloRequest.newBuilder()
                                                                      .setName("Armeria")
                                                                      .build())));
}

@codecov
Copy link

codecov bot commented May 4, 2019

Codecov Report

Merging #1749 into master will increase coverage by 0.04%.
The diff coverage is 94%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #1749      +/-   ##
============================================
+ Coverage      71.8%   71.85%   +0.04%     
- Complexity     8419     8428       +9     
============================================
  Files           760      761       +1     
  Lines         33780    33827      +47     
  Branches       4158     4162       +4     
============================================
+ Hits          24255    24305      +50     
+ Misses         7420     7405      -15     
- Partials       2105     2117      +12
Impacted Files Coverage Δ Complexity Δ
...meria/spring/AnnotatedServiceRegistrationBean.java 85% <ø> (ø) 9 <0> (ø) ⬇️
...rmeria/spring/AbstractServiceRegistrationBean.java 92.85% <ø> (ø) 9 <0> (ø) ⬇️
...rp/armeria/spring/HttpServiceRegistrationBean.java 80% <ø> (ø) 3 <0> (ø) ⬇️
...rp/armeria/spring/GrpcServiceRegistrationBean.java 100% <100%> (ø) 3 <3> (?)
...ecorp/armeria/spring/ArmeriaAutoConfiguration.java 78.57% <83.33%> (+0.19%) 8 <0> (ø) ⬇️
.../web/reactive/ArmeriaReactiveWebServerFactory.java 72.5% <83.33%> (+0.57%) 21 <0> (ø) ⬇️
...eria/internal/spring/ArmeriaConfigurationUtil.java 61.3% <95.83%> (+3.14%) 38 <5> (+5) ⬆️
...corp/armeria/common/stream/FixedStreamMessage.java 80.35% <0%> (-3.58%) 18% <0%> (-2%)
...lient/circuitbreaker/CircuitBreakerHttpClient.java 69.69% <0%> (-3.04%) 10% <0%> (-1%)
...inecorp/armeria/client/grpc/ArmeriaClientCall.java 80.59% <0%> (-1.5%) 36% <0%> (-1%)
... and 9 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4566782...ba7754e. Read the comment docs.

@matsumana matsumana force-pushed the feature/add-grpc-integration-for-spring-boot-autoconfigure branch from 231d9ae to 1fef5de Compare May 4, 2019 19:40
@matsumana matsumana force-pushed the feature/add-grpc-integration-for-spring-boot-autoconfigure branch from 1fef5de to 678fef6 Compare May 4, 2019 19:45
@trustin trustin added this to the 0.85.0 milestone May 8, 2019
Copy link
Member

@trustin trustin left a comment

Choose a reason for hiding this comment

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

Very nice work! Left just a few minor comments. 👍

spring/boot-autoconfigure/build.gradle Outdated Show resolved Hide resolved
spring/boot-autoconfigure/build.gradle Outdated Show resolved Hide resolved
Because gradle-scripts takes care that.
@matsumana
Copy link
Member Author

Thank you for your comment!
updated.

The test failed in AppVeyor, but it seems to have nothing to do with the last push, right?

@trustin
Copy link
Member

trustin commented May 9, 2019

it seems to have nothing to do with the last push, right?

Yeah, just an issue with AppVeyor.

Copy link
Member

@trustin trustin left a comment

Choose a reason for hiding this comment

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

👍

* An abstract bean with information for registering a service object. It enables micrometer
* monitoring of the service automatically.
* An abstract bean with information for registering a service object.
* It enables Micrometer metric collection of the service automatically.
Copy link
Member

Choose a reason for hiding this comment

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

Thanks! 🙇‍♂️

Copy link
Member

@minwoox minwoox left a comment

Choose a reason for hiding this comment

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

Great job!

docServiceBuilder,
grpcServiceRegistrationBean.orElseGet(Collections::emptyList),
meterIdPrefixFuncFactory,
armeriaSettings.getDocsPath());
Copy link
Member

Choose a reason for hiding this comment

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

nit: Could extract armeriaSettings.getDocsPath() as a variable and reuse it?

return exampleRequest;
}

public static ExampleRequest of(String serviceType, String methodName, Object exampleRequest) {
Copy link
Member

Choose a reason for hiding this comment

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

nit: Could add @NotNull to all parameters?

Copy link
Contributor

@hyangtack hyangtack left a comment

Choose a reason for hiding this comment

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

Great job!

@trustin
Copy link
Member

trustin commented May 9, 2019

@matsumana Would you mind resolving the conflict and addressing @minwoox's comments?

@matsumana
Copy link
Member Author

Updated!

@trustin trustin merged commit 031981f into line:master May 9, 2019
@trustin
Copy link
Member

trustin commented May 9, 2019

Thanks, @matsumana !

@matsumana
Copy link
Member Author

Thank you!

@matsumana matsumana deleted the feature/add-grpc-integration-for-spring-boot-autoconfigure branch May 9, 2019 11:27
@xmeng1
Copy link

xmeng1 commented Oct 11, 2019

is it possible to config with the DocServiceBuilder?

fmguerreiro pushed a commit to fmguerreiro/armeria that referenced this pull request Sep 19, 2020
…ine#1749)

Motivation:

- Makes it easy to configure gRPC server in Spring Boot applications

Modifications:

- Add `GrpcServiceRegistrationBean.java`
- Add `configureGrpcServices` method in `ArmeriaConfigurationUtil.java` then call it in `ArmeriaAutoConfiguration.java` and `ArmeriaReactiveWebServerFactory.java`

Result:

- It can be configured as follows

```java
@bean
public GrpcServiceRegistrationBean helloService() {
    return new GrpcServiceRegistrationBean()
            .setServiceName("helloService")
            .setService(new GrpcServiceBuilder()
                                .addService(new HelloService())
                                .supportedSerializationFormats(
                                        GrpcSerializationFormats.values())
                                .enableUnframedRequests(true)
                                .build())
            .setDecorators(LoggingService.newDecorator())
            .setExampleRequests(List.of(ExampleRequest.of(HelloServiceGrpc.SERVICE_NAME,
                                                          "Hello",
                                                          HelloRequest.newBuilder()
                                                                      .setName("Armeria")
                                                                      .build())));
}
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants