[EPIC] Refactor Elasticsearch Exporter Tests

[lenaschoenburg]

Description

The elasticsearch-exporter integration tests are currently quite convoluted. Historically, the elasticsearch-exporter tests used to depend on zeebe-test which was removed in #8608. During the removal, some of the zeebe-test utilities ended up in the elasticsearch-exporter module where we also needed to add a lot of test scoped dependencies.

The main goals here were:

• Cover as much as possible via only unit tests. This gives us a faster feedback cycle when changing something - developers can focus on first fixing unit tests, which have smaller blast radius and faster execution, before even looking at integration tests.
• Narrow the scope of the integration tests as much as possible. This meant getting rid of the broker, workflow engine semantics, etc., and just testing the integration of the exporter instance with Elasticsearch.

In general, I wanted to keep the same coverage as before. I think the coverage is now higher, but that was not the original motivation.

Breakdown

• Refactor Elasticsearch Exporter Tests Spike #9162
• Refactor ProtocolFactory to ensure that it uses a fixed seed. #9315
• Refactor ProtocolFactory for convenience to assign a type parameter for the value type #9316
• Refactor ProtocolFactory to only generate positive values for objects of type long #9317
• Update the checkstyle configuration to allow suppressing specific checkstyle warnings via annotations #9318
• Add test implementations for the exporter-api #9319
• Refactor ElasticsearchClient, splitting into separate concerns for easier testing #9320
• Refactor the ElasticsearchExporter tests #9321
• Clean up ElasticsearchExporter test refactoring

[lenaschoenburg]

Currently

This is an overview of the current tests in the elasticsearch module. There is one class with unit tests and a couple more for integration tests.

Unit Tests in ElasticsearchExporterTest

• don't fail on open if elasticsearch is unreachable
• create index templates
• exporting respects enabled/disabled value and record types
• exceptions during flush are not swallowed
• position is updated on flush, also with bulk delay enabled
• configured prefix is validated

Integration Tests in Elasticsearch*IT

• records are exported even if elasticsearch is initially not available
• indexes are correctly configured (number of shards and replicas)
• bulk flush throws on invalid json
• repeated records are ignored and not exported more than once
• bulk flush occurs when memory limit is reached
• the exporter can authenticate with elasticsearch
• configuration values are validated

[lenaschoenburg]

Unit tests are looking better now but we can still make some improvements to the integration tests:

• Get rid of AbstractElasticsearchExporterIntegrationTestCase and the rules in io.camunda.zeebe.exporter.util.it
  • These are tech-debt from the now-removed zeebe-test module. We should have good alternatives for all of the rules, for example ClusteringRule
• Rewrite some tests as unit tests
  • ElasticsearchExporterConfigurationIT seems like a good candidate
• Split integration tests
  • Integration tests between the exporter and elasticsearch should stay in the elasticsearch-exporter module
  • End to end tests between the broker, exporter and elasticsearch should move to the qa module

[npepinpe]

Here's what I'm thinking of doing, in no special order. I'm adding a bit of refactoring to clean things up as well, but most likely that will be done towards the end. The end goal is that the tests should be mostly unit tests, and the integration tests living in the exporter module will only starting the exporter without the whole broker around them, and testing the integration of the exporter itself with Elastic. Again, without the broker. This should greatly reduce flakiness surrounding the tests, and additionally make them much simpler. There will still be some end to end tests in the QA module, but much less than what currently exists.

• Introduce a new exporter-tests module, which provides a simple test harness to test the exporter by itself without a broker. This will be similar to the old test harness, but with a much reduced API surface. This is also not meant to be a public module for now.
• Split up the exporter into smaller components to allow testing them separately without the complete test harness
• Split up the ElasticsearchExporterClient into smaller components for easier testing and maintainability
• Rewrite the integration tests to make use of the new test harness
• Delete the AbstractElasticsearchExporterIntegrationTestCase class
• Replace using the WorkloadGenerator here in favor of a simpler record factory. Using protocol-jackson, we can just have a factory which returns exactly one of each record. We can even test to make sure that a record for each known ValueType is returned. This greatly simplifies testing, but keeps the same coverage (and perhaps more, actually). This may require us to do Export missing records to ES #8337 as well. Doing this also allows us to export these records, deserialize them back from ES, and use plain equals() to compare them.

There might be more. I'll do a quick spike, then split it off in issues where applicable. Let me know if anything sounds crazy.

[npepinpe]

Alright, so the spike was accepted and we'll now start breaking it down and merging it piece by piece. This means the end result will be something similar to #9258

Here's the breakdown:

• Refactor ProtocolFactory to ensure that it uses a fixed seed.
• Refactor ProtocolFactory for convenience to assign a type parameter for the value type.
• Refactor ProtocolFactory to only generate positive values (>= 0) for objects of type long or Long.
• Update the checkstyle configuration to allow suppressing specific checkstyle warnings via annotations. At the same time we need to update the code quality workflow to install build-tools first, so that the latest checkstyle configuration is always picked up before running the rest of our checks.
• Add test implementations for the exporter-api.
• Refactor ElasticsearchClient, splitting into separate concerns for easier testing.
• Refactor the ElasticsearchExporter tests.
• Clean up (removing outdated utilities and so on)

/cc @saig0 - regarding the ProtocolFactory, I will spread the reviews so it's not just you reviewing this.