Span#set_attribute with strings & symbols

[indrekj]

Span#set_attribute currently accepts both symbols and strings for both keys and values.

We had a similar discussion here: opentracing/opentracing-ruby#35 and went with only strings. Currently we can still decide what's the best approach.

The problem with allowing both strings and symbols as attribute keys has two main problems:

• memory allocation in case we need to convert one to other
• what happens if somebody does span[:foo] = "bar"; span["foo"] = "baz". who wins?

[mwear]

As the person who opened that issue, I'll add a few comments here. The OpenTracing Issue is worth reading. I really want to be able to use symbols in methods like set_attribute. They are the most natural and idiomatic code to write as a Rubyist. However, this flexibility comes at a pretty big penalty, both in terms of API complexity (for the Tracer) and performance, by way of object allocations. Ultimately symbol keys will have to be turned into strings at serialization time. The best way to avoid object allocations in this scenario is for attributes to be frozen strings. Because of this, I would be in favor of changing Span#set_attribute to accept strings and not symbols.

[mwear]

We discussed this at the SIG meeting and I wanted to summarize what we talked about.

With the flexibility of accepting strings or symbols for attribute keys / values comes some complications:

• string allocations - Symbols will need to be converted to strings at serialization time
• key clashes - what happens when both a string and symbol key is specified with an equivalent representation, i.e. span[:foo] = "bar"; span["foo"] = "baz"

A tracer could work around these issues by maintaining a symbol to frozen string cache to reduce string allocations and establishing a rule that keys with equivalent string-symbol representations will be treated the same ('foo' would be handled the same as :foo).

The most performant and simple solution would be for the API to only accept strings, and for users to use frozen strings. In order to take advantage of this, users will have to careful to freeze their strings, and handle symbol to string conversion themselves.

There is a tradeoff between the flexibility of using strings and symbols in terms of simplicity and performance.

@luvtechno suggested that we could start by only accepting strings and it would be possible to relax the API to accept symbols in the future, if we decided we wanted to go that direction.

[fbogsany]

Implemented suggestion (remove Symbol support) in #45