TagDrop does not work as documented

[zhang8473]

Bug report

Current behavior (here is your code):

func (f *Filter) shouldTagsPass(tags map[string]string) bool {
if f.TagPass != nil {
for _, pat := range f.TagPass {
if pat.filter == nil {
continue
}
if tagval, ok := tags[pat.Name]; ok {
if pat.filter.Match(tagval) {
return true
}
}
}
return false
}

if f.TagDrop != nil {
	for _, pat := range f.TagDrop {
		if pat.filter == nil {
			continue
		}
		if tagval, ok := tags[pat.Name]; ok {
			if pat.filter.Match(tagval) {
				return false
			}
		}
	}
	return true
}

return true

}

Desired behavior:

tagdrop: The inverse of tagpass. If a match is found the point is discarded. This is tested on points after they have passed the tagpass test.

e.g. I have:
abc, tag1=1, tag2=2 values tms
abc, tag1=1, tag2=3 values tms
abc, tag1=8, tag2=2 values tms

I want all metrics with tag1=1 and tag2!=3.

Based on your document, I could set tagpass tag1=1 and set tagdrop tag2=3. However, based on your code, it is not possible.

Use case: [Why is this important (helps with prioritizing requests)]

further filter tags after tagpass

[danielnelson]

I'm sorry but I don't understand what behavior you are expecting. Perhaps you can write an example that show what happens and what you think is going to happen. With tagdrop the entire point is dropped.

[zhang8473]

I added an example in the original poster.

[danielnelson]

Okay let me try to reiterate the expectations:

Config:

[[outputs.file]]
  files = ["stdout"]
  data_format = "influx"
  [outputs.file.tagpass]
    tag1 = ["1"]
  [outputs.file.tagdrop]
    tag2 = ["3"]

Input:

abc,tag1=1,tag2=2 value=42 1234567890123456789
abc,tag1=1,tag2=3 value=42 1234567890123456789
abc,tag1=8,tag2=2 value=42 1234567890123456789

Actual Output:

abc,tag2=2,tag1=1 value=42 1234567890123456789
abc,tag2=3,tag1=1 value=42 1234567890123456789

Expected Output:

abc,tag2=2,tag1=1 value=42 1234567890123456789

Yeah, looks like a bug to me.

[dsalbert]

Hi,

I have a tested code + unit test to solve this issue.

// shouldTagsPass returns true if the metric should pass, false if should drop
// based on the tagdrop/tagpass filter parameters
func (f *Filter) shouldTagsPass(tags map[string]string) bool {

	tagPass := func(f *Filter) bool {
		for _, pat := range f.TagPass {
			if pat.filter == nil {
				continue
			}
			if tagval, ok := tags[pat.Name]; ok {
				if pat.filter.Match(tagval) {
					return true
				}
			}
		}
		return false
	}

	tagDrop := func(f *Filter) bool {
		for _, pat := range f.TagDrop {
			if pat.filter == nil {
				continue
			}
			if tagval, ok := tags[pat.Name]; ok {
				if pat.filter.Match(tagval) {
					return false
				}
			}
		}
		return true
	}

	// Add additional logic in case where both parameters are set.
	// see: https://github.com/influxdata/telegraf/issues/2860
	if f.TagPass != nil && f.TagDrop != nil {
		// return true only in case when tag pass and won't be dropped (true, true).
		// in case when the same tag should be passed and dropped it will be dropped (true, false).
		return tagPass(f) && tagDrop(f)
	} else if f.TagPass != nil {
		return tagPass(f)
	} else if f.TagDrop != nil {
		return tagDrop(f)
	}

	return true
}

I see that the same behavior is present for NamePass and FieldPass (return from function in conditional block when Pass is defined). According to documentation, there is an information for namedrop and tagdrop that says, that drop is going to be verify on points after they have passed the namepass/tagpass test (there is no such information for fieldpass). With this fix, that will be true for tagpass, but we still need to fix logic in shouldNamePass function:

// shouldNamePass returns true if the metric should pass, false if should drop
// based on the drop/pass filter parameters
func (f *Filter) shouldNamePass(key string) bool {
	if f.namePass != nil {
		if f.namePass.Match(key) {
			return true
		}
		return false
	}

	if f.nameDrop != nil {
		if f.nameDrop.Match(key) {
			return false
		}
	}
	return true
}

Please let me know what do you think.

[danielnelson]

Thanks @DanKans, I merged your fix and it will be in 1.4.

Sounds like we need to fix namedrop/namepass and fielddrop/fieldpass as well as update the documentation for fielddrop.

[dsalbert]

I'm going to work on it and I will prepare a proper MR for this. We can keep this issue open to track further work on this.

Thanks!

[danielnelson]

The field and name filtering is now also fixed, thanks to @DanKans. These will also be in 1.4