TLS option not effective

[mightyguava]

Describe the bug
Setting Tls: true on the redis options is not effective.

To reproduce
Set Tls: true for Redis to connect to a TLS enabled Redis instance.

Expected behavior
LaunchDarkly client initializes.

Logs

INFO: 2019/08/05 19:37:16 relay.go:435: Using Redis feature store: <REDACTED> with prefix: <REDACTED>
INFO: 2019/08/05 19:37:16 redis.go:317: RedisFeatureStore: Using url: redis://<REDACTED>
INFO: 2019/08/05 19:37:16 relay.go:364: Proxying events for
Starting LaunchDarkly streaming connection
Connecting to LaunchDarkly stream using URL: https://stream.launchdarkly.com/all
Timeout exceeded when initializing LaunchDarkly client.

SDK version
4.10.0

Additional context
By passing in the TLS DialOption, specifying host/port instead of URL, redigo will override the TLS dialoption. https://github.com/garyburd/redigo/blob/569eae59ada904ea8db7a225c2b47165af08735a/redis/conn.go#L288

There's an additional issue here, that issues with not being able to connect to Redis shows up in logs only as not being able to connect to launch darkly. There's no indication of an issue with Redis. This is probably more important the bug itself.

[eli-darkly]

There's an additional issue here, that issues with not being able to connect to Redis shows up in logs only as not being able to connect to launch darkly

That would be a Go SDK issue rather than anything the Relay code can control, but from looking at the Go SDK code it seems to me like such an error would be logged - at least, if Redigo actually returns an error result. If its behavior is instead to just hang, then due to the overall architecture of the SDK it's going to be hard for us to determine that the problem is at the feature store level.

Possibly the issue is that Redigo doesn't have a default connect timeout. If a connect timeout were specified, my guess is that it would return an error in this case instead of hanging. But for us to add a default connect timeout here might be a breaking change for users who for whatever reason have a Redis server that is slow to accept connections.

By passing in the TLS DialOption, specifying host/port instead of URL, redigo will override the TLS dialoption.

Looking at the Redigo code, I'm thinking that what you meant to say here is the opposite: it overrides it because we are specifying the URL, not the host/port? (If so, we can fix that, but again that is a Go SDK issue rather than a Relay issue.)

[mightyguava]

Sure, it could be fixed in either the relay or the Go SDK. The problem with the relay is https://github.com/launchdarkly/ld-relay/blob/v5/relay.go#L436, where it's turning host/port into a URL with redis:// instead of rediss://. From redigo's perspective, it would be ambiguous whether the scheme here should take precedence or the dialTLS option.

[eli-darkly]

That makes sense and is easy to fix - thanks.

[eli-darkly]

Please retest when you have a chance - 5.6.1 should fix this.