process.command_line accepts multiple value types

[anuraaga]

I noticed process.command_line accepts different types of values. I don't think we generally do this but was this intended? I would just use a string for it in all cases.

https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/resource/semantic_conventions/process.md#process

[Oberon00]

This was intentional and was discussed at #635 (comment). In some environments (Windows) a single string (space separated or however it was typed in the shell) is the raw form of this. In others (Linux) a NUL-separated string that is easily translated to a list is the raw form. Additionally, some runtimes support only obtaining the commandline as list of arguments, which would need to be escaped (according to which shell's syntax?) if you want them as single string.
If we only support a single string, we would additionally need an attribute that tells us the format the string is in.

[Oberon00]

Actually, a much nicer summary of the pros & cons is at #635 (comment) by @james-bebbington :

There is also concern around using two different data types to represent the same attribute value: #635 (comment). It might create additional complexity on back end systems to have to manage potentially different data types for the same attribute, though I'm not sure how much of a concern this would be? I guess parsing null-characters on the back end would likely be equivalently complex.

Note this isn't strictly ambiguous btw. If that was a command with no arguments, and you were looking at the Unix representation, it would have a null-character as the last character in the string. On Windows it would not. Admittedly that is not a great representational difference, and could be difficult to distinguish in some programming languages.

After thinking about this some more, I think I do prefer the list of strings approach, but I'm not sure I'm aware of all the pros/cons. For now I have changed it back to that, and updated the example for each OS accordingly. But for the record here's the three options considered and pros/cons:

1. Join arguments with a string:
   Advantages: Most consistent representation across OSs.
   Disadvantages: Potentially lossy/non-trivial conversion on Unix - do we need to enquote parameters with spaces in them (and escape internal quotes), etc?
2. Pass through string returned from OS as is (null-delimited on Unix):
   Advantages: No processing needed by code producing telemetry, can just provide what OS returns.
   Disadvantages: Back end needs to parse null-characters.
3. Convert to list of strings (for Unix only):
   Advantages: Clean ("properly typed") representation.
   Disadvantages: Inconsistent data type for the same attribute between OSs, and back end needs to handle this differently.

[anuraaga]

Yeah I can definitely see the motivation for this. At the same time, instrumentation and backend does get quite a bit more complex because of the dual-type so I don't know if this will be easy to implement - generating code from the semantic spec yaml seems especially problematic.

@thisthat Can you check if your semantic spans work be able to cover this case? #571

[Oberon00]

At the same time, instrumentation and backend does get quite a bit more complex because of the dual-type

Instrumentation gets simpler because it has a choice, and can do whatever is easier. Or am I missing something?

[Oberon00]

@thisthat Can you check if your semantic spans work be able to cover this case? #571

Currently the tool doesn't support this, but it can be extended. In fact we had the possibility of such union types in mind from the beginning.

[anuraaga]

I see - currently in Java we use an abstraction like StringAttributeSetter for operating on attributes. We don't yet use it for resource attributes but I think we should and probably will eventually. So we would need to introduce a clunky StringOrArrayAttributeSetter - it's not the end of the world, but this is what I meant by instrumentation getting more complex (probably only applies to typesafe language).

[Oberon00]

You can also have two constants for this semantic attribute, e.g. a StringAttributeSetter PROCESS_COMMANDLINE_STR and PROCESS_COMMANDLINE_LIST. Though I begin to see your point. We could instead have two alternative semantic attributes with two different names in the semantic conventions. It would be a bit clunky in the spec then though.

[anuraaga]

Anyways it's a very small point - I was surprised to see it but since it was already discussed it's fine with me. Thanks for clarifying it.

[anuraaga]

Actually let me keep this open. The spec text says either primitive or array

https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/common/common.md#attributes

So I think we need to tweak that to more formalize the possibility of union types.

[Oberon00]

The actual Attribute will not have an union type, it will have either one or the other type. Only the semantic convention has the union type.