Skip to content
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

Metrics terminology: Grouping instruments as (opposed to Adding instruments) #625

Closed
jmacd opened this issue May 28, 2020 · 9 comments · Fixed by #983
Closed

Metrics terminology: Grouping instruments as (opposed to Adding instruments) #625

jmacd opened this issue May 28, 2020 · 9 comments · Fixed by #983
Assignees
Labels
area:sdk Related to the SDK priority:p1 Highest priority level release:required-for-ga Must be resolved before GA release, or nice to have before GA spec:metrics Related to the specification/metrics directory

Comments

@jmacd
Copy link
Contributor

jmacd commented May 28, 2020

The current metrics spec uses the term "Additive" to describe the Counter, UpDownCounter, SumObserver, and UpDownSumObserver instruments. This term was derived as follows: the input to these instruments convey a sum that is of primary interest, whether expressed as the change (synchronous case) or as the sum itself (synchronous case).

The other instruments have not had a proper term applied. Sometimes "non-additive", these instruments recognize individual events cannot be simply added, that they are averaged instead of added. This proposal suggests the term Collective to refer to these instruments, because:

Examples of use:

  • Collective instruments generally cost more than Additive instruments
  • Collective instruments are used to capture a distribution of values
  • The default aggregator for Collective instruments is MinMaxSumCount
  • The legacy term "Gauge" usually implies a Collective instrument
  • Add AbsoluteValueRecorder instrument #608 proposes to add a new Collective instrument.
@jmacd jmacd added the spec:metrics Related to the specification/metrics directory label May 28, 2020
@jkwatson
Copy link
Contributor

The thing that bugs me a little bit with both "Additive" and "Collective" is that they sound very "static", rather than dynamic. Somehow, a more active form like "Collecting" somehow evokes the meaning a little closer to me. What would you say to "Adding" and "Collecting", using the gerund forms, rather than static-evoking adjectives?

All that being said, if we clearly define this in the spec glossary, I think this would be fine, and is definitely better than "non-additive".

@jmacd
Copy link
Contributor Author

jmacd commented May 29, 2020

I think you're right about preferring a gerund form. But this also leads me not to prefer "Collecting" and to think of "Grouping". Collecting is a loaded term for us, given the collector. These instruments form groups of measurements, whereas the other instruments add measurements.

Example uses:

  • Counter is an Adding Instrument
  • ValueRecorder is a Grouping Instrument
  • Grouping instruments are more expensive than Adding instruments

The particular point that makes me agree strongly relates to #608. In that case, the proposal is to add a new Grouping instrument that accepts Additive inputs. I want to use Additive to describe inputs, Adding to describe instruments.

@jkwatson
Copy link
Contributor

/me breaks out his inner thesaurus...
Grouping, Collating, Aggregating, Accumulating, Accreting

I'm ok with Adding/Grouping.

Interesting to have more static words for inputs... I think I like it, but I need to mull it over in my head a bit.

@jmacd jmacd changed the title Metrics terminology: Collective instruments as (opposed to Additive instruments) Metrics terminology: Grouping instruments as (opposed to Adding instruments) May 29, 2020
@jmacd
Copy link
Contributor Author

jmacd commented May 29, 2020

An additive input is defined as one that semantically supports addition. Of course, numbers unconditionally support addition in the mathematical sense, but semantically there is nothing you can add to an individual measurement to transform it into a different individual measurement. You can group two individual measurements, but you can't add them.

However if the inputs to a grouping instrument are additive, then the sum is additive. I think it's time to decide on the merits of #608, this terminology will be helpful in the specification but ultimately doesn't impact the API choices we make.

@carlosalberto carlosalberto added the area:sdk Related to the SDK label Jun 26, 2020
@jmacd jmacd added the release:required-for-ga Must be resolved before GA release, or nice to have before GA label Jun 29, 2020
@mtwo mtwo added this to Required for GA in GA Burndown Jun 29, 2020
@mtwo mtwo added this to Required for GA, needs action. Add label:release:required-for-ga to issues and PRs when moving them to this column. in GA Spec Burndown Jun 29, 2020
@dashed
Copy link

dashed commented Jul 20, 2020

Unsure if this is the right issue to ask this, but is it safe to sample "non-additive" metrics? From reading the definitions, it seems like we should be able to. I'm unsure if this is already clarified elsewhere.

@jmacd
Copy link
Contributor Author

jmacd commented Jul 21, 2020

@dashed I'd like to direct you to a few open PRs where this question is under discussion.

open-telemetry/opentelemetry-proto#159 is discussing a sample_count field in the raw value message struct. This field would be available and meaningful to both ADDING and GROUPING measurements to indicate a sample weight.

open-telemetry/opentelemetry-proto#168 is discussing the protocol change where GROUPING and ADDING kinds are introduced to OTLP.

The RawValue sample_count field applies differently in these cases:

  • For ADDING data values, sample_count is a multiplier to the value being counted
  • For GROUPING data values, sample_count is a multiplier to the count of the individual value

There is an algorithm or two that we will need to compute exemplars with meaningful sample count, but the answer to your question is that yes, we should be able to. Thanks!

@dashed
Copy link

dashed commented Jul 21, 2020

@jmacd thanks! I'll follow those two PRs you've linked.

@andrewhsu
Copy link
Member

discussed at the spec sig mtg today and initially assigning to @jmacd

@jmacd
Copy link
Contributor Author

jmacd commented Aug 19, 2020

These terms have stood up very well. We have used the term "Structure" to describe this categorization. Here are example uses:

  • Instruments with adding structure are typically aggregated by a Sum aggregator
  • Instruments with grouping structure are typically aggregated by a Histogram aggregator
  • ValueRecorder and ValueObserver have grouping structure
  • The remaining instruments have adding structure
  • The adding vs. grouping structure distinction can be thought of as "things you add" vs. "things you average"
  • Adding structure timeseries semantically describe a sum
  • Grouping structure timeseries semantically describe a distribution.

@jmacd jmacd closed this as completed Aug 19, 2020
GA Spec Burndown automation moved this from Required for GA, needs action. Add label:release:required-for-ga to issues and PRs when moving them to this column. to Required for GA, done Aug 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:sdk Related to the SDK priority:p1 Highest priority level release:required-for-ga Must be resolved before GA release, or nice to have before GA spec:metrics Related to the specification/metrics directory
Projects
No open projects
GA Burndown
  
Required for GA; add release:required...
GA Spec Burndown
  
Required/Allowed for GA, resolved.
Development

Successfully merging a pull request may close this issue.

6 participants