to_s all the tags

[jacobbednarz]

During a migration from 1.6 to 3.0 we discovered that the ability to use
symbols as tags is no longer available and raises a TypeError: can't dup Symbol exception. The reason for this is that we are running each of the
tags through a block and calling #dup on each value without checking what
data type it is. It's a fair assumption that all values
should be strings however our application has shown that this isn't the
case and we the DataDog collector handles this fine in 1.6 so I would
expect the same to be true in 3.0 unless explicitly stated otherwise.

To address the issue here I'm running each value via #to_s before we pass
it to #dup which will ensure that it never gets a symbol and chokes.

Closes #50.

[jacobbednarz]

Using the excellent ground work from @janester in this gist, I did benchmark this and an alternative which looks (slightly) better but does cause a bunch more allocations + memory.

This PR (3bc17a5)

Calculating -------------------------------------
              master    80.520M memsize (     0.000  retained)
                         1.403M objects (     0.000  retained)
                        50.000  strings (     0.000  retained)
            proposed    80.316M memsize (     0.000  retained)
                         1.402M objects (     0.000  retained)
                        50.000  strings (     0.000  retained)

Comparison:
            proposed:   80316435 allocated
              master:   80519933 allocated - 1.00x more

Using .map(&:to_s)

--- a/lib/datadog/statsd.rb
+++ b/lib/datadog/statsd.rb
@@ -410,7 +410,7 @@ module Datadog
         end


-        tag_arr = opts[:tags].to_a
+        tag_arr = opts[:tags].to_a.map(&:to_s)
         tag_arr.map! { |tag| t = tag.dup; escape_tag_content!(t); t }
         ts = tags.to_a + tag_arr
         unless ts.empty?

Calculating -------------------------------------
              master    80.298M memsize (     0.000  retained)
                         1.402M objects (     0.000  retained)
                        50.000  strings (     0.000  retained)
            proposed    84.301M memsize (     0.000  retained)
                         1.502M objects (     0.000  retained)
                        50.000  strings (     0.000  retained)

Comparison:
              master:   80298488 allocated
            proposed:   84301045 allocated - 1.05x more

[pschambacher]

@jacobbednarz I'm addressing the same issue with #53

[degemer]

@jacobbednarz @pschambacher Thanks for the contributions!

I think both are needed, as @pschambacher's works for events/service checks + class level tags, but not for the specific use case of @jacobbednarz, @statsd.increment('stat', tags: [:sample_tag]).

Thanks for the benchmark, the current method LGTM!

[mikhailov]

@degemer I'm interested in this PR's status...

[jeremy-lq]

@degemer Do we need anything else to get this and/or #53 merged?

[zippolyte]

Hi @jacobbednarz, i'm going to merge this, thanks for your contribution.

[mikhailov]

@zippolyte I wonder if you have a date when git tag with the changes above will be released?

[mikhailov]

It would be awesome if you can release a patch v3.0.1 with the changes from that PR and #53 one together!