Skip to content

Clarify Metric data type#2106

Merged
jmacd merged 8 commits intoopen-telemetry:mainfrom
reyang:reyang/metrics-sdk-clarify-metric-type
Nov 16, 2021
Merged

Clarify Metric data type#2106
jmacd merged 8 commits intoopen-telemetry:mainfrom
reyang:reyang/metrics-sdk-clarify-metric-type

Conversation

@reyang
Copy link
Copy Markdown
Member

@reyang reyang commented Nov 5, 2021

Fixes #2073.

This change adds extra clarification, it doesn't change the spec requirement.

@reyang reyang requested review from a team November 5, 2021 19:31
@reyang reyang added area:sdk Related to the SDK spec:metrics Related to the specification/metrics directory labels Nov 5, 2021
Copy link
Copy Markdown
Member

@cijothomas cijothomas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

reyang and others added 2 commits November 5, 2021 19:39
Co-authored-by: Cijo Thomas <cithomas@microsoft.com>
Copy link
Copy Markdown
Contributor

@MrAlias MrAlias left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This tries to clarify the types that implementations can use, but I think the original issue that this was opened to resolve is that the term Metric itself is not well defined. Here in the specification we are using the term to describe a group of MetricPoints, but above it also describes a "metric time series". This seems like a overloading of the term.

Additionally, above we refer to Aggregated Metrics above, but it seems to be referring to the same conceptual thing we a describing here.

I think adding these changes can help clarify implementation details for implementers, but we should also be consistent in our use of terms. I would recommend using Aggregated Metrics here if they are the same as above, or maybe instead just say "a batch of MetricPoint groups".

@reyang
Copy link
Copy Markdown
Member Author

reyang commented Nov 15, 2021

I think adding these changes can help clarify implementation details for implementers, but we should also be consistent in our use of terms. I would recommend using Aggregated Metrics here if they are the same as above, or maybe instead just say "a batch of MetricPoint groups".

I've updated the wording, PTAL.

@reyang reyang force-pushed the reyang/metrics-sdk-clarify-metric-type branch from a405c96 to 7eeede9 Compare November 15, 2021 18:17
@jmacd jmacd merged commit e2a4f05 into open-telemetry:main Nov 16, 2021
@reyang reyang deleted the reyang/metrics-sdk-clarify-metric-type branch November 17, 2021 03:59
carlosalberto pushed a commit to carlosalberto/opentelemetry-specification that referenced this pull request Oct 31, 2024
* clarify Metric data type

* minor wording adjustment

* Update specification/metrics/sdk.md

Co-authored-by: Cijo Thomas <cithomas@microsoft.com>

* rewrap

* improve wording

Co-authored-by: Cijo Thomas <cithomas@microsoft.com>
schmikei pushed a commit to schmikei/opentelemetry-specification that referenced this pull request Apr 17, 2025
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 spec:metrics Related to the specification/metrics directory

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Clarification on model metric of exporter.export(batch)

9 participants