[jaeger-v2] add cassandra e2e integration tests #5398

akagami-harsh · 2024-04-29T11:24:31Z

Which problem is this PR solving?

part of [jaeger-v2] Storage backend integration tests #5254

Description of the changes

added cassandra integration tests
added method to purge cassandra storage

How was this change tested?

some tests are failing

Checklist

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

Signed-off-by: Harshvir Potpose <[email protected]>

codecov · 2024-04-29T11:37:06Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.57%. Comparing base (13cbaed) to head (d0bd53c).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5398      +/-   ##
==========================================
+ Coverage   94.54%   94.57%   +0.03%     
==========================================
  Files         346      346              
  Lines       16947    16954       +7     
==========================================
+ Hits        16022    16035      +13     
+ Misses        724      717       -7     
- Partials      201      202       +1

Flag	Coverage Δ
badger_v1	`10.28% <0.00%> (-0.01%)`	⬇️
badger_v2	`6.56% <0.00%> (-0.01%)`	⬇️
cassandra-3.x	`?`
cassandra-3.x-v1	`18.12% <19.04%> (?)`
cassandra-3.x-v2	`6.31% <0.00%> (?)`
cassandra-4.x	`?`
cassandra-4.x-v1	`18.12% <19.04%> (?)`
cassandra-4.x-v2	`6.29% <0.00%> (?)`
elasticsearch-5.x	`5.90% <0.00%> (-0.01%)`	⬇️
elasticsearch-6.x	`5.90% <0.00%> (+<0.01%)`	⬆️
elasticsearch-7.x	`5.89% <0.00%> (-0.01%)`	⬇️
elasticsearch-8.x	`5.90% <0.00%> (+<0.01%)`	⬆️
grpc_v1	`12.59% <0.00%> (-0.04%)`	⬇️
grpc_v2	`11.52% <0.00%> (-0.01%)`	⬇️
kafka	`9.95% <0.00%> (-0.01%)`	⬇️
opensearch-1.x	`5.90% <0.00%> (?)`
opensearch-2.x	`5.89% <0.00%> (-0.01%)`	⬇️
unittests	`91.39% <100.00%> (+0.05%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

akagami-harsh · 2024-04-29T12:50:18Z

most of the test are passing just these FindTrace tests are failing :

"Tags_in_one_spot_-_Tags",
"Tags_in_one_spot_-_Logs",
"Tags_in_one_spot_-_Process",
"Tags_in_different_spots",

Signed-off-by: Harshvir Potpose <[email protected]>

.github/workflows/ci-cassandra.yml

scripts/cassandra-integration-test.sh

Signed-off-by: Harshvir Potpose <[email protected]>

akagami-harsh · 2024-04-30T20:22:11Z

most of the test are passing just these FindTrace tests are failing :
"Tags_in_one_spot_-_Tags",
"Tags_in_one_spot_-_Logs",
"Tags_in_one_spot_-_Process",
"Tags_in_different_spots",

i tried some fixes but couldn't fix these test. do you have any clue why only the tests with Tags.... are failing

Signed-off-by: Harshvir Potpose <[email protected]>

plugin/storage/cassandra/factory.go

Signed-off-by: Yuri Shkuro <[email protected]>

## Which problem is this PR solving? - #5398 (comment) ## Description of the changes - added purge for method for cassandra ## How was this change tested? - via integration tests ## 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` --------- Signed-off-by: Harshvir Potpose <[email protected]> Signed-off-by: Yuri Shkuro <[email protected]> Co-authored-by: Yuri Shkuro <[email protected]>

yurishkuro · 2024-05-04T00:18:22Z

cmd/jaeger/internal/integration/cassandra_test.go

+			SkipList: []string{
+				"Tags_+_Operation_name_+_Duration_range",
+				"Tags_+_Duration_range",
+				"Tags_+_Operation_name_+_max_Duration",
+				"Tags_+_max_Duration",
+				"Operation_name_+_Duration_range",
+				"Duration_range",
+				"max_Duration",
+				"Multiple_Traces",
+			},


I suggest defining this as a constant (CassandraSkippedTests) in v1 tests and reusing here.

added it in the integration.go

## Which problem is this PR solving? - jaegertracing#5398 (comment) ## Description of the changes - added purge for method for cassandra ## How was this change tested? - via integration tests ## 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` --------- Signed-off-by: Harshvir Potpose <[email protected]> Signed-off-by: Yuri Shkuro <[email protected]> Co-authored-by: Yuri Shkuro <[email protected]>

Signed-off-by: Harshvir Potpose <[email protected]>

