[cmd/telemetrygen] Add ability to ensure uniqueness of timeseries within a time period #39935
Conversation
| numMetrics int // how many metrics the worker has to generate (only when duration==0) | ||
| enforceUnique bool // if true, the worker will generate unique timeseries | ||
| totalDuration time.Duration // how long to run the test for (overrides `numMetrics`) | ||
| limitPerSecond rate.Limit // how many metrics per second to generate |
There was a problem hiding this comment.
Couldn't we just have a limit of 1 here?
There was a problem hiding this comment.
The limit wouldn't allow for load-test levels of telemetry to be generated for backend systems without an astronomical number of binaries running. This creates new timeseries to ensure we're sending unique values within the provided time limit.
Somewhat answered this from your other comment, but the difference here is that we still want to be able to generate close to the same amount of load configurable by
Totally, updates incoming. |
|
I may have misunderstood the idea of |
…series' into fix/overriding-timeseries
|
I was looking at go's rate.Limiter functionality to see if it made sense, and I don't think it does since we aren't actually wanting to limit generation of timeseries, just to ensure that within X configurable amount of time, we're using a unique timeseries. This pattern seems to hold up well with my tests against a prom remote write backend ( telemetrygen -> collector -> prometheus compatible backend ) at 1s and 15s intervals now that the flag is configurable. This change does mean that telemetrygen just became a cardinality generator as well though, as setting a high rate with Possibly dangerous, but a good testing tool to have in our belt imo. |
|
@mx-psi Any additional feedback on this guy? |
|
@mx-psi @codeboten please review as codeowners. |
|
Hey all, I'm still here and open to additional feedback on this one. |
|
Sorry, I am a bit under the water currently but will take a look at this eventually |
|
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
|
Just checking in so this doesn't end up completely disappearing 😅 |
|
/remove-label stale |
|
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
|
/remove-label stale |
|
@mx-psi I know you've been overwhelmed, if there's anything I can take off your plate to help out with this one please let me know. I've been using this against our systems since this PR opened and it's been a killer help to ensure I don't get errors from the prometheus backend when sending via an otel collector. |
|
@codeboten would you please review? |
…hin a time period (open-telemetry#39935) #### Description Adds a new `timeBox` object which has the sole purpose of generating unique attributes per second. This allows for telemetrygen to not override it's own values when sending to backends. <!-- Issue number (e.g. open-telemetry#1234) or full URL to issue, if applicable. --> #### Link to tracking issue Fixes open-telemetry#39933 <!--Describe what testing was performed and which tests were added.--> #### Testing Additional test added to cover new functionality #### Documentation New Flag added along with help criteria --------- Co-authored-by: Pablo Baeyens <[email protected]>
Description
Adds a new
timeBoxobject which has the sole purpose of generating unique attributes per second. This allows for telemetrygen to not override it's own values when sending to backends.Link to tracking issue
Fixes #39933
Testing
Additional test added to cover new functionality
Documentation
New Flag added along with help criteria