Default Instrument unit does not comply with spec

[pichlermarc]

Currently the spec states here that:

If the unit is not provided or the unit is null, the API and SDK MUST make sure that the behavior is the same as an empty unit string.

However, the current implementation does change undefined and null units to '1' in createInstrumentDescriptor(), which is different from the empty string. An empty string is usually kept.

The Metrics API and SDK specs does not define a "default" unit, therefore to make this compliant with the spec we have two options:

1. treat empty strings as '1'
2. treat null and undefined as an empty string.

I think the spec allows for these two options so it might also make sense to ask for clarification in the spec repo.

[dyladan]

Hmm that's an interesting one. AFAIK the "default unit" was '1' at one point, which is probably why null/undefined was transformed to '1'. Empty string not being transformed seems like a reasonable behavior as well because a user specifically included a string. Reading the spec I think we should treat null/undefined as empty string and not transform to '1'

[pichlermarc]

+1 for treating null/undefined as an empty string. This allows backends to distinguish between a truely undefined unit and '1' being used on purpose. 🙂

This sounds like a candidate for "good first issue". 🙂

[andyfleming]

I'll tackle this 👍

[dyladan]

@andyfleming any update on this?

[andyfleming]

Getting back to this now. Opened up a PR. There's an outstanding issue, but we can continue that discussion over there.