Profiles: Replace has_* fields with an enum.

[aalexand]

The current has_* fields given 16 choices and most of those are not practical. This is a proposal to limit this to a more practical and guided set.

[aalexand]

@jhalliday @felixge @florianl FYI - sending this for feedback.

[christos68k]

@aalexand Why not have this as an attribute? (I admit I already forgot the relevant part in today's discussion). Looked up the mutability point in the agenda, and just to clarify: wouldn't this be either set once, at origin or if not set at origin, at a later point (e.g. in a processor)? Trying to understand why an attribute wouldn't be a fit here.

If it's just a matter of having to remove a previously set attribute, wouldn't this only apply to SYMBOLIZATION_LEVEL_UNSPECIFIED (which we can define as the absence of the attribute) or would we need more elaborate processing that takes more cases into account?

One reason that springs to mind is implementors not having to deal with OTel KeyValue which IIRC is a concern that you had in the past. Just wondering if there is something else, feel fee to correct my assumptions 😅

[aalexand]

The most elaborate "life of a profile" in terms of symbolization that I can think of is something like:

1. A collector gathers a C++ profile. It sets the symbolization level to SYMBOLIZATION_LEVEL_NONE to indicate that only IP addresses are available and nothing else.
2. The profile is transferred to the backend and stored there.
3. The offline symbolizer processes the profile. At this point, initially, only stripped binary is available to the symbolizer so it symbolizes the profile at SYMBOLIZATION_LEVEL_SYMBOLS level.
4. The offine symbolizer at some point gets full DWARF for the binary and re-symbolizes the profile, storing it with SYMBOLIZATION_LEVEL_LINES_INLINE level now.

The re-symbolization part is not something that all profilers may want to support, but I wanted to indicate that the level upgrade is possible.

Of course, replacing an attribute in a proto in memory is not a huge deal - iterate over the repeated opentelemetry.proto.common.v1.KeyValue attributes list and replace or append the attribute with the desired key and new value, but it's still microchurn and for something that is so common as symbolization level it feels that having a simple explicit field is nicer. But it's definitely not a dealbreaker, so I'd love to hear what others think.

As a side note, I wonder if it would be better if repeated opentelemetry.proto.common.v1.KeyValue attributes would be a map<Key, Value> instead. I thought maybe it's to support multi-value attributes, but there is an explicit support for lists in this type, so it's not that.

One other thing with commony used attributes also is that the definition of allowed values in their semantics is so far away from the proto itself. With an enum one gets compile-time checks, code completion etc. With attributes I don't even know what kind of presubmit or code generation support exists. This aspect probably shouldn't solely guide our decisions on what should be an attribute vs a field, but I think it's still a factor.