reporter: do not add empty attributes

[florianl]

PR #212 changed the behaviour of addProfileAttributes(). Before adding an attribute the length of the string was checked - see

opentelemetry-ebpf-profiler/reporter/otlp_reporter.go

Lines 747 to 749 in 0c58efd

	if attr.value == "" {
	return
	}

This returns to the original behaivour, where string attributes with no values are not added.

[athre0z]

LGTM!

Edit: ah, "empty" variant of TestGetSampleAttributes test needs fix-up.

[rockdaboot]

PR #212 changed the behaviour of addProfileAttributes(). Before adding an attribute the length of the string was checked.

This has been changed on purpose because how else are we going to differentiate in the backend
a) key exists with empty value
b) key does not exist
?

If the backend doesn't want to store empty values, these should be filtered out in the backend.
Other backends may want to store empty and NULL values.

[florianl]

To meet OTel SemConv requirements, a field was added to attrKeyValue indicating Required attributes, so that they are sent also with empty values. I have checked the OTel SemConv attributes we are sending out atm and non of them is marked as a Required attribute so far.

[rockdaboot]

To meet OTel SemConv requirements, a field was added to attrKeyValue indicating Required attributes, so that they are sent also with empty values. I have checked the OTel SemConv attributes we are sending out atm and non of them is marked as a Required attribute so far.

IMO these are two different pairs of shoes.
To my understanding, a "semconv required" field is required because otherwise parts or all of the data doesn't make much sense to the backend/receiver.
And in this PR it is about allowing the backend/receiver to differentiate between "information not available" and "information available: empty string" or not. We should allow the backend/receiver to distinguish these cases and act accordingly. In other words, the client makes this decision without knowing anything about the backend, which is plain wrong in my eyes.

[florianl]

IMO these are two different pairs of shoes.
To my understanding, a "semconv required" field is required because otherwise parts or all of the data doesn't make much sense to the backend/receiver.

Please provide a list of attributes that should be always sent, also with empty values. What qualifies an attribute to be required to be sent always, also without value, if the OTel SemConv does not set the requirement level Required for this attribute?

[rockdaboot]

IMO these are two different pairs of shoes.
To my understanding, a "semconv required" field is required because otherwise parts or all of the data doesn't make much sense to the backend/receiver.

Please provide a list of attributes that should be always sent, also with empty values. What qualifies an attribute to be required to be sent always, also without value, if the OTel SemConv does not set the requirement level Required for this attribute?

It's two different things. I am seeking for clarification for how to transport the information "no value" and "empty value" to the server, no matter if the attribute is required or not.

This PR skips sending an attribute in both cases, so the server side can simply not say if that is due to a missing value or due to an explicit empty string.

At least the Elastic backend currently doesn't distinguish between those two cases for the existing attributes, so I am fine with this PR. And the cases where it matters are mostly theoretical and can be worked around in other ways.

[florianl]

I am seeking for clarification for how to transport the information "no value" and "empty value" to the server, no matter if the attribute is required or not.

If there is an essential attribute that should be sent always, please name it. Then the required field in attrKeyValue can be set for it and it will be send always - even with empty values.

[rockdaboot]

I am seeking for clarification for how to transport the information "no value" and "empty value" to the server, no matter if the attribute is required or not.

If there is an essential attribute that should be sent always, please name it. Then the required field in attrKeyValue can be set for it and it will be send always - even with empty values.

I originally thought about the COMM value (thread name), which can be set to an arbitrary value by the application, even to an empty string. But currently it won't matter to our backend if it is empty or not available.

Another thought: should we also skip sending integer attributes if the value is 0?

[rockdaboot]

Is there any blocker or can this be merged?

[florianl]

sorry - I did forget about this one. too many tasks on the radar.