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.

Sign up for GitHub

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

Jump to bottom

Add metric.metadata for supporting additional metadata on metrics #514

Merged
merged 9 commits into from
Feb 2, 2024
Merged

Add metric.metadata for supporting additional metadata on metrics #514

merged 9 commits into from
Feb 2, 2024

Conversation

dashpole
Copy link
Contributor

@dashpole dashpole commented Nov 29, 2023 •
edited
Loading

Part of open-telemetry/opentelemetry-specification#3058

Alternative to #496.

Language is borrowed from scope attributes, with the addition of: Attributes are non-identifying.

The primary motivation is to support the prometheus unknown type without explicitly adding it to the data model.

This provides a place for additional information about metrics to be provided that isn't associated with individual data points. It is non-identifying to ensure it can be safely ignored by exporters if they don't support them.

Examples of problems this could be used to solve (not proposed, just examples):

  • Support additional types: prometheus.type = "unknown", or statsd.type = "set"
  • Support vendor-specific features: gcp.value_type = "bool"
    • GCP currently solves this by using a "special" unit.
  • Support additional features on top-of OTel: e.g. k8s.stability_level = "beta"
    • K8s currently encodes stability level in the description.

Alternatives:

  • Add gauge subtype for prometheus unknown type #496
    • Pros: An enum is a smaller change. Additional attributes may cause confusion if users are looking for data point attributes.
    • Cons: Enum solves fewer use-cases, and would likely need follow-up changes for other additional types. Adding attributes (proposed here) would likely mean fewer changes to the proto in the future to support new classes of uses-cases.
  • string instead of `repeated opentelemetry.proto.common.v1.KeyValue.
    • Cons: A string would make it harder to use
  • attributes on Gauge instead of on Metric.
    • Pros: Meets primary use-case (prometheus unknown)
    • Cons: Likely (?) to be extended to other data types eventually, which would result in a slightly more complex protocol.
  • attributes on DataPoint instead of on Metric.
    • Cons: Duplicates functionality of existing attributes field of DataPoint, and would create confusion over which to use.

@bogdandrutu @jack-berg @jsuereth @jmacd @tigrannajaryan

@dashpole dashpole requested review from a team November 29, 2023 21:37
@jack-berg jack-berg mentioned this pull request Dec 1, 2023
Open
@dashpole @jack-berg
Update opentelemetry/proto/metrics/v1/metrics.proto
928e06b 
Co-authored-by: jack-berg <34418638+jack-berg@users.noreply.github.com>
@arminru arminru changed the title Add metric.attributes for supporting additional metadata on metrics Add metric.metadata_attributes for supporting additional metadata on metrics Dec 12, 2023
@arminru arminru changed the title Add metric.metadata_attributes for supporting additional metadata on metrics Add metric.metadata for supporting additional metadata on metrics Dec 12, 2023
@carlosalberto carlosalberto mentioned this pull request Jan 9, 2024
Merged
@dashpole dashpole mentioned this pull request Jan 10, 2024
Closed
@carlosalberto
Copy link
Contributor

It seems we are good to go, specially after 1.1.0 was released. @bogdandrutu maybe a last look?

@carlosalberto
Copy link
Contributor

I think we are ready to merge (briefly mentioned this in the Spec call a week or two ago, and no reviews ever since).

cc @bogdandrutu

@dashpole @arminru
Update opentelemetry/proto/metrics/v1/metrics.proto
2d23a97 
Co-authored-by: Armin Ruech <7052238+arminru@users.noreply.github.com>
@carlosalberto carlosalberto merged commit c451441 into open-telemetry:main Feb 2, 2024
15 checks passed
@dashpole dashpole mentioned this pull request Feb 7, 2024
Merged
5 tasks
@dashpole dashpole deleted the metric_attributes branch February 29, 2024 16:09
@tigrannajaryan tigrannajaryan mentioned this pull request Mar 21, 2024
Open
@carlosalberto carlosalberto mentioned this pull request Mar 25, 2024
Merged
@Kielek Kielek mentioned this pull request Apr 8, 2024
Merged
1 task
carlosalberto pushed a commit to open-telemetry/opentelemetry-specification that referenced this pull request Apr 8, 2024
@dashpole
Prometheus compatibility: represent Prometheus Info, StateSet and Unk…
ebc157f 
…nown-typed metrics in OTLP (#3868)

Fixes
#3058
Fixes
#1712
Fixes
#2409

## Changes

Use metric.metadata (added in
open-telemetry/opentelemetry-proto#514) to
support representing prometheus metric types in OTLP.
VinozzZ pushed a commit to VinozzZ/opentelemetry-proto that referenced this pull request Jun 21, 2024
@dashpole @VinozzZ
Add metric.metadata for supporting additional metadata on metrics (op…
27b5c20 
…en-telemetry#514)

* add metric.attributes for supporting additional metadata on metrics
VinozzZ pushed a commit to VinozzZ/opentelemetry-proto that referenced this pull request Jun 21, 2024
@dashpole @VinozzZ
Add metric.metadata for supporting additional metadata on metrics (op…
3cedf1f 
…en-telemetry#514)

* add metric.attributes for supporting additional metadata on metrics
VinozzZ pushed a commit to VinozzZ/opentelemetry-proto that referenced this pull request Jun 21, 2024
@dashpole @VinozzZ
Add metric.metadata for supporting additional metadata on metrics (op…
a45b1c4 
…en-telemetry#514)

* add metric.attributes for supporting additional metadata on metrics
VinozzZ pushed a commit to VinozzZ/opentelemetry-proto that referenced this pull request Jun 21, 2024
@dashpole @VinozzZ
Add metric.metadata for supporting additional metadata on metrics (op…
99806a4 
…en-telemetry#514)

* add metric.attributes for supporting additional metadata on metrics
carlosalberto pushed a commit to carlosalberto/opentelemetry-specification that referenced this pull request Oct 31, 2024
@dashpole
Prometheus compatibility: represent Prometheus Info, StateSet and Unk…
77eed44 
…nown-typed metrics in OTLP (open-telemetry#3868)

Fixes
open-telemetry#3058
Fixes
open-telemetry#1712
Fixes
open-telemetry#2409

## Changes

Use metric.metadata (added in
open-telemetry/opentelemetry-proto#514) to
support representing prometheus metric types in OTLP.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jack-berg jack-berg jack-berg approved these changes

@reyang reyang reyang approved these changes

@tigrannajaryan tigrannajaryan tigrannajaryan approved these changes

@arminru arminru arminru approved these changes

@jmacd jmacd jmacd approved these changes

@carlosalberto carlosalberto carlosalberto approved these changes

@jsuereth jsuereth jsuereth approved these changes

Assignees

@reyang reyang

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@dashpole @carlosalberto @jsuereth @jmacd @tigrannajaryan @arminru @reyang @jack-berg