-
Notifications
You must be signed in to change notification settings - Fork 303
Description
Hi team,
I found a possible pain point in the current structure of the profiles protocol while revamping Async-Profiler's OTEL dump implementation (async-profiler/async-profiler#1624).
This is the current definition of ProfilesData:
opentelemetry-proto/opentelemetry/proto/profiles/v1development/profiles.proto
Lines 188 to 202 in b805017
| message ProfilesData { | |
| // An array of ResourceProfiles. | |
| // For data coming from an SDK profiler, this array will typically contain one | |
| // element. Host-level profilers will usually create one ResourceProfile per | |
| // container, as well as one additional ResourceProfile grouping all samples | |
| // from non-containerized processes. | |
| // Other resource groupings are possible as well and clarified via | |
| // Resource.attributes and semantic conventions. | |
| // Tools that visualize profiles should prefer displaying | |
| // resources_profiles[0].scope_profiles[0].profiles[0] by default. | |
| repeated ResourceProfiles resource_profiles = 1; | |
| // One instance of ProfilesDictionary | |
| ProfilesDictionary dictionary = 2; | |
| } |
I propose to explode ProfilesDictionary into ProfilesData, so we would have something like this:
message ProfilesData {
repeated ResourceProfiles resource_profiles = 1;
repeated Mapping mapping_table = 2;
repeated Location location_table = 3;
repeated Function function_table = 4;
repeated Link link_table = 5;
repeated string string_table = 6;
repeated KeyValueAndUnit attribute_table = 7;
repeated Stack stack_table = 8;
}
This would let writers interleave profiles with constant pools segments, since Protobuf allows interleaving non-packed repeated fields. It would be convenient for some writers which dump profiles in bulk at end of the profiling period.
I see a related discussion is happening in #682. What I propose here is different and would require close to zero changes in existing implementations; however, I think it stems from a similar idea: not all constants need to be global, and keeping them around (read in memory) may not be ideal for some profile writers.