elastic / elastic-integration-corpus-generator-tool

Command line tool used for generating events corpus dynamically given a specific integration
Other
21 stars 12 forks source link

Counter support #129

Closed aspacca closed 6 months ago

aspacca commented 7 months ago

Adding counter config for long and double type. With counter: true, ever-increasing values will be generated. counter: true and range.min/max config cannot be set at the same time and will result in an error stopping the generator. fuzziness is applied if defined.

Related to https://github.com/elastic/elastic-integration-corpus-generator-tool/issues/124

aspacca commented 7 months ago

@ruflin , @aliabbas-elastic . counters can be reset. the case is not covered in this PR.

I see a few alternatives for that:

  1. having the possibility to enable/disable just totally random occurrences of the reset.
  2. having the possibility to define the probability that the counter is reset every time a new value is generated
  3. having the possibility to specify the number of events after which the counter must be reset

If 1. is enough to start from it's fine for me to add directly in this PR.

If and when we will add at least one of the three I suggest to change the counter config type to a string. The string value will drive all three cases.

If we decided to add in a separated PR this will result in a breaking change. If we do not want to have a breaking change, we must add instead another separated config to drive the reset cases.

Just giving context in order for you to decide :)

PS: indeed adding cardinality to a field with a counter: true is the equivalent of 3. This is a side effect of the behaviour of the current cardinality implementation. We already planned to change this behaviour. Once we implement the new behaviour, counter: true and cardinality config cannot be set at the same time and will result in an error stopping the generator.

aspacca commented 7 months ago

@ruflin , @aliabbas-elastic , sorry, after some considerations I made there is no value in having counter also configuring the reset, it is indeed way better to have a separated config.

this excludes at all dealing with a breaking change, so there's no benefit/tradeoff to consider to cover reset in this PR.

I will add to https://github.com/elastic/elastic-integration-corpus-generator-tool/issues/124 a proposal for a reset configuration, covering all the 3 cases mentioned.

ruflin commented 7 months ago
lalit-satapathy commented 6 months ago

I like the idea of counter reset but it is not clear to me if it is a top priority. Is there currently an issue we have if no reset happens? For the reset, agree it should be a separate config. But I would skip it for now. 


Agree this can be taken up later, I am not sure, where we need such a case immediately.