cloudfoundry / loggregator-release

Cloud Native Logging
Apache License 2.0
215 stars 149 forks source link

fix: Replace deprecated `grpc.Dial` with `grpc.NewClient` #745

Closed ctlong closed 5 months ago

ctlong commented 5 months ago

Description

Replaces grpc.Dial usage with grpc.NewClient. gRPC deprecated (and then briefly un-deprecated, but plan to re-deprecate) Dial and DialContext in favor of NewClient. See https://github.com/grpc/grpc-go/pull/7010 for more info about NewClient.

Type of change

Testing performed?

Checklist:

If you have any questions, or want to get attention for a PR or issue please reach out on the #logging-and-metrics channel in the cloudfoundry slack

ctlong commented 5 months ago

@rroberts2222 and I tested these changes within CF-D and saw that they resulted in RLP and TrafficController not initiating any TCP connections to Dopplers until a client attempted to connect to them. This is in contrast to the old behaviour where the Doppler connection pools were pre-filled to the max. We think this change is fine.

@acrmp and tested the RLP Gateway changes and saw that clients will consistently receive 503 errors now when the RLP is not available, e.g. from the log-stream plugin:

unexpected status code 503: {"message":"streaming is temporarily unavailable","error":"streaming_unavailable"}

The RLP Gateway logs will not emit stack traces in those cases, but will emit an error line for every failed request:

2024/04/16 21:41:25 failed to open stream from logs provider: rpc error: code = Unavailable desc = connection error: desc = "transport: Error while dialing: dial tcp 127.0.0.1:8082: connect: connection refused"