diff --git a/lib/fluent/plugin/prometheus/placeholder_expander.rb b/lib/fluent/plugin/prometheus/placeholder_expander.rb index 53f5d28..565c4b8 100644 --- a/lib/fluent/plugin/prometheus/placeholder_expander.rb +++ b/lib/fluent/plugin/prometheus/placeholder_expander.rb @@ -2,6 +2,10 @@ module Fluent module Plugin module Prometheus class ExpandBuilder + # ${tag}, ${tag_parts[...]}, ${tag_prefix[...]} and ${tag_suffix[...]} + # must be built from the tag only. + TAG_DERIVED_PLACEHOLDER = /\A\$\{tag(_parts|_prefix|_suffix)?(\[[^\]]*\])?\}\z/.freeze + def self.build(placeholder, log:) new(log: log).build(placeholder) end @@ -12,6 +16,7 @@ def initialize(log:) def build(placeholder_values) placeholders = {} + tag_placeholders = {} placeholder_values.each do |key, value| case value when Array @@ -26,13 +31,22 @@ def build(placeholder_values) end else if key == 'tag' - placeholders.merge!(build_tag(value)) + tag_placeholders = build_tag(value) else placeholders["${#{key}}"] = value end end end + # A record may have an array named "tag_parts", or a key named + # "tag_parts[0]". Both make the same placeholder as the tag does. + # merge! is not enough here, because a record can also use an index + # which build_tag does not make, such as ${tag_parts[3]} for a tag + # of 3 parts, or ${tag_prefix[-1]}. So remove them first, then such + # a placeholder stays unknown. + placeholders.delete_if { |k, _| TAG_DERIVED_PLACEHOLDER.match?(k) } + placeholders.merge!(tag_placeholders) + Fluent::Plugin::Prometheus::ExpandBuilder::PlaceholderExpander.new(@log, placeholders) end diff --git a/spec/fluent/plugin/filter_prometheus_spec.rb b/spec/fluent/plugin/filter_prometheus_spec.rb index f98c8c6..c22c884 100644 --- a/spec/fluent/plugin/filter_prometheus_spec.rb +++ b/spec/fluent/plugin/filter_prometheus_spec.rb @@ -45,4 +45,73 @@ it_behaves_like 'instruments record' end + + # a label which uses the tag must be expanded with the tag, not with the + # record. + describe 'a record which has a tag placeholder name' do + let(:config) { + BASE_CONFIG + %( + + name tagged + type counter + desc Something foo. + key foo + + part ${tag_parts[0]} + + + ) + } + + it 'labels the metric with the tag' do + driver.run(default_tag: tag) do + driver.feed(event_time, {'tag' => 'forged.tag', 'foo' => 1, 'tag_parts' => ['forged']}) + end + # even though tag_parts exists in the record, it should not be taken as "part". + expect(registry.get(:tagged).values.keys).to eq([{part: 'prometheus'}]) + end + + it 'labels the metric with the tag, not with the record key' do + driver.run(default_tag: tag) do + driver.feed(event_time, {'tag' => 'forged.tag', 'foo' => 1, 'tag_parts[0]' => 'forged'}) + end + # the record has a key named "tag_parts[0]", but the tag must win. + expect(registry.get(:tagged).values.keys).to eq([{part: 'prometheus'}]) + end + end + + # a record must not fill an index which the tag does not have. + describe 'a record which uses an index out of the tag' do + let(:config) { + BASE_CONFIG + %( + + name out_of_range + type counter + desc Something foo. + key foo + + part ${tag_parts[2]} + prefix ${tag_prefix[-1]} + suffix ${tag_suffix[-1]} + + + ) + } + + it 'leaves the label unexpanded' do + driver.run(default_tag: tag) do + driver.feed(event_time, { + 'foo' => 1, + 'tag_parts[2]' => 'forged', + 'tag_prefix[-1]' => 'forged', + 'tag_suffix[-1]' => 'forged', + }) + end + # the tag "prometheus.test" has 2 parts, and tag_prefix and tag_suffix + # have no negative index. So these labels must not be expanded. + expect(registry.get(:out_of_range).values.keys).to eq( + [{part: '${tag_parts[2]}', prefix: '${tag_prefix[-1]}', suffix: '${tag_suffix[-1]}'}] + ) + end + end end diff --git a/spec/fluent/plugin/prometheus/placeholder_expander_spec.rb b/spec/fluent/plugin/prometheus/placeholder_expander_spec.rb index aea1565..d949156 100644 --- a/spec/fluent/plugin/prometheus/placeholder_expander_spec.rb +++ b/spec/fluent/plugin/prometheus/placeholder_expander_spec.rb @@ -50,6 +50,94 @@ expander.expand('${hostname}') end + # tag_parts, tag_prefix and tag_suffix must be expanded with the tag, + # not with "forged" + context 'with a value named after a tag placeholder' do + let(:forged_placeholder) do + { + 'tag' => '1.2.3', + 'tag_parts' => %w[forged forged forged], + 'tag_prefix' => %w[forged forged forged], + 'tag_suffix' => %w[forged forged forged], + } + end + + it 'expands the placeholders with the tag' do + expander = builder.build(forged_placeholder) + + expect(expander.expand('${tag_parts[0]}.${tag_parts[1]}.${tag_parts[2]}')).to eq('1.2.3') + expect(expander.expand('${tag_parts[-3]}.${tag_parts[-2]}.${tag_parts[-1]}')).to eq('1.2.3') + expect(expander.expand('${tag_prefix[0]},${tag_prefix[1]},${tag_prefix[2]}')).to eq('1,1.2,1.2.3') + expect(expander.expand('${tag_suffix[0]},${tag_suffix[1]},${tag_suffix[2]}')).to eq('3,2.3,1.2.3') + end + + it 'does not expand an index which the tag does not have' do + # tag_prefix and tag_suffix have no negative index, so they are kept + # as they are. This checks that the record does not fill them. + expander = builder.build(forged_placeholder) + + expect(expander.expand('${tag_prefix[-1]}')).to eq('${tag_prefix[-1]}') + expect(expander.expand('${tag_suffix[-1]}')).to eq('${tag_suffix[-1]}') + + # the tag has 3 parts, so the 4th value of the record is out of it. + longer = forged_placeholder.merge('tag_parts' => %w[forged forged forged forged]) + expander = builder.build(longer) + + expect(expander.expand('${tag_parts[3]}')).to eq('${tag_parts[3]}') + expect(expander.expand('${tag_parts[-4]}')).to eq('${tag_parts[-4]}') + end + end + + # a record may also have a key named "tag_parts[0]", which makes the + # same placeholder as the tag + context 'with a value whose key is a tag placeholder' do + it 'expands the placeholders with the tag' do + spelled = { + 'tag' => '1.2.3', + 'tag_parts[0]' => 'forged', + 'tag_prefix[0]' => 'forged', + 'tag_suffix[0]' => 'forged', + } + expander = builder.build(spelled) + + expect(expander.expand('${tag_parts[0]}')).to eq('1') + expect(expander.expand('${tag_prefix[0]}')).to eq('1') + expect(expander.expand('${tag_suffix[0]}')).to eq('3') + end + + it 'does not expand an index which the tag does not have' do + spelled = { + 'tag' => '1', + 'tag_parts[1]' => 'forged', + 'tag_parts[-2]' => 'forged', + 'tag_prefix[-1]' => 'forged', + 'tag_suffix[-1]' => 'forged', + } + expander = builder.build(spelled) + + expect(expander.expand('${tag_parts[1]}')).to eq('${tag_parts[1]}') + expect(expander.expand('${tag_parts[-2]}')).to eq('${tag_parts[-2]}') + expect(expander.expand('${tag_prefix[-1]}')).to eq('${tag_prefix[-1]}') + expect(expander.expand('${tag_suffix[-1]}')).to eq('${tag_suffix[-1]}') + end + + it 'keeps the tag itself' do + spelled = { + 'tag' => '1.2.3', + 'tag_parts' => 'forged', + 'tag_prefix' => 'forged', + 'tag_suffix' => 'forged', + } + expander = builder.build(spelled) + + expect(expander.expand('${tag}')).to eq('1.2.3') + # these are not built from the tag, so they are kept as they are + expect(expander.expand('${tag_parts}')).to eq('${tag_parts}') + expect(expander.expand('${tag_prefix}')).to eq('${tag_prefix}') + expect(expander.expand('${tag_suffix}')).to eq('${tag_suffix}') + end + end + context 'when not found placeholder' do it 'prints wanring log and as it is' do expect(log).to receive(:warn).with('unknown placeholder `${tag_prefix[100]}` found').once @@ -87,6 +175,14 @@ expect(expander.expand('${tag_suffix[0]}.${tag_suffix[1]}.${tag_suffix[2]}', dynamic_placeholders: dynamic_placeholder)).to eq('3.2.3.1.2.3') end + it 'expands the placeholders with the dynamic tag' do + forged = static_placeholder.merge('tag_parts' => %w[forged forged forged]) + expander = builder.build(forged) + + expect(expander.expand('${tag_parts[0]}.${tag_parts[1]}.${tag_parts[2]}', + dynamic_placeholders: dynamic_placeholder)).to eq('1.2.3') + end + it 'does not create expander twice if given the same placeholder' do builder # cached before mock