[chore][tests] Clean up integration tests to remove archive reader / writer #6625

mahadzaryab1 · 2025-01-27T23:48:41Z

Which problem is this PR solving?

Towards Phase out the distinction between primary and archive storage #6065

Description of the changes

This PR cleans up the integration test suite for jaeger-v1 by removing the archive related fields for all tests since they were being skipped anyway, with the exception of elasticsearch.
Since the only storage that has a difference between primary and archive storage is ElasticSearch, this PR moves the archive trace test to elasticsearch_test.go.

How was this change tested?

CI

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: npm run lint and npm run test

Signed-off-by: Mahad Zaryab <mahadzaryab1@gmail.com>

mahadzaryab1 · 2025-01-27T23:49:27Z

plugin/storage/integration/cassandra_test.go

-	af := s.initializeCassandraFactory(t, []string{
-		"--cassandra-archive.keyspace=jaeger_v1_dc1_archive",
-		"--cassandra-archive.enabled=true",
-		"--cassandra-archive.servers=127.0.0.1",
-		"--cassandra-archive.basic.allowed-authenticators=org.apache.cassandra.auth.PasswordAuthenticator",
-		"--cassandra-archive.password=" + password,
-		"--cassandra-archive.username=" + username,
-	}, cassandra.NewArchiveFactory)


@yurishkuro thoughts on keeping vs removing this? we were initializing the archive span reader / writer but never actual ran the archive test

codecov · 2025-01-27T23:56:32Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.92%. Comparing base (9d3a516) to head (0305cf2).
Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6625      +/-   ##
==========================================
- Coverage   95.97%   95.92%   -0.06%     
==========================================
  Files         365      365              
  Lines       20616    20616              
==========================================
- Hits        19787    19776      -11     
- Misses        630      640      +10     
- Partials      199      200       +1

Flag	Coverage Δ
badger_v1	`9.92% <ø> (ø)`
badger_v2	`1.84% <ø> (ø)`
cassandra-4.x-v1-manual	`14.93% <ø> (-0.16%)`	⬇️
cassandra-4.x-v2-auto	`1.83% <ø> (ø)`
cassandra-4.x-v2-manual	`1.83% <ø> (ø)`
cassandra-5.x-v1-manual	`14.93% <ø> (-0.16%)`	⬇️
cassandra-5.x-v2-auto	`1.83% <ø> (ø)`
cassandra-5.x-v2-manual	`1.83% <ø> (ø)`
elasticsearch-6.x-v1	`19.30% <ø> (ø)`
elasticsearch-7.x-v1	`19.38% <ø> (ø)`
elasticsearch-8.x-v1	`19.56% <ø> (ø)`
elasticsearch-8.x-v2	`1.84% <ø> (ø)`
grpc_v1	`10.90% <ø> (-0.15%)`	⬇️
grpc_v2	`7.89% <ø> (ø)`
kafka-3.x-v1	`10.21% <ø> (ø)`
kafka-3.x-v2	`1.84% <ø> (ø)`
memory_v2	`1.84% <ø> (ø)`
opensearch-1.x-v1	`19.44% <ø> (ø)`
opensearch-2.x-v1	`19.44% <ø> (ø)`
opensearch-2.x-v2	`1.84% <ø> (ø)`
tailsampling-processor	`0.48% <ø> (ø)`
unittests	`94.80% <ø> (+0.01%)`	⬆️

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.

plugin/storage/integration/elasticsearch_test.go

Signed-off-by: Mahad Zaryab <mahadzaryab1@gmail.com>

yurishkuro · 2025-01-29T00:06:20Z

plugin/storage/integration/elasticsearch_test.go

