From 100918f4ba66a4267f75b747afbdcb3d0d05fe4f Mon Sep 17 00:00:00 2001 From: Florian Lehner Date: Fri, 25 Apr 2025 09:59:36 +0200 Subject: [PATCH 1/6] profiles: use single Profile.sample_type MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implements https://github.com/open-telemetry/opentelemetry-proto/issues/633#issuecomment-2772648117. Signed-off-by: Florian Lehner Co-authored-by: Felix Geisendörfer --- .../profiles/v1development/profiles.proto | 34 +++++++------------ 1 file changed, 12 insertions(+), 22 deletions(-) diff --git a/opentelemetry/proto/profiles/v1development/profiles.proto b/opentelemetry/proto/profiles/v1development/profiles.proto index 28777aa6..a0c2ccdc 100644 --- a/opentelemetry/proto/profiles/v1development/profiles.proto +++ b/opentelemetry/proto/profiles/v1development/profiles.proto @@ -207,13 +207,10 @@ message ScopeProfiles { message Profile { // A description of the samples associated with each Sample.value. // For a cpu profile this might be: - // [["cpu","nanoseconds"]] or [["wall","seconds"]] or [["syscall","count"]] + // ["cpu","nanoseconds"] or ["wall","seconds"] or ["syscall","count"] // For a heap profile, this might be: - // [["allocations","count"], ["space","bytes"]], - // If one of the values represents the number of events represented - // by the sample, by convention it should be at index 0 and use - // sample_type.unit == "count". - repeated ValueType sample_type = 1; + // ["allocations","count"] or ["space","bytes"], + ValueType sample_type = 1; // The set of samples recorded in this profile. repeated Sample sample = 2; @@ -238,22 +235,20 @@ message Profile { // for human-friendly content. The profile must stay functional if this field // is cleaned. repeated int32 comment_strindices = 8; // Indices into ProfilesData.string_table. - // Index into the sample_type array to the default sample type. - int32 default_sample_type_index = 9; // A globally unique identifier for a profile. The ID is a 16-byte array. An ID with // all zeroes is considered invalid. // // This field is required. - bytes profile_id = 10; + bytes profile_id = 9; // 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 = 11; + uint32 dropped_attributes_count = 10; // Specifies format of the original payload. Common values are defined in semantic conventions. [required if original_payload is present] - string original_payload_format = 12; + string original_payload_format = 11; // 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. @@ -261,7 +256,7 @@ message Profile { // 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 = 13; + bytes original_payload = 12; // References to attributes in attribute_table. [optional] // It is a collection of key/value pairs. Note, global attributes @@ -276,7 +271,7 @@ message Profile { // https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/common/README.md#attribute // Attribute keys MUST be unique (it is not allowed to have more than one // attribute with the same key). - repeated int32 attribute_indices = 14; + repeated int32 attribute_indices = 13; } // Represents a mapping between Attribute Keys and Units. @@ -384,21 +379,16 @@ message Sample { // locations_length along with locations_start_index refers to a slice of locations in Profile.location_indices. // Supersedes location_index. int32 locations_length = 2; - // 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. - // When aggregating multiple samples into a single sample, the - // result has a list of values that is the element-wise sum of the - // lists of the originals. - repeated int64 value = 3; + // The type and unit of each value is defined by Profile.sample_type. + repeated int64 values = 3; // References to attributes in ProfilesData.attribute_table. [optional] repeated int32 attribute_indices = 4; // Reference to link in ProfilesData.link_table. [optional] optional int32 link_index = 5; - // Timestamps associated with Sample represented in nanoseconds. These timestamps are expected - // to fall within the Profile's time range. [optional] + // Timestamps associated with Sample represented in nanoseconds. These + // timestamps are expected to fall within the Profile's time range. [optional] repeated uint64 timestamps_unix_nano = 6; } From 42b3e4d00202bf5805f46ae54334966187e59a3c Mon Sep 17 00:00:00 2001 From: Florian Lehner Date: Mon, 5 May 2025 07:53:18 +0200 Subject: [PATCH 2/6] Apply suggestions from code review Co-authored-by: Christos Kalkanis --- opentelemetry/proto/profiles/v1development/profiles.proto | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/opentelemetry/proto/profiles/v1development/profiles.proto b/opentelemetry/proto/profiles/v1development/profiles.proto index a0c2ccdc..bc6e5c2b 100644 --- a/opentelemetry/proto/profiles/v1development/profiles.proto +++ b/opentelemetry/proto/profiles/v1development/profiles.proto @@ -205,11 +205,11 @@ message ScopeProfiles { // for ease of understanding data migration, it is not intended that pprof:Profile and // OpenTelemetry:Profile encoding be wire compatible. message Profile { - // A description of the samples associated with each Sample.value. + // The type and unit of all Sample.values in this profile. // For a cpu profile this might be: - // ["cpu","nanoseconds"] or ["wall","seconds"] or ["syscall","count"] + // ["cpu","count"] or ["off_cpu","nanoseconds"] // For a heap profile, this might be: - // ["allocations","count"] or ["space","bytes"], + // ["allocated_objects","count"] or ["allocated_space","bytes"], ValueType sample_type = 1; // The set of samples recorded in this profile. repeated Sample sample = 2; From 82cfc1f781788c669f253d97797eed93cd2493e8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felix=20Geisend=C3=B6rfer?= Date: Sun, 11 May 2025 14:21:44 +0200 Subject: [PATCH 3/6] profiles: improve sample_type comment I think using nanoseconds is the better default unit for CPU profiles because the weight of a count can be different depending on the sample rate. Using the same unit for off_cpu and cpu is more consistent. The comment above was a little confusing, since it referred only to a cpu profile, but was then followed by an off_cpu profile sample as well. --- opentelemetry/proto/profiles/v1development/profiles.proto | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/opentelemetry/proto/profiles/v1development/profiles.proto b/opentelemetry/proto/profiles/v1development/profiles.proto index bc6e5c2b..904d283c 100644 --- a/opentelemetry/proto/profiles/v1development/profiles.proto +++ b/opentelemetry/proto/profiles/v1development/profiles.proto @@ -206,8 +206,8 @@ message ScopeProfiles { // OpenTelemetry:Profile encoding be wire compatible. message Profile { // The type and unit of all Sample.values in this profile. - // For a cpu profile this might be: - // ["cpu","count"] or ["off_cpu","nanoseconds"] + // For a cpu or off-cpu profile this might be: + // ["cpu","nanoseconds"] or ["off_cpu","nanoseconds"] // For a heap profile, this might be: // ["allocated_objects","count"] or ["allocated_space","bytes"], ValueType sample_type = 1; From 79f44c9acb67a6de3258b7c1cffdfbe789536488 Mon Sep 17 00:00:00 2001 From: Florian Lehner Date: Tue, 27 May 2025 14:35:19 +0200 Subject: [PATCH 4/6] fixup: apply feedback https://github.com/open-telemetry/opentelemetry-proto/pull/649/files#r2083519081 Signed-off-by: Florian Lehner --- .../proto/profiles/v1development/profiles.proto | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/opentelemetry/proto/profiles/v1development/profiles.proto b/opentelemetry/proto/profiles/v1development/profiles.proto index 401fd319..155a3b53 100644 --- a/opentelemetry/proto/profiles/v1development/profiles.proto +++ b/opentelemetry/proto/profiles/v1development/profiles.proto @@ -374,10 +374,14 @@ message ValueType { AggregationTemporality aggregation_temporality = 3; } -// Each Sample records values encountered in some program -// context. The program context is typically a stack trace, perhaps -// augmented with auxiliary information like the thread-id, some -// indicator of a higher level request being handled etc. +// Each Sample records values encountered in some program context. The program +// context is typically a stack trace, perhaps augmented with auxiliary +// information like the thread-id, some indicator of a higher level request +// being handled etc. +// +// A Sample MUST have have at least one values or timestamps_unix_nano entry. If +// both fields are populated, they MUST contain the same number of elements, and +// the elements at the same index MUST refer to the same event. 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; @@ -393,7 +397,7 @@ message Sample { optional int32 link_index = 5; // Timestamps associated with Sample represented in nanoseconds. These - // timestamps are expected to fall within the Profile's time range. [optional] + // timestamps should fall within the Profile's time range. repeated uint64 timestamps_unix_nano = 6; } From b66ec4e342b6d470ec7387e7f39eb289679d36b5 Mon Sep 17 00:00:00 2001 From: Florian Lehner Date: Tue, 3 Jun 2025 10:32:31 +0200 Subject: [PATCH 5/6] fixup: add default_sample_type Signed-off-by: Florian Lehner --- opentelemetry/proto/profiles/v1development/profiles.proto | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/opentelemetry/proto/profiles/v1development/profiles.proto b/opentelemetry/proto/profiles/v1development/profiles.proto index 155a3b53..1eef208c 100644 --- a/opentelemetry/proto/profiles/v1development/profiles.proto +++ b/opentelemetry/proto/profiles/v1development/profiles.proto @@ -178,6 +178,13 @@ message ScopeProfiles { // https://opentelemetry.io/docs/specs/otel/schemas/#schema-url // This schema_url applies to all profiles in the "profiles" field. string schema_url = 3; + + // The preferred type and unit of Samples in at least one Profile. + // For a cpu or off-cpu profile this might be: + // ["cpu","nanoseconds"] or ["off_cpu","nanoseconds"] + // For a heap profile, this might be: + // ["allocated_objects","count"] or ["allocated_space","bytes"], + ValueType default_sample_type = 4; } // Profile is a common stacktrace profile format. From 5f0767dd04a875015b994ef9ac11800adf9a357d Mon Sep 17 00:00:00 2001 From: Florian Lehner Date: Fri, 6 Jun 2025 10:35:18 +0200 Subject: [PATCH 6/6] fixup: referr from ScopeProfiles.default_sampel_type to Profile.sample_type Signed-off-by: Florian Lehner --- opentelemetry/proto/profiles/v1development/profiles.proto | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/opentelemetry/proto/profiles/v1development/profiles.proto b/opentelemetry/proto/profiles/v1development/profiles.proto index 1eef208c..8c8e3223 100644 --- a/opentelemetry/proto/profiles/v1development/profiles.proto +++ b/opentelemetry/proto/profiles/v1development/profiles.proto @@ -180,10 +180,7 @@ message ScopeProfiles { string schema_url = 3; // The preferred type and unit of Samples in at least one Profile. - // For a cpu or off-cpu profile this might be: - // ["cpu","nanoseconds"] or ["off_cpu","nanoseconds"] - // For a heap profile, this might be: - // ["allocated_objects","count"] or ["allocated_space","bytes"], + // See Profile.sample_type for possible values. ValueType default_sample_type = 4; }