-
Notifications
You must be signed in to change notification settings - Fork 308
Add Exemplar support to Metrics proto #159
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
Changes from 3 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -44,6 +44,37 @@ message InstrumentationLibraryMetrics { | |
| repeated Metric metrics = 2; | ||
| } | ||
|
|
||
| // A representation of raw measurements, which can have statistical meaning based | ||
| // on how the measurement was sampled, as well as a span ID and trace ID of the | ||
| // active span when the measurement was recorded | ||
| message RawValue { | ||
| // The set of labels that were dropped by the aggregator, but recorded | ||
| // alongside the original measurement. Only labels that were dropped by the aggregator should be included | ||
| repeated opentelemetry.proto.common.v1.StringKeyValue labels = 1; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What is the relationship between these labels and the labels in the DataPoint:
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For exemplars these labels are only the labels not included in the DataPoint's labels. I would change this field to be
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @jmacd I know you tried to share the messages, can you help define the behavior here?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think you recommended calling these "dropped_labels". This sounds good to me. |
||
|
|
||
| // time_unix_nano is the exact time when this RawValue was recorded | ||
| // | ||
| // Value is UNIX Epoch time in nanoseconds since 00:00:00 UTC on 1 January 1970. | ||
| fixed64 time_unix_nano = 2; | ||
|
bogdandrutu marked this conversation as resolved.
|
||
|
|
||
| // Numerical value of the measurement that was recorded. Only one of these | ||
| // two fields is used for the data, based on MetricDescriptor.measurement_value_type | ||
| double double_value = 3; | ||
| int64 int64_value = 4; | ||
|
|
||
| // (Optional) Span ID of the current trace. | ||
| // span_id may be missing if the measurement is not recorded inside a trace or if the trace is not sampled. | ||
| bytes span_id = 5; | ||
|
bogdandrutu marked this conversation as resolved.
|
||
|
|
||
| // (Optional) Trace ID of the current trace. | ||
| // trace_id may be missing if the measurement is not recorded inside a trace or if the trace is not sampled. | ||
| bytes trace_id = 6; | ||
|
|
||
| // (Optional) When sample_count is non-zero, this exemplar has been chosen in a statistically | ||
| // unbiased way such that the exemplar is representative of `sample_count` individual events | ||
| double sample_count = 7; | ||
| } | ||
|
|
||
| // Defines a Metric which has one or more timeseries. | ||
| // | ||
| // The data model and relation between entities is shown in the | ||
|
|
@@ -352,6 +383,10 @@ message Int64DataPoint { | |
|
|
||
| // value itself. | ||
| int64 value = 4; | ||
|
|
||
| // (Optional) List of exemplars collected from | ||
| // measurements that were used to form the data point | ||
| repeated RawValue exemplars = 5; | ||
| } | ||
|
|
||
| // DoubleDataPoint is a single data point in a timeseries that describes the time-varying | ||
|
|
@@ -382,6 +417,10 @@ message DoubleDataPoint { | |
|
|
||
| // value itself. | ||
| double value = 4; | ||
|
|
||
| // (Optional) List of exemplars collected from | ||
| // measurements that were used to form the data point | ||
| repeated RawValue exemplars = 5; | ||
| } | ||
|
|
||
| // HistogramDataPoint is a single data point in a timeseries that describes the time-varying | ||
|
|
@@ -452,6 +491,10 @@ message HistogramDataPoint { | |
| // If we decide to also support (a, b] intervals we should add support for these by defining | ||
| // a boolean value which decides what type of intervals to use. | ||
| repeated double explicit_bounds = 7; | ||
|
|
||
| // (Optional) List of exemplars collected from | ||
| // measurements that were used to form the data point | ||
| repeated RawValue exemplars = 8; | ||
|
cnnradams marked this conversation as resolved.
Outdated
|
||
| } | ||
|
|
||
| // SummaryDataPoint is a single data point in a timeseries that describes the time-varying | ||
|
|
@@ -508,4 +551,8 @@ message SummaryDataPoint { | |
| // A list of values at different quantiles of the distribution calculated | ||
| // from the current snapshot. The quantiles must be strictly increasing. | ||
| repeated ValueAtQuantile quantile_values = 6; | ||
|
|
||
| // (Optional) List of exemplars collected from | ||
| // measurements that were used to form the data point | ||
| repeated RawValue exemplars = 7; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would this just be a list of random samples from the whole window? Open question in the OTEP:
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The proto does not define how the exemplars were sampled, not sure your question?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nvm, I see now
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. At the recent OTLP discussion meeting, we agreed to remove the
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @bogdandrutu does this sound right to you, for now?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I think we agreed that you will evaluate what is better for performance/semantics:
My point is that I don't have a strong opinion between the both, and was trying to make you investigate and decide which way. Here are my thoughts:
I feel
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we go with exemplars in every DataPoint I would say to rename the message to "Exemplar" :)
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This discussion makes me want a way to intern label sets to avoid the "re-map every exemplar" problem. I don't feel inclined to invest time in this now, so we should probably choose "repeated RawValue exemplars in every point".
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Even with an "intern label" you still need to map every exemplar to a point (that mapping may be faster if we have "intern label" but still needs some work) |
||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.