akagami-harsh · 2024-05-04T17:13:04Z

the FindTrace tests are failing because the vType of vBinary tag is getting converted form BINARY to STRING which causes this error:

=== RUN   TestCassandraStorage/FindTraces/Tags_in_one_spot_-_Tags
    trace_compare.go:38: Expected and actual differ: [0].Spans[0].Tags[0].VType: 4 != 0
    trace_compare.go:38: Expected and actual differ: [0].Spans[0].Tags[0].VStr: "" != "AAAwOQ=="
    trace_compare.go:38: Expected and actual differ: [0].Spans[0].Tags[0].VBinary: []uint8[4] != []uint8[0]
    trace_compare.go:44: Actual traces: [{"spans":[{"trace_id":"AAAAAAAAAAAAAAAAAAAAAQ==","span_id":"AAAAAAAAAAI=","operation_name":"some-operation","references":null,"flags":0,"start_time":"2024-05-03T16:46:31.639875Z","duration":7000,"tags":[{"key":"blob","v_str":"AAAwOQ=="},{"key":"sameplacetag1","v_str":"sameplacevalue"},{"key":"sameplacetag2","v_type":2,"v_int64":123},{"key":"sameplacetag3","v_type":3,"v_float64":72.5},{"key":"sameplacetag4","v_type":1,"v_bool":true}],"logs":null,"process":{"service_name":"query01-service","tags":null}}],"process_map":null}]
    trace_compare.go:45: Expected traces: [{"spans":[{"trace_id":"AAAAAAAAAAAAAAAAAAAAAQ==","span_id":"AAAAAAAAAAI=","operation_name":"some-operation","references":[],"flags":0,"start_time":"2024-05-03T16:46:31.639875Z","duration":7000,"tags":[{"key":"blob","v_type":4,"v_binary":"AAAwOQ=="},{"key":"sameplacetag1","v_str":"sameplacevalue"},{"key":"sameplacetag2","v_type":2,"v_int64":123},{"key":"sameplacetag3","v_type":3,"v_float64":72.5},{"key":"sameplacetag4","v_type":1,"v_bool":true}],"logs":[],"process":{"service_name":"query01-service","tags":[]}}],"process_map":null}]

I don't know yet which part of code is causing it. so should i modify the trace comparison logic to handle this condition?

yurishkuro · 2024-05-04T17:55:17Z

We had the same issue with badger, and there is a flag in the main struct to skip binary tags

Signed-off-by: Harshvir Potpose <[email protected]>

yurishkuro · 2024-05-04T21:56:45Z

plugin/storage/cassandra/factory.go

@@ -90,6 +91,9 @@ func NewFactoryWithConfig(
 	}
 	f := NewFactory()
 	f.primaryConfig = &cfg
+	f.Options.Index.Tags = true
+	f.Options.Index.Logs = true
+	f.Options.Index.ProcessTags = true


this doesn't make sense to me - the objective of this function is to create a factory with externally provided configuration. Instead, we're setting some configuration properties directly - why can they not be set in the caller?

made index configurable and added above fields in config file

yurishkuro

lgtm aside from one question about config settings

Signed-off-by: Harshvir Potpose <[email protected]>

Signed-off-by: Yuri Shkuro <[email protected]>

yurishkuro · 2024-05-05T04:18:38Z

🎉 🎈

add cassandra integraion tests

a136015

Signed-off-by: Harshvir Potpose <[email protected]>

akagami-harsh requested a review from a team as a code owner April 29, 2024 11:24

akagami-harsh requested a review from yurishkuro April 29, 2024 11:24

fix

863bf3f

Signed-off-by: Harshvir Potpose <[email protected]>

akagami-harsh and others added 2 commits April 29, 2024 18:20

fix lint

15a1107

Signed-off-by: Harshvir Potpose <[email protected]>

Merge branch 'main' into cassandra

5d35264

yurishkuro reviewed Apr 30, 2024

View reviewed changes

.github/workflows/ci-cassandra.yml Show resolved Hide resolved

yurishkuro reviewed Apr 30, 2024

View reviewed changes

scripts/cassandra-integration-test.sh Outdated Show resolved Hide resolved

yurishkuro added the changelog:test Change that's adding missing tests or correcting existing tests label Apr 30, 2024

fix for open comments

6fa9324

Signed-off-by: Harshvir Potpose <[email protected]>

akagami-harsh and others added 2 commits May 1, 2024 01:54

fix merge conflicts

12874e2

Signed-off-by: Harshvir Potpose <[email protected]>

Merge branch 'main' into cassandra

449608a

