[jaeger-v2] Align Cassandra Storage Config With OTEL #5949
Conversation
Signed-off-by: Mahad Zaryab <[email protected]>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #5949 +/- ##
==========================================
+ Coverage 96.82% 96.84% +0.01%
==========================================
Files 345 345
Lines 16523 16524 +1
==========================================
+ Hits 15998 16002 +4
+ Misses 339 335 -4
- Partials 186 187 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Signed-off-by: Mahad Zaryab <[email protected]>
pkg/cassandra/config/config.go
Outdated
| Authenticator Authenticator `mapstructure:",squash"` | ||
| DisableAutoDiscovery bool `mapstructure:"-"` | ||
| TLS tlscfg.Options `mapstructure:"tls"` | ||
| Servers []string `valid:"required,url" mapstructure:"servers"` |
There was a problem hiding this comment.
please move valid to the right of mapstructure
pkg/cassandra/config/config.go
Outdated
| Keyspace string `mapstructure:"keyspace"` | ||
| LocalDC string `mapstructure:"local_dc"` | ||
| ConnectionsPerHost int `mapstructure:"connections_per_host"` | ||
| Timeout time.Duration `mapstructure:"-"` |
There was a problem hiding this comment.
needs a mapstructure name. Also needs a better name - which timeout is it?
There was a problem hiding this comment.
it's the query timeout - added the tag as well
Signed-off-by: Mahad Zaryab <[email protected]>
Signed-off-by: Mahad Zaryab <[email protected]>
Signed-off-by: Mahad Zaryab <[email protected]>
Signed-off-by: Mahad Zaryab <[email protected]>
Signed-off-by: Mahad Zaryab <[email protected]>
Signed-off-by: Mahad Zaryab <[email protected]>
pkg/cassandra/config/config.go
Outdated
| ConnectionsPerHost int `mapstructure:"connections_per_host"` | ||
| ReconnectInterval time.Duration `mapstructure:"reconnect_interval"` | ||
| SocketKeepAlive time.Duration `mapstructure:"socket_keep_alive"` | ||
| MaxRetryAttempts int `mapstructure:"max_retry_attempts"` |
There was a problem hiding this comment.
This is for query? If yes I would prefix as such.
pkg/cassandra/config/config.go
Outdated
| ConnectTimeout time.Duration `mapstructure:"connection_timeout"` | ||
| Authenticator Authenticator `mapstructure:"auth"` | ||
| ProtoVersion int `mapstructure:"proto_version"` | ||
| Consistency string `mapstructure:"consistency"` |
There was a problem hiding this comment.
I'm starting to think you may want to partition these further into connection and query - it seems the fields are pretty separate for those.
There was a problem hiding this comment.
Created a separate grouping for query. Should Consistency be under Query as well?
There was a problem hiding this comment.
I think so - it's passed as part of queries, not as connection parameter, is it not?
Signed-off-by: Mahad Zaryab <[email protected]>
Signed-off-by: Mahad Zaryab <[email protected]>
Signed-off-by: Mahad Zaryab <[email protected]>
Signed-off-by: Mahad Zaryab <[email protected]>
Signed-off-by: Mahad Zaryab <[email protected]>
Signed-off-by: Mahad Zaryab <[email protected]>
|
Is |
Which problem is this PR solving?
Description of the changes
Validatemethodconfigtls.ClientConfigtypeHow was this change tested?
Checklist
jaeger:make lint testjaeger-ui:yarn lintandyarn test