+	tID := model.NewTraceID(uint64(11), uint64(22))
+	expected := &model.Span{
+		OperationName: "archive_span",
+		StartTime:     time.Now().Add(-maxSpanAge * 5).Truncate(time.Microsecond),


maxSpanAge is a constant in this file, but it is not passed as a configuration to the storage factory

let's add in a separate PR

@yurishkuro its just using the default - https://github.com/jaegertracing/jaeger/blob/main/plugin/storage/es/options.go#L403

I know, but it's disassociated from that default. We should pass max age as parameter to the factory here.

Remove Archive Integration Tests For Unsupported Factories

41d3614

Signed-off-by: Mahad Zaryab <mahadzaryab1@gmail.com>

mahadzaryab1 commented Jan 27, 2025

View reviewed changes

mahadzaryab1 added the changelog:ci Change related to continuous integration / testing label Jan 27, 2025

yurishkuro reviewed Jan 28, 2025

View reviewed changes

plugin/storage/integration/elasticsearch_test.go Show resolved Hide resolved

yurishkuro reviewed Jan 28, 2025

View reviewed changes

plugin/storage/integration/elasticsearch_test.go Show resolved Hide resolved

Remove GRPC Archive Server And Fix Lint

188965d

Signed-off-by: Mahad Zaryab <mahadzaryab1@gmail.com>

mahadzaryab1 marked this pull request as ready for review January 28, 2025 02:34

mahadzaryab1 requested a review from a team as a code owner January 28, 2025 02:34

mahadzaryab1 requested a review from albertteoh January 28, 2025 02:34

dosubot bot added area/storage storage/elasticsearch labels Jan 28, 2025

mahadzaryab1 requested a review from yurishkuro January 28, 2025 02:36

mahadzaryab1 mentioned this pull request Jan 28, 2025

Upgrade storage integration test: Use V2 Archive ReaderWriter #6489

Merged

6 tasks

mahadzaryab1 and others added 2 commits January 28, 2025 17:56

Update Documentation

ad88071

Signed-off-by: Mahad Zaryab <mahadzaryab1@gmail.com>

Merge branch 'main' into integration-test-cleanup

0305cf2

yurishkuro approved these changes Jan 29, 2025

View reviewed changes

yurishkuro reviewed Jan 29, 2025

View reviewed changes

yurishkuro approved these changes Jan 29, 2025

View reviewed changes

yurishkuro merged commit 4ac036b into jaegertracing:main Jan 29, 2025
55 checks passed

mahadzaryab1 deleted the integration-test-cleanup branch April 2, 2025 17:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[chore][tests] Clean up integration tests to remove archive reader / writer #6625

[chore][tests] Clean up integration tests to remove archive reader / writer #6625

Uh oh!

mahadzaryab1 commented Jan 27, 2025 •

edited

Loading

Uh oh!

mahadzaryab1 Jan 27, 2025

Uh oh!

yurishkuro Jan 28, 2025

Uh oh!

codecov bot commented Jan 27, 2025 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

yurishkuro Jan 29, 2025

Uh oh!

yurishkuro Jan 29, 2025

Uh oh!

mahadzaryab1 Jan 29, 2025

Uh oh!

yurishkuro Jan 29, 2025

Uh oh!

Uh oh!

Uh oh!

[chore][tests] Clean up integration tests to remove archive reader / writer #6625

[chore][tests] Clean up integration tests to remove archive reader / writer #6625

Uh oh!

Conversation

mahadzaryab1 commented Jan 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Which problem is this PR solving?

Description of the changes

How was this change tested?

Checklist

Uh oh!

mahadzaryab1 Jan 27, 2025

Choose a reason for hiding this comment

Uh oh!

yurishkuro Jan 28, 2025

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Jan 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

yurishkuro Jan 29, 2025

Choose a reason for hiding this comment

Uh oh!

yurishkuro Jan 29, 2025

Choose a reason for hiding this comment

Uh oh!

mahadzaryab1 Jan 29, 2025

Choose a reason for hiding this comment

Uh oh!

yurishkuro Jan 29, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

mahadzaryab1 commented Jan 27, 2025 •

edited

Loading

codecov bot commented Jan 27, 2025 •

edited

Loading