Add a generic grpc server settings config, cleanup client config #1183
Conversation
bogdandrutu
requested review from
dmitryax,
flands,
james-bebbington,
owais,
pjanotti,
rghetia and
tigrannajaryan
as code owners
Codecov Report
@@ Coverage Diff @@
## master #1183 +/- ##
==========================================
- Coverage 87.61% 87.59% -0.02%
==========================================
Files 204 204
Lines 14697 14677 -20
==========================================
- Hits 12877 12857 -20
Misses 1378 1378
Partials 442 442
Continue to review full report at Codecov.
|
bogdandrutu
force-pushed
the
grpcserver
branch
3 times, most recently
from
22a70c5 to
f6e8583
Compare
| func GrpcSettingsToDialOptions(settings GRPCClientSettings) ([]grpc.DialOption, error) { | ||
| type GRPCServerSettings struct { | ||
| // Configures the generic server protocol. | ||
| configprotocol.ProtocolServerSettings `mapstructure:",squash"` // squash ensures fields are correctly decoded in embedded struct |
There was a problem hiding this comment.
Is it worth putting consideration into how this will look in cases where we support multiple protocols over the same endpoint? e.g. gRPC Gateway or the Jaeger receiver config (something to think about for when generic HTTP Server settings are added as well)
Maybe gRPC Gateway is a special case where it makes sense to just continue to specificy CorsOrigins separate to the gRPC settings
There was a problem hiding this comment.
Also it seems weird to me that in gRPC (and HTTP) Client Settings, TLS settings are squashed, but in gRPC Server settings, TLS settings are under tls_configuration (inside ProtocolServerSettings). I know it may require a breaking change, but is there any possibilty we can make that more consistent?
There was a problem hiding this comment.
Will fix that in a future PR.
There was a problem hiding this comment.
Alright - I assume you're referring to the second comment.
Any thoughts on the first comment?
There was a problem hiding this comment.
Both will come soon :)
Signed-off-by: Bogdan Drutu <bogdandrutu@gmail.com>
…n-telemetry#1183) Signed-off-by: Bogdan Drutu <bogdandrutu@gmail.com>
Backwards compatible change for the configuration.
Depends on #1172
Fixes #1014