[jaeger-v2] Align Cassandra Storage Config With OTEL

mahadzaryab1 commented 3 weeks ago

Which problem is this PR solving?

Resolves #5928

Description of the changes

Revamped configuration for Cassandra to be logically grouped
Added unit tests for the Validate method
Changed the configuration to use OTEL's configtls.ClientConfig type
Migration guide

How was this change tested?

New unit tests
CI

Checklist

[x] I have read https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md
[x] I have signed all commits
[x] I have added unit tests for the new functionality
[x] I have run lint and test steps successfully
- for jaeger: make lint test
- for jaeger-ui: yarn lint and yarn test

codecov[bot] commented 3 weeks ago

Codecov Report

Attention: Patch coverage is 97.95918% with 2 lines in your changes missing coverage. Please review.

Project coverage is 96.90%. Comparing base (d7f01d1) to head (2f1f0e9). Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
pkg/cassandra/config/config.go	96.72%	1 Missing and 1 partial :warning:

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #5949 +/- ## ========================================== + Coverage 96.80% 96.90% +0.10% ========================================== Files 349 349 Lines 16581 16582 +1 ========================================== + Hits 16051 16069 +18 + Misses 342 329 -13 + Partials 188 184 -4 ``` | [Flag](https://app.codecov.io/gh/jaegertracing/jaeger/pull/5949/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=jaegertracing) | Coverage Δ | | |---|---|---| | [badger_v1](https://app.codecov.io/gh/jaegertracing/jaeger/pull/5949/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=jaegertracing) | `8.00% <0.00%> (-0.01%)` | :arrow_down: | | [badger_v2](https://app.codecov.io/gh/jaegertracing/jaeger/pull/5949/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=jaegertracing) | `1.82% <0.00%> (-0.01%)` | :arrow_down: | | [cassandra-4.x-v1](https://app.codecov.io/gh/jaegertracing/jaeger/pull/5949/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=jaegertracing) | `15.78% <87.75%> (-0.79%)` | :arrow_down: | | [cassandra-4.x-v2](https://app.codecov.io/gh/jaegertracing/jaeger/pull/5949/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=jaegertracing) | `1.75% <0.00%> (-0.01%)` | :arrow_down: | | [cassandra-5.x-v1](https://app.codecov.io/gh/jaegertracing/jaeger/pull/5949/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=jaegertracing) | `15.78% <87.75%> (-0.79%)` | :arrow_down: | | [cassandra-5.x-v2](https://app.codecov.io/gh/jaegertracing/jaeger/pull/5949/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=jaegertracing) | `1.75% <0.00%> (-0.01%)` | :arrow_down: | | [elasticsearch-6.x-v1](https://app.codecov.io/gh/jaegertracing/jaeger/pull/5949/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=jaegertracing) | `18.72% <0.00%> (-0.01%)` | :arrow_down: | | [elasticsearch-7.x-v1](https://app.codecov.io/gh/jaegertracing/jaeger/pull/5949/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=jaegertracing) | `18.78% <0.00%> (-0.01%)` | :arrow_down: | | [elasticsearch-8.x-v1](https://app.codecov.io/gh/jaegertracing/jaeger/pull/5949/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=jaegertracing) | `18.98% <0.00%> (-0.01%)` | :arrow_down: | | [elasticsearch-8.x-v2](https://app.codecov.io/gh/jaegertracing/jaeger/pull/5949/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=jaegertracing) | `1.82% <0.00%> (-0.01%)` | :arrow_down: | | [grpc_v1](https://app.codecov.io/gh/jaegertracing/jaeger/pull/5949/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=jaegertracing) | `9.53% <0.00%> (-0.01%)` | :arrow_down: | | [grpc_v2](https://app.codecov.io/gh/jaegertracing/jaeger/pull/5949/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=jaegertracing) | `7.12% <0.00%> (-0.02%)` | :arrow_down: | | [kafka-v1](https://app.codecov.io/gh/jaegertracing/jaeger/pull/5949/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=jaegertracing) | `9.71% <0.00%> (-0.01%)` | :arrow_down: | | [kafka-v2](https://app.codecov.io/gh/jaegertracing/jaeger/pull/5949/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=jaegertracing) | `1.82% <0.00%> (-0.01%)` | :arrow_down: | | [memory_v2](https://app.codecov.io/gh/jaegertracing/jaeger/pull/5949/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=jaegertracing) | `1.82% <0.00%> (-0.01%)` | :arrow_down: | | [opensearch-1.x-v1](https://app.codecov.io/gh/jaegertracing/jaeger/pull/5949/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=jaegertracing) | `18.84% <0.00%> (-0.01%)` | :arrow_down: | | [opensearch-2.x-v1](https://app.codecov.io/gh/jaegertracing/jaeger/pull/5949/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=jaegertracing) | `18.84% <0.00%> (-0.01%)` | :arrow_down: | | [opensearch-2.x-v2](https://app.codecov.io/gh/jaegertracing/jaeger/pull/5949/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=jaegertracing) | `1.81% <0.00%> (-0.02%)` | :arrow_down: | | [tailsampling-processor](https://app.codecov.io/gh/jaegertracing/jaeger/pull/5949/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=jaegertracing) | `0.46% <0.00%> (-0.01%)` | :arrow_down: | | [unittests](https://app.codecov.io/gh/jaegertracing/jaeger/pull/5949/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=jaegertracing) | `95.70% <95.91%> (+0.41%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=jaegertracing#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

mahadzaryab1 commented 2 weeks ago

Is NamespaceConfig still relevant (https://github.com/jaegertracing/jaeger/blob/main/plugin/storage/cassandra/options.go#L70-L74)? Or can we replace it with the Configuration type?

mahadzaryab1 commented 5 days ago

@yurishkuro This is ready for another look. The patch code coverage is failing because of a couple of lines which were modified that I'm assuming weren't tested before. Should I add some tests for NewCluster?

yurishkuro commented 4 days ago

migration doc not readable

mahadzaryab1 commented 4 days ago

@yurishkuro thank you so much for writing the tests! the linter was failing so I just fixed that. Auto merge was disabled so will need to be enabled or merged in again.

mahadzaryab1 commented 4 days ago

migration doc not readable

sorry about that! just opened the access up

mahadzaryab1 commented 4 days ago

I think we have a one-off failure for the ES test. I think a rerun of the CI should fix it since we didn't touch anything ES related in this PR.

jaegertracing / jaeger