From 721223af608a298b18a96d0a36d77facb49e6c43 Mon Sep 17 00:00:00 2001 From: Alexey Alexandrov Date: Mon, 5 May 2025 18:07:57 -0700 Subject: [PATCH 1/4] Optimized profile stack trace representation. In #645 a simplified stack trace representation is proposed. A discussion there asks for a more efficient way to represent stacks that slightly differ at the leaf. A specialized solution proposed there would put the leaf location as a separate field. That seems partial and potentially error-prone. This PR proposes an alternative stack trace representation, shaped as a tree in the form of two arrays. See the code for details and an example. The advantage of this approach is that stacks with the same prefix are encoded more compactly. Also, since we use only two arrays for the encoding, the in-memory representation is also more efficient in terms of the number of allocations required. The main disadvantage is that this approach is more complex semantically and for this reason can be more error-prone. --- .../profiles/v1development/profiles.proto | 34 ++++++++++++++----- 1 file changed, 26 insertions(+), 8 deletions(-) diff --git a/opentelemetry/proto/profiles/v1development/profiles.proto b/opentelemetry/proto/profiles/v1development/profiles.proto index 28777aa6..d0437135 100644 --- a/opentelemetry/proto/profiles/v1development/profiles.proto +++ b/opentelemetry/proto/profiles/v1development/profiles.proto @@ -132,6 +132,29 @@ message ProfilesData { // Represents a mapping between Attribute Keys and Units. repeated AttributeUnit attribute_units = 8; + + // Stack tree encoded as two parallel arrays: one for the parent array, + // another for the ProfilesData.location_table index of the location + // corresponding to the given stack tree node. The two arrays must have the + // same length. The parent array uses -1 for the root nodes. + // + // For example, these stacks: + // + // main -> foo -> bar + // main -> foo -> bar -> baz1 + // main -> foo -> bar -> baz2 + // main -> foo -> baz2 + // + // would be expressed with a location_table like: + // location_table = ["", "main", "foo", "bar", "baz1", "baz2"] + // + // And the stack tree arrays would be: + // (stack index) = 0 1 2 3 4 5 6 + // stack_parent_array = [-1, 0, 1, 2, 3, 3, 2] + // stack_location_index = [ 0, 1, 2, 3, 4, 5, 5] + // + repeated int32 stack_parent_array = 9; + repeated int32 stack_location_index = 10; } @@ -217,9 +240,6 @@ message Profile { // The set of samples recorded in this profile. repeated Sample sample = 2; - // References to locations in ProfilesData.location_table. - repeated int32 location_indices = 3; - // The following fields 4-14 are informational, do not affect // interpretation of results. @@ -379,11 +399,9 @@ message ValueType { // augmented with auxiliary information like the thread-id, some // indicator of a higher level request being handled etc. 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; + // Index into ProfilesData.{stack_parent_array, stack_location_index} stack + // table, representing the stack tree node of the sample. + int32 stack_index = 1; // The type and unit of each value is defined by the corresponding // entry in Profile.sample_type. All samples must have the same // number of values, the same as the length of Profile.sample_type. From 016b489391d28c4ce37b4a09d6e113e4be1915a5 Mon Sep 17 00:00:00 2001 From: Alexey Alexandrov Date: Tue, 6 May 2025 09:12:56 -0700 Subject: [PATCH 2/4] Correct the example. --- .../profiles/v1development/profiles.proto | 22 +++++++++---------- 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/opentelemetry/proto/profiles/v1development/profiles.proto b/opentelemetry/proto/profiles/v1development/profiles.proto index d0437135..f8f0c837 100644 --- a/opentelemetry/proto/profiles/v1development/profiles.proto +++ b/opentelemetry/proto/profiles/v1development/profiles.proto @@ -138,26 +138,24 @@ message ProfilesData { // corresponding to the given stack tree node. The two arrays must have the // same length. The parent array uses -1 for the root nodes. // - // For example, these stacks: + // For example, the following location and stack tree data: // - // main -> foo -> bar - // main -> foo -> bar -> baz1 - // main -> foo -> bar -> baz2 - // main -> foo -> baz2 - // - // would be expressed with a location_table like: // location_table = ["", "main", "foo", "bar", "baz1", "baz2"] + // (stack index) = 0 1 2 3 4 5 + // stack_parent_array = [-1, 0, 1, 2, 2, 1] + // stack_location_index = [ 1, 2, 3, 4, 5, 5] + // + // are encoding these stacks (the number is the stack index): // - // And the stack tree arrays would be: - // (stack index) = 0 1 2 3 4 5 6 - // stack_parent_array = [-1, 0, 1, 2, 3, 3, 2] - // stack_location_index = [ 0, 1, 2, 3, 4, 5, 5] + // main|0 -> foo|1 -> bar|2 + // main|0 -> foo|1 -> bar|2 -> baz1|3 + // main|0 -> foo|1 -> bar|2 -> baz2|4 + // main|0 -> foo|1 -> baz2|5 // repeated int32 stack_parent_array = 9; repeated int32 stack_location_index = 10; } - // A collection of ScopeProfiles from a Resource. message ResourceProfiles { reserved 1000; From 81ad52e7fa50dd8027f2f9f9ef014ebb75731bb6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felix=20Geisend=C3=B6rfer?= Date: Mon, 12 May 2025 06:09:30 +0200 Subject: [PATCH 3/4] profiles: improve stack tree description --- .../profiles/v1development/profiles.proto | 44 +++++++++++++++---- 1 file changed, 35 insertions(+), 9 deletions(-) diff --git a/opentelemetry/proto/profiles/v1development/profiles.proto b/opentelemetry/proto/profiles/v1development/profiles.proto index f8f0c837..aac32d9d 100644 --- a/opentelemetry/proto/profiles/v1development/profiles.proto +++ b/opentelemetry/proto/profiles/v1development/profiles.proto @@ -138,20 +138,46 @@ message ProfilesData { // corresponding to the given stack tree node. The two arrays must have the // same length. The parent array uses -1 for the root nodes. // - // For example, the following location and stack tree data: + // For example, the following locations and stack tree data: // - // location_table = ["", "main", "foo", "bar", "baz1", "baz2"] - // (stack index) = 0 1 2 3 4 5 - // stack_parent_array = [-1, 0, 1, 2, 2, 1] - // stack_location_index = [ 1, 2, 3, 4, 5, 5] + // +-------------+----------------+--------------------+----------------------+ + // | stack index | location_table | stack_parent_array | stack_location_index | + // +-------------+----------------+--------------------+----------------------+ + // | 0 | "" | -1 | 1 | + // | 1 | "main" | 0 | 2 | + // | 2 | "foo" | 1 | 3 | + // | 3 | "bar" | 2 | 4 | + // | 4 | "baz1" | 2 | 5 | + // | 5 | "baz2" | 1 | 5 | + // +-------------+----------------+--------------------+----------------------+ // // are encoding these stacks (the number is the stack index): // - // main|0 -> foo|1 -> bar|2 - // main|0 -> foo|1 -> bar|2 -> baz1|3 - // main|0 -> foo|1 -> bar|2 -> baz2|4 - // main|0 -> foo|1 -> baz2|5 + // main (0) + // └── foo (1) + // ├── bar (2) + // │ ├── baz1 (3) + // │ └── baz2 (4) + // └── baz2 (5) // + // And here is an example of the operations to decode the stack trace + // "main->foo->bar->baz2" at stack index 4. + // + // 1. Lookup stack_location_index[4] = 5 + // 2. Lookup location_table[5] = "baz2" + // 3. Lookup stack_parent_array[4] = 2 + // + // 4. Lookup stack_location_index[2] = 3 + // 5. Lookup location_table[3] = "bar" + // 6. Lookup stack_parent_array[2] = 1 + // + // 7. Lookup stack_location_index[1] = 2 + // 8. Lookup location_table[2] = "foo" + // 9. Lookup stack_parent_array[1] = 0 + // + // 10. Lookup stack_location_index[0] = 1 + // 11. Lookup location_table[1] = "main" + // 12. Lookup stack_parent_array[0] = -1 repeated int32 stack_parent_array = 9; repeated int32 stack_location_index = 10; } From ae680bff43d2236b77f25808d5c8cf961d49193e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felix=20Geisend=C3=B6rfer?= Date: Tue, 13 May 2025 08:57:47 +0200 Subject: [PATCH 4/4] profiles: split the stack tree ASCII table in the comment into two Based on internal feedback, putting the location and stack indexed lists into the same table is very confusing because they are really two separate tables. Make this clear by splitting them up into two. --- .../profiles/v1development/profiles.proto | 31 +++++++++++++------ 1 file changed, 21 insertions(+), 10 deletions(-) diff --git a/opentelemetry/proto/profiles/v1development/profiles.proto b/opentelemetry/proto/profiles/v1development/profiles.proto index aac32d9d..f20d2bd2 100644 --- a/opentelemetry/proto/profiles/v1development/profiles.proto +++ b/opentelemetry/proto/profiles/v1development/profiles.proto @@ -140,16 +140,27 @@ message ProfilesData { // // For example, the following locations and stack tree data: // - // +-------------+----------------+--------------------+----------------------+ - // | stack index | location_table | stack_parent_array | stack_location_index | - // +-------------+----------------+--------------------+----------------------+ - // | 0 | "" | -1 | 1 | - // | 1 | "main" | 0 | 2 | - // | 2 | "foo" | 1 | 3 | - // | 3 | "bar" | 2 | 4 | - // | 4 | "baz1" | 2 | 5 | - // | 5 | "baz2" | 1 | 5 | - // +-------------+----------------+--------------------+----------------------+ + // +----------------+----------------+ + // | location index | location_table | + // +----------------+----------------+ + // | 0 | "" | + // | 1 | "main" | + // | 2 | "foo" | + // | 3 | "bar" | + // | 4 | "baz1" | + // | 5 | "baz2" | + // +----------------+----------------+ + // + // +-------------+--------------------+----------------------+ + // | stack index | stack_parent_array | stack_location_index | + // +-------------+--------------------+----------------------+ + // | 0 | -1 | 1 | + // | 1 | 0 | 2 | + // | 2 | 1 | 3 | + // | 3 | 2 | 4 | + // | 4 | 2 | 5 | + // | 5 | 1 | 5 | + // +-------------+--------------------+----------------------+ // // are encoding these stacks (the number is the stack index): //