grafana / k6

A modern load testing tool, using Go and JavaScript - https://k6.io
GNU Affero General Public License v3.0
25.15k stars 1.25k forks source link

Use a dedicated option instead of MaxMetricSamplesPerPackage #3156

Closed codebien closed 1 year ago

codebien commented 1 year ago

What

As defined by the code's comment below, we should use a better name that matches the purpose for MaxMetricSamplesPerPackage in cloudv2.

cloudv2 uses MaxMetricSamplesPerPackage for defining the max number of time series in a single flush batch. Instead, the current option defines the number of k6 samples.

https://github.com/grafana/k6/blob/1df72a0a1b23d7b0827813f6b709c7ad7bb4193b/cloudapi/config.go#L34-L36

https://github.com/grafana/k6/blob/1df72a0a1b23d7b0827813f6b709c7ad7bb4193b/output/cloud/expv2/output.go#L110-L112

Why

MaxMetricSamplesPerPackageis inherited from v1 but we should use a cleaner and dedicated field.

Suggestion

Add a dedicated new field for defining the max number of series in a single flush batch.

imiric commented 1 year ago

Whatever the new name is, it shouldn't reference "packages", which I'm not sure what they are. I think this was meant to be "per (network) packet", but even that would be wrong, as HTTP is not concerned with TCP/IP packets, and requests could be split over several packets. So "per (HTTP) request" would be more accurate, or since the v2 output now deals with protobuf, maybe "per (protobuf) message", or more generically "per payload", would be better.