Opentelemetry-java: Update `null` value handling for span and resource attributes

Created on 25 Sep 2020  路  5Comments  路  Source: open-telemetry/opentelemetry-java

https://github.com/open-telemetry/opentelemetry-specification/pull/992 changed null handling for span and resource attribute values from "delete previous value, otherwise ignore" to "undefined behavior". Handling of null within arrays stays unchanged.
We should document this in the API accordingly and also add a @Nonnull annotation.

I would go for just making it a no-op if null is passed nevertheless. WDYT?

Please also update the https://github.com/open-telemetry/opentelemetry-specification/blob/master/spec-compliance-matrix.md once this is done.

API SDK p1 required-for-ga

Most helpful comment

I think if we can make things work with the @NonNull annotation, that we should, as that will signal very strongly to users that they shouldn't pass nulls in for the values.

All 5 comments

Required for GA is documenting the undefined behavior in the API. Changing the actual handling to a no-op is not a MUST but would probably make sense since the current deletion behavior was quite opposed by some.

We should document this in the API accordingly and also add a @Nonnull annotation.

Sounds something great to have.

I would go for just making it a no-op if null is passed nevertheless. WDYT?

I think this would defeat the purpose of making this an undefined behavior. It was already discussed in the previous related PR, no? IIRC it was discussed we should remove a line around this.

Please also update the https://github.com/open-telemetry/opentelemetry-specification/blob/master/spec-compliance-matrix.md once this is done.

If we were to change the handling to make it no-op, yes. Otherwise, I don't see adding a @Nonnull annotation as a requirement.

IIRC it was discussed we should remove a line around this.

What do you mean by that?

If we were to change the handling to make it no-op, yes. Otherwise, I don't see adding a @Nonnull annotation as a requirement.

The line in the matrix I was referring to says: null values documented as invalid/undefined to verify that this aspect of the API is stated accordingly.

I want to make this a no-op, but have it documented as "undefined behavior". It'll definitely make the code simpler to just no-op any null valued attributes.

I think if we can make things work with the @NonNull annotation, that we should, as that will signal very strongly to users that they shouldn't pass nulls in for the values.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

jkwatson picture jkwatson  路  5Comments

carlosalberto picture carlosalberto  路  4Comments

songy23 picture songy23  路  3Comments

poogurgaon picture poogurgaon  路  4Comments

jkwatson picture jkwatson  路  3Comments