Signed-off-by: Harshvir Potpose <[email protected]>

yurishkuro reviewed Apr 30, 2024

View reviewed changes

plugin/storage/cassandra/factory.go Show resolved Hide resolved

yurishkuro mentioned this pull request May 3, 2024

[jaeger-v2] Storage backend integration tests #5254

Closed

5 tasks

Merge branch 'main' into cassandra

5b876d8

Signed-off-by: Yuri Shkuro <[email protected]>

yurishkuro changed the title ~~[jaeger-v2] add cassandra e2e integraion tests~~ [jaeger-v2] add cassandra e2e integration tests May 3, 2024

yurishkuro added 2 commits May 3, 2024 18:07

Purge(_ context.Context)

79a4ee8

Signed-off-by: Yuri Shkuro <[email protected]>

fix

5572cc5

Signed-off-by: Yuri Shkuro <[email protected]>

akagami-harsh mentioned this pull request May 3, 2024

Add Purge method for cassandra #5414

Merged

4 tasks

Merge branch 'main' into cassandra

f79b2e1

yurishkuro reviewed May 4, 2024

View reviewed changes

Merge branch 'main' into cassandra

bf5c459

enable index config

9feaf9e

Signed-off-by: Harshvir Potpose <[email protected]>

akagami-harsh added 3 commits May 5, 2024 03:01

skip binary attribute

c755924

Signed-off-by: Harshvir Potpose <[email protected]>

fix

bb2a83d

Signed-off-by: Harshvir Potpose <[email protected]>

fix

2a4acae

Signed-off-by: Harshvir Potpose <[email protected]>

akagami-harsh force-pushed the cassandra branch from 3dc9541 to 2a4acae Compare May 4, 2024 21:34

yurishkuro reviewed May 4, 2024

View reviewed changes

make index configurable

bc07cd8

Signed-off-by: Harshvir Potpose <[email protected]>

yurishkuro mentioned this pull request May 5, 2024

[jaeger-v2] Rethink components configuration to align with OTEL #5229

Closed

16 tasks

yurishkuro added 3 commits May 4, 2024 23:27

add tests

c0ed049

Signed-off-by: Yuri Shkuro <[email protected]>

minimize dependency on private "servers" string

511b666

Signed-off-by: Yuri Shkuro <[email protected]>

remove confusing private "servers"

d0bd53c

Signed-off-by: Yuri Shkuro <[email protected]>

yurishkuro approved these changes May 5, 2024

View reviewed changes

yurishkuro merged commit 4fed4f4 into jaegertracing:main May 5, 2024
40 checks passed

yurishkuro mentioned this pull request May 5, 2024

jaeger-v2 e22 tests are unstable #5418

Closed

akagami-harsh deleted the cassandra branch May 5, 2024 05:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[jaeger-v2] add cassandra e2e integration tests #5398

[jaeger-v2] add cassandra e2e integration tests #5398

akagami-harsh commented Apr 29, 2024

codecov bot commented Apr 29, 2024 •

edited

Loading

akagami-harsh commented Apr 29, 2024

akagami-harsh commented Apr 30, 2024 •

edited

Loading

yurishkuro May 4, 2024

akagami-harsh May 4, 2024

akagami-harsh commented May 4, 2024 •

edited

Loading

yurishkuro commented May 4, 2024

yurishkuro May 4, 2024

akagami-harsh May 5, 2024

yurishkuro left a comment

yurishkuro commented May 5, 2024

[jaeger-v2] add cassandra e2e integration tests #5398

[jaeger-v2] add cassandra e2e integration tests #5398

Conversation

akagami-harsh commented Apr 29, 2024

Which problem is this PR solving?

Description of the changes

How was this change tested?

Checklist

codecov bot commented Apr 29, 2024 • edited Loading

Codecov Report

akagami-harsh commented Apr 29, 2024

akagami-harsh commented Apr 30, 2024 • edited Loading

yurishkuro May 4, 2024

Choose a reason for hiding this comment

akagami-harsh May 4, 2024

Choose a reason for hiding this comment

akagami-harsh commented May 4, 2024 • edited Loading

yurishkuro commented May 4, 2024

yurishkuro May 4, 2024

Choose a reason for hiding this comment

akagami-harsh May 5, 2024

Choose a reason for hiding this comment

yurishkuro left a comment

Choose a reason for hiding this comment

yurishkuro commented May 5, 2024

codecov bot commented Apr 29, 2024 •

edited

Loading

akagami-harsh commented Apr 30, 2024 •

edited

Loading

akagami-harsh commented May 4, 2024 •

edited

Loading