Skip to content

Tot events instead of tot size #94

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
May 17, 2023

Conversation

aspacca
Copy link
Contributor

@aspacca aspacca commented May 17, 2023

talking with different people consuming the package we found out that is preferred to indicate to total number of events to generate rather than the total size.
there are a couple of reasons mostly:

  1. current users have knowledge of how many events they want to generate, and find themselves to figure out to translate this information in terms of size instead
  2. internally we try to figure out back the number of events from the size: this is required to provide evenly distributed data in features we will want to add (like providing a field configuration definition of a period to generate data for in case of a date field): the way we do this is to rendere once the target template and divide its size by the total size requested. this indeed is a limit and it requires a better strategy (like rendering the template multiple times: most probably up to the highest cardinality) in order to achieve a meaningful approximation (currently for aws.ec2_metrics, for example, the tool can end up generating double the size requested because of a the non meaningful approximation).

considering the two points above we decided to just drop indicating a total size to generate in favour of a total number of events. beware that it is now possible, omitting the --tot-events flag or setting it to 0 to generate an infinite number of events

@aspacca aspacca requested a review from endorama May 17, 2023 06:34
@aspacca aspacca self-assigned this May 17, 2023
Copy link
Member

@endorama endorama left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

left a comment on default value for tot-events but PR looks good!

docs/usage.md Outdated

`package`, `dataset` and `version` are mandatory. `--tot-size` is mandatory.
`package`, `dataset` and `version` are mandatory. `--tot-events` is not mandatory and in case it is not provided an infinite number of events will be generated.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think of setting default tot-events to 1? I'm thinking about the principle of least surprise and helping with command discovery for new devs. An infinite stream would be a big surprise to me and not useful if I'm learning the tool or discovering its capabilities.
Creating 1 by default could also help when writing a template, when you generally want 1 event at a time for testing.

@aspacca aspacca merged commit 981dcdb into elastic:main May 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants