profcheck: use dedicated check for AttributeTable#109
Conversation
As KeyValueAndUnit uses pcommon.AnyValue as type for field Value we need to explicitly check the value of pcommon.AnyValue if it is nil rather than check if Value of KeyValueAndUnit is nil. Fixes open-telemetry/opentelemetry-ebpf-profiler#1360 and avoid the need for work arounds in various places. Signed-off-by: Florian Lehner <florian.lehner@elastic.co>
| // key and unit indices and the value field holds nil as value. | ||
| func checkAttributeTableZeroVal(attrTable []*profiles.KeyValueAndUnit) error { | ||
| if len(attrTable) == 0 { | ||
| return errors.New("empty table, must have at least zero value entry") |
There was a problem hiding this comment.
| return errors.New("empty table, must have at least zero value entry") | |
| return errors.New("empty attribute table, must have at least zero value entry") |
There was a problem hiding this comment.
For consistency with checkZeroVal() I'm keeping the error message.
There was a problem hiding this comment.
I think in this case, we can do better as the primary utility of this logic is to point out violations. So all these empty table errors (not just attribute table) can be a lot better in mentioning the actual table that violates the spec (rather than "empty table" which leaves the user manually having to inspect data to figure out the source of the violation).
We don't have to do that in this PR, LMK what you think.
|
@florianl Curious - why is it not "pure" zero value with all fields unset? Why is the value field set? |
The core issue is an encoding problem. Since the |
3 participants
As KeyValueAndUnit uses pcommon.AnyValue as type for field Value we need to explicitly check the value of pcommon.AnyValue if it is nil rather than check if Value of KeyValueAndUnit is nil.
Fixes open-telemetry/opentelemetry-ebpf-profiler#1360 and avoid the need for work arounds in various places.