[jaeger-v2] add badger e2e integration test

[akagami-harsh]

Which problem is this PR solving?

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

Description of the changes

• added badger e2e integration test

How was this change tested?

• tested locally

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

[codecov]

Codecov Report

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

Project coverage is 95.22%. Comparing base (afc4b2f) to head (c2fcfe3).

Files	Patch %	Lines
...r/internal/integration/storagecleaner/extension.go	94.87%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5355      +/-   ##
==========================================
- Coverage   95.22%   95.22%   -0.01%     
==========================================
  Files         343      346       +3     
  Lines       16813    16890      +77     
==========================================
+ Hits        16010    16083      +73     
- Misses        606      608       +2     
- Partials      197      199       +2     

Flag	Coverage Δ
badger	12.52% <29.87%> (+2.04%)	⬆️
cassandra-3.x	18.38% <ø> (ø)
cassandra-4.x	18.38% <ø> (ø)
elasticsearch-5.x	20.89% <ø> (-0.02%)	⬇️
elasticsearch-6.x	20.87% <ø> (-0.04%)	⬇️
elasticsearch-7.x	20.98% <ø> (+0.02%)	⬆️
elasticsearch-8.x	21.12% <ø> (-0.05%)	⬇️
grpc	14.49% <29.87%> (-0.06%)	⬇️
kafka	10.17% <ø> (ø)
opensearch-1.x	20.99% <ø> (-0.02%)	⬇️
opensearch-2.x	21.01% <ø> (ø)
unittests	91.64% <67.94%> (-0.12%)	⬇️

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]

Some tests are failing, but pass when run individually.

[yurishkuro]

Tests may be failing because we do not provide a mechanism to clean up the data between runs. Simply reinitializing the remote storage (by restarting the binary) worked for the memstore, but won't work for persistent backends.

cc @james-ryans

[james-ryans]

Tests may be failing because we do not provide a mechanism to clean up the data between runs. Simply reinitializing the remote storage (by restarting the binary) worked for the memstore, but won't work for persistent backends.

cc @james-ryans

Yes, it is because the span data are persisted between test cases. In this case for badger, restarting the binary is the only way for now.

[yurishkuro]

Yes, it is because the span data are persisted between test cases. In this case for badger, restarting the binary is the only way for now.

I don't think restarting the binary would help, because the data will still be in the file system (we're using persistent file system as I can see from the config, not the ephemeral mode). We will have similar problems with Cassandra and ES/OS. However, in the case of read DBs we could actually have a separate connection opened from the test that performs a cleanup, but Badger is not a sharable DB, only one process can access the data. So here are some options:

1. we could use ephemeral mode for Badger tests. It's probably the easiest solution, but not ideal since I would prefer e2e tests to be more realistic and people use Badger for its file storage, not for ephemeral mode.
2. we could implement a backdoor specifically in the Badger factory and expose it as an optional endpoint in the remote storage binary (e.g. POST /purge or even DELETE /purge )

I'd say let's reduce the scope and go with (1) for now. This is what the existing integration tests were doing, I believe, since unlike ES/Cassandra tests that were actually deleting data, the Badger test was only closing the factory (so I assume it was using ephemeral mode).

But @james-ryans , didn't your implementation of cleanup already kill & restart the remote storage binary? Or was it only done at the whole test suite scope, not for each test? Is it going to be too expensive to restart for each test?

[akagami-harsh]

tests are passing in my local machine but failing here.

[yurishkuro]

extension.go is having many coverage misses, do you want to add tests?

https://github.com/jaegertracing/jaeger/pull/5355/checks?check_run_id=24174925354

[yurishkuro]

When we run regular integration tests we collect code coverage "from everything" which allows us to test certain code paths that cannot be covered purely with unit tests (ie code that needs db connection). Something tells me that perhaps we don't do the same for e2e tests? Otherwise some of the code paths in the extension would've been covered.

[yurishkuro]

we're on a final stretch, let's finish it @akagami-harsh

[akagami-harsh]

nish it @akagami-harsh

Yes, apologies for the late response, i had an exam yesterday. i am working on it now