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

Why is AggregationTemporality redefined pprofextended.proto #547

Open
jack-berg opened this issue Apr 24, 2024 · 2 comments
Open

Why is AggregationTemporality redefined pprofextended.proto #547

jack-berg opened this issue Apr 24, 2024 · 2 comments

Comments

@jack-berg
Copy link
Member

Why is AggregationTemporality redefined in this file instead of reusing the definition in metrics.proto?

Originally posted by @jack-berg in #534 (comment)

@aalexand
Copy link

I think this profiler's proto enum was copied from the metrics proto because we expected that it would need some customization. That customization hasn't happened yet.

TBH the current DELTA and CUMULATIVE values in the num are not 100% match, I think they still (especially the comments) carry a bit of monitoring specifics, such as server restarts.

For profiling, common metric types are:

  1. Metric accumulated over the profiling duration. Example - CPU time profiler which runs for certain period of time (e.g. 10 seconds) and reports CPU*seconds consumed over that duration. I guess DELTA is more or less a fit here but the comment needs to be updated to be more relatable to profiling and include some examples (like CPU profiling that I mentioned).
  2. Gauge metrics that represent instantaneous value. Example - RAM / heap usage reported by a durationless profiler that simply takes a snapshot of the heap at a point in time. I don't think either DELTA or CUMULATIVE are a fit here.

There is also a flavor of 1) where the profiler returns a snapshot of the data, but the data is actually cumulative since the process start. I think this still fits the DELTA type and it's just a matter of capturing in the profile the right time window to express over what time interval the data was recorded.

@aalexand
Copy link

There is also a flavor of 1) where the profiler returns a snapshot of the data, but the data is actually cumulative since the process start. I think this still fits the DELTA type and it's just a matter of capturing in the profile the right time window to express over what time interval the data was recorded.

Thinking more about this, I think we do need to distinguish "delta over the profiling duration" (example: CPU time over 10 seconds in a CPU profile), "cumulative since process start" (example: cumulative alloc bytes in Go heap profiles) and "gauge aka instantaneous value" (example: current heap live bytes in a C++ or Go heap profile's "in use" metrics).

I also wonder if the "aggregation temporality" name could be clearer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants