[FR] Possible package push improvements

[shedimon]

Hello everyone! I have some questions/proposals to improve the user experience when pushing metrics using this library. Some of them are not backward-compatible, so it's up to you to decide on these :)

• Changing the data type of extraLabels from the string expecting it to be properly formatted to map[string]string would allow to prepare the extra labels in the expected way without exposing these details and checking for the validity of the data
• Instead of adding each extra label to each metric on each push it is possible to pass these extra labels as extra_label query arg once when initializing, something like

  args := url.Values{}
  for label, value := range extraLabels {
  args.Add("extra_label", fmt.Sprintf("%s=%s", label, value))
  }
  pu.RawQuery = args.Encode()
  url := parsed.String()

• Adding some metrics for visibility could be useful for debug, something like

  metrics_push_interval_seconds
  metrics_push_total
  metrics_push_errors_total
  metrics_push_bytes_pushed_total
  metrics_push_duration_seconds
  metrics_push_block_size_bytes

[valyala]

@shedimon , thanks for the feature request!

The proposed metrics_push_* metrics are published starting from v1.21.0 - see e9b4bb1534f39252df9b249495c3211ceba204db for implementation details.

As for the proposed change for extraLabels arg, it is unlikely it will be implemented because of the following issues:

• This narrows down the ability to push metrics to external systems, since the extra_label query arg is supported only by VictoriaMetrics.
• While map[string]string type allows avoiding programmer errors with incorrect labels' formatting, this type may require more lines of code comparing to string type. Compare the following code snippets:

extraArgs := map[string]string{
  "instance": hostname,
  "job": "foobar",
}

extraArgs := fmt.Sprintf(`instance=%q,job=%q`, hostname, "foobar")

The fmt.Sprintf() approach is already recommended by metrics package for creating metric names with non-empty set of labels. So it looks logical to keep consistency with this approach for extraLabels arg as well.

[shedimon]

Sure, the most important thing for me were metrics, thanks a lot!

This narrows down the ability to push metrics to external systems, since the extra_label query arg is supported only by VictoriaMetrics.

I didn't know that there are systems that implement plaintext metrics import like VM does, so thought of this as a "VM only" feature, in that case it would have made more sense to me to use the query args :)

Thanks a lot for the swift implementation!

[valyala]

Closing the feature request as partially done. The remaining parts of the feature request won't be implemented because of reasons outlined here.