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

Add optional data point flags to all metrics data points, support staleness markers #316

Merged
merged 5 commits into from
Aug 9, 2021

Conversation

jmacd
Copy link
Contributor

@jmacd jmacd commented Jun 23, 2021

Adds a bit to represent stale points they way Prometheus does when scraping for metrics yields no results. This avoids the use of IEEE 754 NaN values in data points that do not necessarily have a floating point field that can be used.

Resolves #315.

Related: open-telemetry/opentelemetry-specification#1078

Partial copy from #310.
Resolves #309.

@jmacd jmacd requested review from a team as code owners June 23, 2021 07:26
@jmacd jmacd changed the title Add optional data point flags to all metrics data points Add optional data point flags to all metrics data points, support staleness markers Jun 23, 2021
@jmacd
Copy link
Contributor Author

jmacd commented Jul 12, 2021

@bogdandrutu @open-telemetry/specs-metrics-approvers Please take a look.

Copy link
Member

@bogdandrutu bogdandrutu left a comment

Choose a reason for hiding this comment

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

LGTM, except one question:
In protobuf is the enum equivalent with an "int32"? If not how would we intend to add new values? An enum has the property that only one value can be sent, if we add a new bit "foo" then are we going to change them to 4 new values NO_FOO_NONE, NO_FOO_NO_RECORD, FOO_NONE, etc.?

Do we need the value in the points to be uint32 and the enum be the bitset mask? Probably I am missing something...

@bogdandrutu
Copy link
Member

@jmacd also this needs a rebase

@jmacd
Copy link
Contributor Author

jmacd commented Aug 5, 2021

In protobuf is the enum equivalent with an "int32"?

Yes, see https://developers.google.com/protocol-buffers/docs/proto3#enum

@jmacd
Copy link
Contributor Author

jmacd commented Aug 6, 2021

I switched this to uint32 based on @bogdandrutu's feedback.

Copy link
Member

@bogdandrutu bogdandrutu left a comment

Choose a reason for hiding this comment

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

Last comment :)

opentelemetry/proto/metrics/v1/metrics.proto Show resolved Hide resolved
@bogdandrutu bogdandrutu merged commit e57e849 into open-telemetry:main Aug 9, 2021
@jmacd
Copy link
Contributor Author

jmacd commented Aug 9, 2021

🎉

carlosalberto pushed a commit to open-telemetry/opentelemetry-specification that referenced this pull request Aug 8, 2024
Fixes #4053

## Changes

This PR extends the metric data model with data point flags, which were
added to the protocol in
open-telemetry/opentelemetry-proto#316.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for Prometheus staleness markers Should we use bit fields to flag data point properties?
9 participants