add core.KV(string, interface{})

[lizthegrey]

This enables someone to run:
span.SetAttributes(core.KV("foo", bar)...)

otherwise, the syntax winds up being way too verbose to instantiate span.SetAttributes(core.Key("foo").Type(bar)), and is a regression vs. other instrumentation frameworks and against other languages that do the type inference.

[lizthegrey]

Oh, the reason I wrote it as returning []KeyValue rather than a singular KeyValue: this way, we could choose to implement, instead of an unhandled non-stringer returning []KeyValue{{"key","%v"}}, just returning nil, which would then drop the attribute on the floor. Up to y'all whether I do that simplification though.

[lizthegrey]

The other simplification I might propose is bringing back SetAttribute for Span, but taking string, interface{} directly, rather than a list of KeyValues... I know, moving in the reverse direction of #302, except for brevity.

[jmacd]

I'd like to propose some other ideas before we go this direction. You wrote:

span.SetAttributes(core.Key("foo").Type(bar))

We already have an abbreviation for this, span.SetAttributes(key.Type("foo", bar)). The package name key has been debated once or twice--maybe label would be a better name for it. Then you'd read span.SetAttributes(label.Type("key", value)).

The method key.New(x) returns the same as core.Key(x) and is more readable. I consider this the proper way to allocate key variables, so that you'd really end up writing:

var myKey = key.New("mine")

func My(...) {
  span.SetAttributes(myKey.String(value))
}

If a key will be used more than once inside a package, I think it's best to assign them to variables.

Other APIs (e.g., OpenTracing Span.LogKV()) have adopted an alternating key-value approach, where you could write span.SetAttributesKV("foo", bar, "other", value, "third", value). These are not efficient from a memory standpoint, since non-pointer values are heap-allocated when passing through interface{}. The improvement on this is roughly like what we have here, OpenTracing's LogFields() takes a log.Field... which is roughly the same as core.KeyValue in OTel.

Your proposal has the same memory-performance downside that LogKV does in the example above. You'll allocate a small object for every value that passes through core.KV(). I prefer your proposal to something compared with span.SetAttributesKV(), and it performs better: yours only allocates for each value, the OpenTracing-style SetAttributesKV(alternating ...interface{}) allocates once per value and once per key.

I'm inclined to ask if we could rename things to improve matters enough that you wouldn't want this helper. We could rename the key package to kv, then you'd write, for your example: span.SetAttributes(kv.String("foo", bar)).
This, at least, has only one allocation for the whole call (the slice of KeyValue).

One more thing: I don't think you need the core.KV() to return a slice. Have it return a core.KeyValue and then modify the call to read span.SetAttributes(core.KV("key", value)).

Well, we know my opinion. I'm inclined to add to the API to avoid memory allocations, which is part of the reason we have core.KeyValue today and its the reason I still want to restore the singular SetAttribute(). I'd like to hear what others think about this matter.

[jmacd]

Another idea:

import "go.opentelemetry.io/otel/api/key"

var String = key.String

func My(...) {
   span.SetAttribute(String("Key", value))
}

[lizthegrey]

cc @paulosman who had the original concerns about verbosity

[jmacd]

#650 may obviate the need for this PR.

[lizthegrey]

#650 may obviate the need for this PR.

I agree. I still worry about people needing to explicitly type rather than just throwing stuff in, but we can leave that for another time. Can we at least make sure KV takes Stringer rather than just String? I think that address 90% of my problem.