Eliminate tag.Map API

[open-telemetry]

Eliminate tag.Map API

[krnowak]

I'd like to pick up that task.

The question I have is what was the reason for having tag.Map as an interface? Was it to make it possible to have a different implementation of the interface from a different vendor of OT SDK?

From what I have understood from the comment is that the current implementation of the map is not thread-safe and it is not feasible to require thread-safety from the vendor implementations. Is this right?

If being able to provide an different tag.Map implementation by some vendor is still important then maybe it would be possible to achieve the thread safety by something like this:

type Map interface {
	Apply(update MapUpdate) Map
	…
}

type lockedMap struct {
	impl Map
	implLock sync.Mutex
}

var _ Map = &lockedMap{}

func (t &lockedMap) Apply(update MapUpdate) Map {
	t.implLock.Lock()
	defer t.implLock.Unlock()
	return t.impl.Apply(update)
}

// same for the rest of the Map interface methods
…

func NewEmptyMap() Map {
	return &lockedMap{
		impl: tagMap{}
	}
}

So in the end, vendor would need to provide an implementation of the tagMap type.

[jmacd]

I'm not sure there was any reason to have a tag.Map interface in the first place. As I think we've discovered in #52 (see also open-telemetry/oteps#11), there will be uses of the tag.Map interface that are expected before the SDK loads. So, I think we should eliminate the interface and use a concrete type. Then, having an immutable map is sufficient to avoid the problems with map concurrency.

I believe the point about deep-copying bytes is also true, but I want to point out some related work. I have an as-yet-unwritten proposal about supporting structured values and lazy values, for various reasons covered in open-telemetry/opentelemetry-specification#76. The code (in Go) is a proof-of-concept: jmacd#3
see the Key.Struct() and Key.Encode() APIs. There, I included an Evaluate API to ensure that lazy and structure-valued values are copied into bytes before they can be modified. I think you're saying that this should be the case for []byte in the present code base, and I agree.