-
Notifications
You must be signed in to change notification settings - Fork 265
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
Eliminate multi-data point timeseries #77
Eliminate multi-data point timeseries #77
Conversation
Resolves open-telemetry#54 After a discussion we came to conclusion that there is no good use case for having multiple data points. All possible ways to combine data points that we imagined are either very expensive or make no semantic sense. This commit eliminates timeseries that can have multiple data points and replaces them with single data points. If in the future we discover a use case for multiple data points we have 2 possible paths to extend the proto: - Add a corresponding points repeated field in the Int64DataPoint. - Introduce a different message type that is capable of storing multiple data points and assign it a different type in the MetricDescriptor.Type enum. There are probably other ways as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you split the removing of the generated code in a separate PR?
@bogdandrutu I do not understand, can you clarify? I did not remove any code, just re-generated it to match proto changes. |
@tigrannajaryan I think I understand what happens. You actually regenerated the files. This is so hard to review. I think in the future will be a big mess if we do re-generate the files in the PR. Maybe we should just configure the CI to regenerate everything after each PR merged to master. |
@open-telemetry/specs-approvers please review. |
Just to add a little more depth, I can imagine two scenarios calling for different formats, ideally. You might wish to export a list of timestamped values where each value has its own distinct timestamp. In that case, the original protocol would do. You might wish to elide the timestamp for a short time period by recording the values and a single timestamp, in which case a |
@open-telemetry/specs-approvers Can I get more reviews on this, please? |
Thanks @yurishkuro |
Resolves #54
After a discussion we came to conclusion that there is no good use case
for having multiple data points. All possible ways to combine data points
that we imagined are either very expensive or make no semantic sense.
This commit eliminates timeseries that can have multiple data points and replaces
them with single data points.
If in the future we discover a use case for multiple data points we have 2
possible paths to extend the proto:
points and assign it a different type in the MetricDescriptor.Type enum.
There are probably other ways as well.