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

Optional Metric Attributes #2077

Open
djaglowski opened this issue Oct 28, 2021 · 7 comments
Open

Optional Metric Attributes #2077

djaglowski opened this issue Oct 28, 2021 · 7 comments
Labels
area:semantic-conventions Related to semantic conventions help wanted Extra attention is needed release:allowed-for-ga Editorial changes that can still be added before GA since they don't require action by SIGs spec:metrics Related to the specification/metrics directory

Comments

@djaglowski
Copy link
Member

djaglowski commented Oct 28, 2021

A metric attribute is used to differentiate data points along a single dimension.

In some cases an attribute should be considered optional. In particular this is necessary for "usage" metrics (as defined here), but there may be other cases as well.

A metric with an optional attribute can be represented either as a set of data points, each with a different attribute value, or as a single data point without the attribute. In the latter case, the dimension is collapsed into a single "total" value, but the attribute itself should not be specified (i.e. should not take a value of "total" or similar). The omission of the attribute in the "total" case is necessary in order to support the notion that data points along a dimension should be meaningfully aggregatable. As a general rule, it follows that each resource should be characterized by a metric containing only "total" data points, or containing no "total" data points.

Additional context.

The OTel collector currently collects process metrics in two different cases. In one case, a "total" value is produced. In the other, values are differentiated by "state". The metric data model should support both use cases. This solution was proposed here and seconded here.

@djaglowski djaglowski added the spec:metrics Related to the specification/metrics directory label Oct 28, 2021
@tigrannajaryan tigrannajaryan changed the title Optional Attributes Optional Metric Attributes Oct 29, 2021
@reyang reyang added the area:semantic-conventions Related to semantic conventions label Nov 2, 2021
@reyang
Copy link
Member

reyang commented Nov 2, 2021

Disucssed during the 11/2/2021 Spec SIG Meeting:

For example:

Process CPU Time Process Id Machine Id
100.0s 1 A
80.0s 2 A
60.0s 3 B
70.0s 4 B
  1. If in a single metrics stream (bug, shouldn't happen):
  • 100, pid=1, mid=A
  • 80, pid=2, mid=A
  • 180, mid=A
  • 130, mid=B
  1. If in a single metrics stream (will happen, although we don't want it):
  • 100, pid=1, mid=A
  • 80, pid=2, mid=A
  • 130, mid=B

@jsuereth suggested that for case 2, we can treat the data as different metrics data streams, so we don't mix different dimensions.

@reyang if a SQL instrumentation library started with 4 dimensions, and later we decided to add another dimension, do we rename the instrumentation library or bump the major version? @jsuereth either way, but it should be a significant change.

@reyang reyang added the release:allowed-for-ga Editorial changes that can still be added before GA since they don't require action by SIGs label Nov 2, 2021
@djaglowski
Copy link
Member Author

Summarizing what I understood to be the decisions made in yesterday's SIG:

  1. The optionality of attributes is already implied in the data model. My understanding is that this is covered by the notion of spatial aggregation (i.e. An omitted label is equivalent to having performed a spatial aggregation along an unwanted dimension).
  2. We need to add some language to the semantic conventions, which establishes an expected minimum scope across which all data points should specify the same set of attributes.
    a. It's implicit that a individual time series should have a consistent set of attributes, since the set of attributes differentiates one time series from another.
    b. An instrumentation library should produce a consistent set of attributes for a given instrument.

@jsuereth, @bogdandrutu, @reyang, please correct me if I've missed or misinterpreted anything.

@tigrannajaryan
Copy link
Member

@jsuereth this is what I was referring to. This says that optional attributes are OK and area implied in the metric data model.
If this is true then the consequence of that is that the instrumentation is free to decide whether to exclude an attribute and then later decide to include it, which is equivalent to adding an attribute that didn't exists before.

@jsuereth
Copy link
Contributor

jsuereth commented Dec 1, 2021

@tigrannajaryan Optional attributes are OK, but there's a line here to pay attention to:

An instrumentation library should produce a consistent set of attributes for a given instrument.

This is in conflict with what's being proposed in the semantic-convention stability PR. It's ok across different resources or instrumentation libraries to have different optional attributes but not for the same instrumentation.

@tigrannajaryan
Copy link
Member

This is in conflict with what's being proposed in the semantic-convention stability PR. It's ok across different resources or instrumentation libraries to have different optional attributes but not for the same instrumentation.

Is this nuance enforceable in practice? I feel that it is going to be tricky to both communicate this nuance in the spec clearly and as a developer it will be also tricky to correctly follow the requirements of it.

In any case I modified the semantic-convention stability PR to disallow this, so we are good there, but I think more clarification is necessary here.

@jsuereth
Copy link
Contributor

jsuereth commented Dec 1, 2021

Is this nuance enforceable in practice? I feel that it is going to be tricky to both communicate this nuance in the spec clearly and as a developer it will be also tricky to correctly follow the requirements of it.

It's actually more that what we'd like to enforce is NOT enforceable in practice.

This nuance basically says:

  • Implementors of instrumentation should consistently use the same set of attributes.
  • If you use MORE THAN ONE instrumentation library, we have no (real) way to enforce consistency of attributes across them, but each should be internally consistent.

@tigrannajaryan
Copy link
Member

Implementors of instrumentation should consistently use the same set of attributes.

I think this is worth calling out directly somewhere. Otherwise we have too much implicit knowledge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:semantic-conventions Related to semantic conventions help wanted Extra attention is needed release:allowed-for-ga Editorial changes that can still be added before GA since they don't require action by SIGs spec:metrics Related to the specification/metrics directory
Development

No branches or pull requests

5 participants