Skip to content
New issue

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

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

Already on GitHub? # to your account

ref(metrics): Add custom and more builtin units [INGEST-939] #1217

Merged
merged 8 commits into from
Apr 7, 2022

Conversation

jan-auer
Copy link
Member

@jan-auer jan-auer commented Mar 31, 2022

Adds a number of grouped builtin units and a "custom unit" variant.

Builtin Unit Groups

Units within the same group are considered convertible. Each group has a default canonical unit, for instance byte for information units. In the future, Relay can streamline incoming metrics by converting to the canonical unit within the respective group. The currently defined builtin units are:

  • Durationnanosecond / microsecond / millisecond / second / minute / hour / day / week
  • Informationbit / byte / kibibyte / mebibyte / gibibyte / tebibyte / pebibyte / exbibyte
  • Fractionpercent / ratio
  • Nonenone, "" (used for: counts)

Custom Units

For forward compatibility, clients can send custom unit names of up to 15 ASCII characters. Beyond ASCII lowercasing, these units do not count as convertible. An example for such a unit would be frame to track the frame rate of a user interface or game.

Follow-Ups

  • Define units for all extracted metrics.
  • Remove the unit field from the aggregation protocol and rely on units in the MRI.

Notes

Based on #1215, will be rebased once merged.
cc @sl0thentr0py

@jan-auer jan-auer self-assigned this Mar 31, 2022
relay-metrics/src/protocol.rs Outdated Show resolved Hide resolved
relay-metrics/src/protocol.rs Outdated Show resolved Hide resolved
relay-metrics/src/protocol.rs Show resolved Hide resolved
@jan-auer jan-auer changed the title ref(metrics): Add custom and more builtin units ref(metrics): Add custom and more builtin units [INGEST-939] Mar 31, 2022
@sl0thentr0py
Copy link
Member

gave it a sweep and lgtm overall!
exposing these on the envelope will be done separately I assume?

@jan-auer
Copy link
Member Author

exposing these on the envelope will be done separately I assume?

Correct, we'll have to add them to the event schema.

Base automatically changed from ref/metrics-mri to master April 7, 2022 17:39
@jan-auer jan-auer marked this pull request as ready for review April 7, 2022 17:47
@jan-auer jan-auer requested a review from a team April 7, 2022 17:47
@jan-auer jan-auer enabled auto-merge (squash) April 7, 2022 17:48
@jan-auer jan-auer disabled auto-merge April 7, 2022 17:48
@jan-auer jan-auer enabled auto-merge (squash) April 7, 2022 17:50
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants