Profiles: simplify profile stack trace representation (ASCII fix, conflicts resolved) (#708)

* Simplify profile stack trace representation

- Introduce a first-class Stack message type and lookup table.
- Replace location index range based stack trace encoding on Sample with
  a single stack_index reference.
- Remove the location_indices lookup table.

The primary motivation is laying the ground work for [timestamp based
profiling][timestamp proposal] where the same stack trace needs to be
referenced much more frequently compared to aggregation based on low
cardinality attributes.

Timestamp based profiling is also expected to be used with the the
upcoming [Off-CPU profiling][off-cpu pr] feature in the eBPF profiler.
Off-CPU stack traces have a different distribution compared to CPU
samples. In particular stack traces are much more repetitive because
they only occur at call sites such as syscalls. For the same reason it
is also uncommon to see a stack trace are a root-prefix of a previously
observed stack trace.

We might need to revisit the previous [previous benchmarks][benchmarks]
to confirm these claims.

The secondary motivation is simplicitly. Arguably the proposed change
here will make it easier to write exporters, processors as well as
receivers.

It seems like we had rough consensus around this change in previous SIG
meetings, and it seems like a good incremental step to make progress on
the timestamp proposal.

[timestamp proposal]: #594
[off-cpu pr]: open-telemetry/opentelemetry-ebpf-profiler#196
[benchmarks]:
https://docs.google.com/spreadsheets/d/1Q-6MlegV8xLYdz5WD5iPxQU2tsfodX1-CDV1WeGzyQ0/edit?gid=2069300294#gid=2069300294

Modified-by: Christos Kalkanis <[email protected]>

* Update ASCII diagram for new Stack message

* Move leaf frame explanation to field inside Stack message

* Update field number

* Update ASCII diagram (Sample->Stack n-1 instead of 1-1)

---------

Co-authored-by: Felix Geisendörfer <[email protected]>

Author: christos68k

opentelemetry/proto/profiles/v1development/profiles.proto

@@ -70,12 +70,18 @@ option go_package = "go.opentelemetry.io/proto/otlp/profiles/v1development";
 // ┌──────────────────┐   1-n   ┌─────────────────┐   ┌──────────┐
 // │      Sample      │ ──────▷ │ KeyValueAndUnit │   │   Link   │
 // └──────────────────┘         └─────────────────┘   └──────────┘
-//   │                    1-n       △      △
-//   │ 1-n        ┌─────────────────┘      │ 1-n
-//   ▽            │                        │
-// ┌──────────────────┐   n-1   ┌──────────────┐
-// │     Location     │ ──────▷ │   Mapping    │
-// └──────────────────┘         └──────────────┘
+//   │                              △      △
+//   │ n-1                          │      │ 1-n
+//   ▽                              │      │
+// ┌──────────────────┐             │      │
+// │      Stack       │             │      │
+// └──────────────────┘             │      │
+//   │                     1-n      │      │
+//   │ 1-n         ┌────────────────┘      │
+//   ▽             │                       │
+// ┌──────────────────┐   n-1   ┌─────────────┐
+// │     Location     │ ──────▷ │   Mapping   │
+// └──────────────────┘         └─────────────┘
 //   │
 //   │ 1-n
 //   ▼
@@ -102,7 +108,7 @@ message ProfilesDictionary {
   // into that address range referenced by locations via Location.mapping_index.
   repeated Mapping mapping_table = 1;
 
-  // Locations referenced by samples via Profile.location_indices.
+  // Locations referenced by samples via Stack.location_indices.
   repeated Location location_table = 2;
 
   // Functions referenced by locations via Line.function_index.
@@ -134,6 +140,9 @@ message ProfilesDictionary {
   // The restrictions take origin from the OpenTelemetry specification:
   // https://github.com/open-telemetry/opentelemetry-specification/blob/v1.47.0/specification/common/README.md#attribute.
   repeated KeyValueAndUnit attribute_table = 6;
+
+  // Stacks referenced by samples via Sample.stack_index.
+  repeated Stack stack_table = 7;
 }
 
 // ProfilesData represents the profiles data that can be stored in persistent storage,
@@ -214,18 +223,19 @@ message ScopeProfiles {
 //   information present to determine the original sampled values.
 //
 // - The profile is represented as a set of samples, where each sample
-//   references a sequence of locations, and where each location belongs
+//   references a stack trace which is a list of locations, each belonging
 //   to a mapping.
-// - There is a N->1 relationship from sample.location_id entries to
-//   locations. For every sample.location_id entry there must be a
+// - There is a N->1 relationship from Stack.location_indices entries to
+//   locations. For every Stack.location_indices entry there must be a
 //   unique Location with that index.
 // - There is an optional N->1 relationship from locations to
 //   mappings. For every nonzero Location.mapping_id there must be a
 //   unique Mapping with that index.
 
-// Represents a complete profile, including sample types, samples,
-// mappings to binaries, locations, functions, string table, and additional metadata.
-// It modifies and annotates pprof Profile with OpenTelemetry specific fields.
+// Represents a complete profile, including sample types, samples, mappings to
+// binaries, stacks, locations, functions, string table, and additional
+// metadata. It modifies and annotates pprof Profile with OpenTelemetry
+// specific fields.
 //
 // Note that whilst fields in this message retain the name and field id from pprof in most cases
 // for ease of understanding data migration, it is not intended that pprof:Profile and
@@ -240,54 +250,51 @@ message Profile {
   // The set of samples recorded in this profile.
   repeated Sample sample = 2;
 
-  // References to locations in ProfilesDictionary.location_table.
-  repeated int32 location_indices = 3;
-
-  // The following fields 4-14 are informational, do not affect
+  // The following fields 3-12 are informational, do not affect
   // interpretation of results.
 
   // Time of collection (UTC) represented as nanoseconds past the epoch.
-  fixed64 time_unix_nano = 4;
+  fixed64 time_unix_nano = 3;
   // Duration of the profile, if a duration makes sense.
-  uint64 duration_nano = 5;
+  uint64 duration_nano = 4;
   // The kind of events between sampled occurrences.
   // e.g [ "cpu","cycles" ] or [ "heap","bytes" ]
-  ValueType period_type = 6;
+  ValueType period_type = 5;
   // The number of events between sampled occurrences.
-  int64 period = 7;
+  int64 period = 6;
   // Free-form text associated with the profile. The text is displayed as is
   // to the user by the tools that read profiles (e.g. by pprof). This field
   // should not be used to store any machine-readable information, it is only
   // for human-friendly content. The profile must stay functional if this field
   // is cleaned.
-  repeated int32 comment_strindices = 8; // Indices into ProfilesDictionary.string_table.
+  repeated int32 comment_strindices = 7; // Indices into ProfilesDictionary.string_table.
 
   // A globally unique identifier for a profile. The ID is a 16-byte array. An ID with
   // all zeroes is considered invalid. It may be used for deduplication and signal
   // correlation purposes. It is acceptable to treat two profiles with different values
   // in this field as not equal, even if they represented the same object at an earlier
   // time.
   // This field is optional; an ID may be assigned to an ID-less profile in a later step.
-  bytes profile_id = 9;
+  bytes profile_id = 8;
 
   // dropped_attributes_count is the number of attributes that were discarded. Attributes
   // can be discarded because their keys are too long or because there are too many
   // attributes. If this value is 0, then no attributes were dropped.
-  uint32 dropped_attributes_count = 10;
+  uint32 dropped_attributes_count = 9;
 
   // Specifies format of the original payload. Common values are defined in semantic conventions. [required if original_payload is present]
-  string original_payload_format = 11;
+  string original_payload_format = 10;
 
   // Original payload can be stored in this field. This can be useful for users who want to get the original payload.
   // Formats such as JFR are highly extensible and can contain more information than what is defined in this spec.
   // Inclusion of original payload should be configurable by the user. Default behavior should be to not include the original payload.
   // If the original payload is in pprof format, it SHOULD not be included in this field.
   // The field is optional, however if it is present then equivalent converted data should be populated in other fields
   // of this message as far as is practicable.
-  bytes original_payload = 12;
+  bytes original_payload = 11;
 
   // References to attributes in attribute_table. [optional]
-  repeated int32 attribute_indices = 13;
+  repeated int32 attribute_indices = 12;
 }
 
 // A pointer from a profile Sample to a trace Span.
@@ -400,23 +407,20 @@ message ValueType {
 //    values: [2, 2, 3, 3]
 //    timestamps_unix_nano: [1, 2, 3, 4]
 message Sample {
-  // locations_start_index along with locations_length refers to to a slice of locations in Profile.location_indices.
-  int32 locations_start_index = 1;
-  // locations_length along with locations_start_index refers to a slice of locations in Profile.location_indices.
-  // Supersedes location_index.
-  int32 locations_length = 2;
+  // Reference to stack in ProfilesDictionary.stack_table.
+  int32 stack_index = 1;
   // The type and unit of each value is defined by Profile.sample_type.
-  repeated int64 values = 3;
+  repeated int64 values = 2;
   // References to attributes in ProfilesDictionary.attribute_table. [optional]
-  repeated int32 attribute_indices = 4;
+  repeated int32 attribute_indices = 3;
 
   // Reference to link in ProfilesDictionary.link_table. [optional]
   // It can be unset / set to 0 if no link exists, as link_table[0] is always a 'null' default value.
-  int32 link_index = 5;
+  int32 link_index = 4;
 
   // Timestamps associated with Sample represented in nanoseconds. These
   // timestamps should fall within the Profile's time range.
-  repeated fixed64 timestamps_unix_nano = 6;
+  repeated fixed64 timestamps_unix_nano = 5;
 }
 
 // Describes the mapping of a binary in memory, including its address range,
@@ -436,6 +440,13 @@ message Mapping {
   repeated int32 attribute_indices = 5;
 }
 
+// A Stack represents a stack trace as a list of locations.
+message Stack {
+  // References to locations in ProfilesDictionary.location_table.
+  // The first location is the leaf frame.
+  repeated int32 location_indices = 1;
+}
+
 // Describes function and line table debug information.
 message Location {
   // Reference to mapping in ProfilesDictionary.mapping_table.