Skip to content

Commit e08be6c

Browse files
committed
Handle nil tag values gracefully in CompiledMetric
- Use %s placeholder for all tag types (instead of %d/%f) for uniform handling - Sanitize string and symbol values only (sprintf handles conversion for other types) - nil values are handled gracefully (converted to empty string by sprintf)
1 parent eee2938 commit e08be6c

File tree

3 files changed

+21
-17
lines changed

3 files changed

+21
-17
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ section below.
66

77
## Unreleased changes
88

9+
- Add support for `nil` tag values in `CompiledMetric` dynamic tags (converted to empty string).
910
- [#407](https://github.com/Shopify/statsd-instrument/pull/407) - Add support for `Symbol` and `:Boolean` types in `CompiledMetric` dynamic tags, allowing symbol and boolean values to be used as tag values.
1011
- [#403](https://github.com/Shopify/statsd-instrument/pull/403) - Add `CompiledMetric::Distribution` as the second metric type to support pre-compiled metric datagrams. It can be used as a replacement over standard `StatsD.distribution`.
1112
- [#404](https://github.com/Shopify/statsd-instrument/pull/404) - Fix bug when using aggregation; finalizer was called with the wrong values.

lib/statsd/instrument/compiled_metric.rb

Lines changed: 6 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -323,22 +323,11 @@ def compile_static_tags(static_tags)
323323
def compile_dynamic_tags(dynamic_tags)
324324
dynamic_tags.map do |key, type|
325325
tag_name = normalize_statsd_string(key)
326-
placeholder =
327-
if type == String
328-
"%s"
329-
elsif type == Integer
330-
"%d"
331-
elsif type == Float
332-
"%f"
333-
elsif type == :Boolean
334-
"%s"
335-
elsif type == Symbol
336-
"%s"
337-
else
338-
raise ArgumentError,
339-
"Unsupported tag value type: #{type}. Use String, Integer, Float, Symbol, or :Boolean."
340-
end
341-
"#{tag_name}:#{placeholder}"
326+
unless [String, Integer, Float, Symbol, :Boolean].include?(type)
327+
raise ArgumentError,
328+
"Unsupported tag value type: #{type}. Use String, Integer, Float, Symbol, or :Boolean."
329+
end
330+
"#{tag_name}:%s"
342331
end
343332
end
344333
end
@@ -371,7 +360,7 @@ def to_datagram(value)
371360
# Fast path: no tag values (static metrics)
372361
return @datagram_blueprint % packed_value if @tag_values.empty?
373362

374-
# Sanitize string and symbol values
363+
# Sanitize string and symbol values (other types handled by sprintf %s)
375364
values = @tag_values.map do |arg|
376365
if arg.is_a?(String)
377366
/[|,]/.match?(arg) ? arg.tr("|,", "") : arg

test/compiled_metric_test.rb

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -224,6 +224,20 @@ def test_sanitizes_symbol_dynamic_tag_values
224224
assert_equal(["status:activewithspecial"], @sink.datagrams.first.tags)
225225
end
226226

227+
def test_handles_nil_tag_values
228+
metric = Class.new(StatsD::Instrument::CompiledMetric::Counter) do
229+
define(
230+
name: "foo.bar",
231+
tags: { shop_id: Integer, name: String, rate: Float },
232+
)
233+
end
234+
235+
metric.increment(1, shop_id: nil, name: nil, rate: nil)
236+
237+
assert_equal(1, @sink.datagrams.size)
238+
assert_equal(["name:", "rate:", "shop_id:"], @sink.datagrams.first.tags.sort)
239+
end
240+
227241
def test_emits_metric_when_cache_exceeded
228242
# Create a metric with a very small cache size
229243
metric = Class.new(StatsD::Instrument::CompiledMetric::Counter) do

0 commit comments

Comments
 (0)