Repository: honeycombio/honeycomb-kubernetes-agent (go)

Issue:
Add config option to ignore filepaths
<!---
Thank you for contributing an idea to this project!

Please see our [OSS process document](https://github.com/honeycombio/home/blob/main/honeycomb-oss-lifecycle-and-practices.md#) to get an idea of how we operate.
--->

**Is your feature request related to a problem? Please describe.**
The agent currently looks at every file in the container `/var/log` directory to generate events. This is a problem when something (like log rotation) creates a copy of the log file with another extension (like `tmp`), because that results in the agent scanning all the same log lines again, and creating duplicate events. As a side effect, this scenario also manifests in what appears as huge event ingest latency in Honeycomb, because the agent is scanning log lines that happened hours prior.

**Describe the solution you'd like**
Allow users to configure ignored file paths, so they can ignore temp files, as well as compressed files, since those cannot be parsed and result in errors in the agent log.

**Describe alternatives you've considered**
We can just add a warning to the docs to be wary of log rotation, but we don't have a good workaround at this time short of disabling log rotation.

**Additional context**
[internal asana](https://app.asana.com/0/526085303034087/1201979509997282/f)

Pull request description:
Add exclude capability for certain logging paths
Implements an exclusion capability for watchers so that old log files can be ignored after rotation.

## Which problem is this PR solving?

Log rotation can create files that the log system should ignore. This PR allows those changes to be ignored by specifying 
something like this in the configuration for a given watcher:
```yaml
exclude: 
   - /**/*.tmp 
   - /**/*.old
```
in the watcher config file.

Fixes #239.

## Short description of the changes

- Import the `doublestar` library for handling `**` strings in file globs
- Add `exclude` member to WatcherConfig
- Implement an exclusion checker as part of the standard file filter
- Make sure that the generated file filter does the right thing in the different cases
- For symmetry, modify existing "paths" behavior to also support doublestar.
- Add some tests
- Update the import paths that ended in v1, which have changed meaning in newer versions of go

This PR also requires a documentation update, https://github.com/honeycombio/docs/pull/1216, which should be merged after this has been released.

Patch:
diff --git a/config/config.go b/config/config.go
index 879c8fb..f1d7119 100644
--- a/config/config.go
+++ b/config/config.go
@@ -34,6 +34,7 @@ type WatcherConfig struct {
 	// string means "select all pods".
 	LabelSelector *string  `yaml:"labelSelector"`
 	FilePaths     []string `yaml:"paths"`
+	ExcludePaths  []string `yaml:"exclude"`
 	ContainerName string   `yaml:"containerName"`
 	Processors    []map[string]map[string]interface{}
 }
diff --git a/go.mod b/go.mod
index 67048dc..9afbaa1 100644
--- a/go.mod
+++ b/go.mod
@@ -3,6 +3,7 @@ module github.com/honeycombio/honeycomb-kubernetes-agent
 go 1.12
 
 require (
+	github.com/bmatcuk/doublestar/v4 v4.0.2
 	github.com/boltdb/bolt v1.3.1
 	github.com/hashicorp/golang-lru v0.5.4
 	github.com/honeycombio/dynsampler-go v0.2.1
diff --git a/go.sum b/go.sum
index 51238b6..48a5544 100644
--- a/go.sum
+++ b/go.sum
@@ -72,6 +72,8 @@ github.com/beorn7/perks v1.0.1/go.mod h1:G2ZrVWU2WbWT9wwq4/hrbKbnv/1ERSJQ0ibhJ6r
 github.com/bgentry/speakeasy v0.1.0/go.mod h1:+zsyZBPWlz7T6j88CTgSN5bM796AkVf0kBD4zp0CCIs=
 github.com/bketelsen/crypt v0.0.4/go.mod h1:aI6NrJ0pMGgvZKL1iVgXLnfIFJtfV+bKCoqOes/6LfM=
 github.com/blang/semver v3.5.1+incompatible/go.mod h1:kRBLl5iJ+tD4TcOOxsy/0fnwebNt5EWlYSAyrTnjyyk=
+github.com/bmatcuk/doublestar/v4 v4.0.2 h1:X0krlUVAVmtr2cRoTqR8aDMrDqnB36ht8wpWTiQ3jsA=
+github.com/bmatcuk/doublestar/v4 v4.0.2/go.mod h1:xBQ8jztBU6kakFMg+8WGxn0c6z1fTSPVIjEY1Wr7jzc=
 github.com/boltdb/bolt v1.3.1 h1:JQmyP4ZBrce+ZQu0dY660FMfatumYDLun9hBCUVIkF4=
 github.com/boltdb/bolt v1.3.1/go.mod h1:clJnj/oiGkjum5o1McbSZDSLxVThjynRyGBgiAx27Ps=
 github.com/census-instrumentation/opencensus-proto v0.2.1/go.mod h1:f6KPmirojxKA12rnyqOA5BBL4O983OfeGPqjHWSTneU=
diff --git a/k8sagent/watcher.go b/k8sagent/watcher.go
index f04440d..7342960 100644
--- a/k8sagent/watcher.go
+++ b/k8sagent/watcher.go
@@ -9,7 +9,7 @@ import (
 
 	"github.com/sirupsen/logrus"
 
-	"k8s.io/api/core/v1"
+	v1 "k8s.io/api/core/v1"
 	v1types "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
diff --git a/main.go b/main.go
index aee10c4..48615b6 100644
--- a/main.go
+++ b/main.go
@@ -3,13 +3,14 @@ package main
 import (
 	"bytes"
 	"fmt"
-	"github.com/honeycombio/honeycomb-kubernetes-agent/service"
 	"os"
 	"os/signal"
 	"strings"
 	"syscall"
 	"time"
 
+	"github.com/honeycombio/honeycomb-kubernetes-agent/service"
+
 	"github.com/honeycombio/honeycomb-kubernetes-agent/config"
 	"github.com/honeycombio/honeycomb-kubernetes-agent/handlers"
 	"github.com/honeycombio/honeycomb-kubernetes-agent/podtailer"
diff --git a/metrics/processor.go b/metrics/processor.go
index 6e3a124..eaabd8c 100644
--- a/metrics/processor.go
+++ b/metrics/processor.go
@@ -149,7 +149,7 @@ func (p *Processor) MemMetrics(s *stats.MemoryStats, limit float64) Metrics {
 	var utilization float64
 	if limit > 0 {
 		if s.UsageBytes != nil {
-			usage := float64(*s.UsageBytes)
+			usage := float64(*s.WorkingSetBytes)
 			utilization = (usage / limit) * 100
 		}
 	}
diff --git a/podtailer/podtailer.go b/podtailer/podtailer.go
index 48ae09e..12d5384 100644
--- a/podtailer/podtailer.go
+++ b/podtailer/podtailer.go
@@ -10,9 +10,10 @@ import (
 	"sync"
 	"time"
 
-	"k8s.io/api/core/v1"
+	v1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/types"
 
+	"github.com/bmatcuk/doublestar/v4"
 	"github.com/honeycombio/honeycomb-kubernetes-agent/config"
 	"github.com/honeycombio/honeycomb-kubernetes-agent/handlers"
 	"github.com/honeycombio/honeycomb-kubernetes-agent/k8sagent"
@@ -292,12 +293,39 @@ func determineLogPattern(pod *v1.Pod, basePath string, legacyLogPaths bool) (str
 	return "", fmt.Errorf("Could not find specified log path for pod %s", pod.UID)
 }
 
-func determineFilterFunc(pod *v1.Pod, basePath string, containerName string, legacyLogPaths bool) func(fileName string) bool {
+func determineFilterFunc(pod *v1.Pod, basePath string, containerName string, legacyLogPaths bool, excludePaths []string) func(fileName string) bool {
+	// this is an exclude function that conforms to the filter specs -- it returns true
+	// if the file should be *included*. By default, it's nil (which is treated as true)
+	var exclusionFilter func(string) bool
+	if len(excludePaths) > 0 {
+		excludes := make([]string, 0)
+		for _, exclude := range excludePaths {
+			// don't try to evaluate patterns that we can't even parse
+			if doublestar.ValidatePattern(exclude) {
+				excludes = append(excludes, exclude)
+				logrus.WithField("exclude", exclude).Debug("Excluding path")
+			}
+		}
+		exclusionFilter = func(fileName string) bool {
+			// check if we should exclude this path
+			for _, exclude := range excludes {
+				shouldExclude, _ := doublestar.Match(exclude, fileName)
+				if shouldExclude {
+					return false // false means exclude
+				}
+			}
+			return true
+		}
+	}
 
 	if containerName == "" {
-		logrus.Debug("No container name specified, no filter function needed")
-		return nil
+		if len(excludePaths) == 0 {
+			logrus.Debug("No container name or exclude paths specified, no filter function needed")
+			return nil
+		}
+		return exclusionFilter
 	}
+
 	if legacyLogPaths {
 		re := fmt.Sprintf(
 			"^%s/containers/%s_%s_%s-.+\\.log",
@@ -311,6 +339,11 @@ func determineFilterFunc(pod *v1.Pod, basePath string, containerName string, leg
 		}).Debug("Container filter function")
 
 		return func(fileName string) bool {
+			// first check if we should exclude this path
+			// If exclusionFilter says no, we're going to say no.
+			if exclusionFilter != nil && !exclusionFilter(fileName) {
+				return false
+			}
 			ok, _ := regexp.Match(re, []byte(fileName))
 			return ok
 		}
@@ -336,15 +369,21 @@ func determineFilterFunc(pod *v1.Pod, basePath string, containerName string, leg
 	}).Debug("Container filter function")
 
 	return func(fileName string) bool {
-		ok, _ := regexp.Match(re1, []byte(fileName))
+		// first check if we should exclude this path
+		// If exclusionFilter says no, we're going to say no.
+		if exclusionFilter != nil && !exclusionFilter(fileName) {
+			return false
+		}
+		// now check our patterns -- if any of them succeeds, we're good
+		ok, _ := regexp.MatchString(re1, fileName)
 		if ok {
 			return ok
 		}
-		ok, _ = regexp.Match(re2, []byte(fileName))
+		ok, _ = regexp.MatchString(re2, fileName)
 		if ok {
 			return ok
 		}
-		ok, _ = regexp.Match(re3, []byte(fileName))
+		ok, _ = regexp.MatchString(re3, fileName)
 		return ok
 	}
 }
@@ -358,7 +397,7 @@ func (pt *PodSetTailer) watcherForPod(pod *v1.Pod, containerName string, podWatc
 
 	// only watch logs for containers matching the given name, if
 	// one is specified
-	filterFunc := determineFilterFunc(pod, logsBasePath, containerName, pt.legacyLogPaths)
+	filterFunc := determineFilterFunc(pod, logsBasePath, containerName, pt.legacyLogPaths, pt.config.ExcludePaths)
 
 	additionalProcessors := []processors.Processor{
 		&processors.KubernetesMetadataProcessor{
diff --git a/tailer/tailer.go b/tailer/tailer.go
index d6dd6fb..38b0bd9 100644
--- a/tailer/tailer.go
+++ b/tailer/tailer.go
@@ -3,16 +3,16 @@
 package tailer
 
 import (
+	"os"
 	"path/filepath"
 	"sync"
 	"time"
 
+	"github.com/bmatcuk/doublestar/v4"
 	"github.com/honeycombio/honeycomb-kubernetes-agent/handlers"
 	"github.com/hpcloud/tail"
 
 	"github.com/sirupsen/logrus"
-
-	"k8s.io/api/core/v1"
 )
 
 // Tailer tails a single file, passing each line off to the handler.
@@ -123,7 +123,6 @@ type PathWatcher struct {
 	handlerFactory handlers.LineHandlerFactory
 	stateRecorder  StateRecorder
 	checkInterval  time.Duration
-	pod            *v1.Pod
 
 	stop         chan bool
 	savedPattern string
@@ -171,6 +170,22 @@ func (p *PathWatcher) Stop() {
 	}
 }
 
+// This emulates filepath.Glob's behavior using doublestar,
+// which means that it now supports /**/ names in its paths.
+func doublestarGlob(pat string) ([]string, error) {
+	base, pattern := doublestar.SplitPattern(pat)
+	fsys := os.DirFS(base)
+	files, err := doublestar.Glob(fsys, pattern)
+	if err != nil {
+		return nil, err
+	}
+	var result []string
+	for _, f := range files {
+		result = append(result, filepath.Join(base, f))
+	}
+	return result, nil
+}
+
 func (p *PathWatcher) check() {
 	// Have we figured out the pattern yet? If not, run our pattern function
 	if p.savedPattern == "" {
@@ -188,7 +203,7 @@ func (p *PathWatcher) check() {
 			"log pattern": pt,
 		}).Info("Log pattern")
 	}
-	files, err := filepath.Glob(p.savedPattern)
+	files, err := doublestarGlob(p.savedPattern)
 	if err != nil {
 		logrus.WithError(err).Error("Error globbing files")
 	}
@@ -204,7 +219,7 @@ func (p *PathWatcher) check() {
 			if p.filter != nil && !p.filter(file) {
 				logrus.WithFields(logrus.Fields{
 					"file": file,
-				}).Warn("File filtered out of tailing list based on containerName")
+				}).Warn("File filtered out of tailing list")
 				continue
 			}
 			handler := p.handlerFactory.New(file)


Test patch:
diff --git a/handlers/handlers_test.go b/handlers/handlers_test.go
index 954cedc..9208f12 100644
--- a/handlers/handlers_test.go
+++ b/handlers/handlers_test.go
@@ -10,7 +10,7 @@ import (
 	"testing"
 	"time"
 
-	"k8s.io/api/core/v1"
+	v1 "k8s.io/api/core/v1"
 	v1types "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
 
diff --git a/metrics/metrics_test.go b/metrics/metrics_test.go
index bcd217f..9a61b98 100644
--- a/metrics/metrics_test.go
+++ b/metrics/metrics_test.go
@@ -222,7 +222,7 @@ func TestMemMetrics(t *testing.T) {
 	require.Equal(t, 7, len(metrics))
 
 	assert.Equal(t, float64(2143281152), metrics[MeasureMemoryUsage].GetValue(), "Memory Usage")
-	assert.InDelta(t, 0.000001, 99.804300603, metrics[MeasureMemoryUtilization].GetValue(), "Memory Utilization")
+	assert.Equal(t, 97.79205322265625, metrics[MeasureMemoryUtilization].GetValue(), "Memory Utilization")
 	assert.Equal(t, float64(23191552), metrics[MeasureMemoryRSS].GetValue(), "Memory RSS")
 }
 
diff --git a/podtailer/podtailer_test.go b/podtailer/podtailer_test.go
index d11fa91..95e48db 100644
--- a/podtailer/podtailer_test.go
+++ b/podtailer/podtailer_test.go
@@ -9,7 +9,7 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
-	"k8s.io/api/core/v1"
+	v1 "k8s.io/api/core/v1"
 )
 
 // test patterns after https://github.com/kubernetes/kubernetes/pull/74441
@@ -130,7 +130,7 @@ func TestDetermineFilterFuncLegacyLogs(t *testing.T) {
 		return ok
 	}
 
-	f := determineFilterFunc(pod, "/var/log", "wibble", true)
+	f := determineFilterFunc(pod, "/var/log", "wibble", true, nil)
 	// functions should have same output
 	assert.Equal(
 		t,
@@ -160,7 +160,7 @@ func TestDetermineFilterFunc(t *testing.T) {
 		return ok
 	}
 
-	f := determineFilterFunc(pod, "/var/log", "wibble", false)
+	f := determineFilterFunc(pod, "/var/log", "wibble", false, nil)
 	assert.Equal(
 		t,
 		expected("/var/log/pods/123456/wibble_0.log"),
@@ -192,7 +192,7 @@ func TestDetermineFilterFuncK8S1Dot10(t *testing.T) {
 		return ok
 	}
 
-	f := determineFilterFunc(pod, "/var/log", "wibble", false)
+	f := determineFilterFunc(pod, "/var/log", "wibble", false, nil)
 	assert.Equal(
 		t,
 		expected("/var/log/pods/123456/wibble/0.log"),
@@ -236,7 +236,7 @@ func TestDetermineFilterFuncK8S74441(t *testing.T) {
 	_, err = os.Create(file2)
 	assert.Nil(t, err)
 
-	f := determineFilterFunc(pod, basePath, "container1", false)
+	f := determineFilterFunc(pod, basePath, "container1", false, nil)
 
 	assert.Equal(
 		t,
@@ -250,3 +250,56 @@ func TestDetermineFilterFuncK8S74441(t *testing.T) {
 		false,
 	)
 }
+
+func TestDetermineFilterFuncExclude(t *testing.T) {
+	t.Parallel()
+	pod := &v1.Pod{}
+	pod.Namespace = "asdf"
+	pod.Name = "foo"
+	pod.UID = "123456"
+	podDirName := fmt.Sprintf("%s_%s_%s", pod.Namespace, pod.Name, pod.UID)
+
+	basePath, err := ioutil.TempDir("", "")
+	assert.Nil(t, err)
+	defer os.RemoveAll(basePath)
+
+	err = os.Mkdir(filepath.Join(basePath, "pods"), 0777)
+	assert.Nil(t, err)
+	err = os.Mkdir(filepath.Join(basePath, "pods", podDirName), 0777)
+	assert.Nil(t, err)
+	err = os.Mkdir(filepath.Join(basePath, "pods", podDirName, "container1"), 0777)
+	assert.Nil(t, err)
+	err = os.Mkdir(filepath.Join(basePath, "pods", podDirName, "container2"), 0777)
+	assert.Nil(t, err)
+
+	tests := []struct {
+		path string
+		name string
+		want bool
+	}{
+		{"container1", "0.log", true},
+		{"container1", "1.log.exclude1only", false},
+		{"container1", "1.log.excludeAlways", false},
+		{"container2", "0.log", true},
+		{"container2", "0.log.excludeAlways", false},
+		{"container2", "1.log", true},
+		{"container2", "1.log.exclude1only", true},
+		{"container2", "1.log.excludeAlways", false},
+	}
+
+	f := determineFilterFunc(pod, basePath, "", false, []string{"**/*.excludeAlways", "**/container1/*.exclude1only"})
+
+	for _, tc := range tests {
+		t.Run(filepath.Join(tc.path, tc.name), func(t *testing.T) {
+			file1 := filepath.Join(basePath, "pods", podDirName, tc.path, tc.name)
+			_, err = os.Create(file1)
+			assert.Nil(t, err)
+
+			assert.Equal(
+				t,
+				tc.want,
+				f(file1),
+			)
+		})
+	}
+}
diff --git a/tailer/tailer_test.go b/tailer/tailer_test.go
index e1136dd..d92b885 100644
--- a/tailer/tailer_test.go
+++ b/tailer/tailer_test.go
@@ -5,6 +5,7 @@ import (
 	"io/ioutil"
 	"os"
 	"path"
+	"path/filepath"
 	"testing"
 	"time"
 
@@ -150,8 +151,9 @@ func TestPathWatching(t *testing.T) {
 	time.Sleep(time.Second)
 	watcher.Stop()
 
-	assert.Equal(t, len(handlerFactory.handlers), 2)
-	for _, h := range handlerFactory.handlers {
+	assert.Equal(t, 2, len(handlerFactory.handlers))
+	for k, h := range handlerFactory.handlers {
+		fmt.Println(k)
 		assert.Equal(t, 2, len(h.lines))
 	}
 	assert.Equal(t, len(watcher.tailers), 0)
@@ -165,9 +167,30 @@ func TestTailingWithoutStateRecorder(t *testing.T) {
 	handler := &mockLineHandler{}
 
 	logFile, err := ioutil.TempFile("/tmp", "honeycomb-log-test")
+	assert.NoError(t, err)
 	logFile.Write([]byte("line1\n"))
 	logFile.Sync()
 	tailer := NewTailer(logFile.Name(), handler, stateRecorder)
 	tailer.Run()
 	tailer.Stop()
 }
+
+func TestGlobbing(t *testing.T) {
+	paths := []string{
+		"/var/log/*",
+		"*",
+		".",
+	}
+
+	for _, p := range paths {
+		t.Run(p, func(t *testing.T) {
+			oldfiles, err := filepath.Glob(p)
+			assert.NoError(t, err)
+
+			newfiles, err := doublestarGlob(p)
+			assert.NoError(t, err)
+
+			assert.Equal(t, oldfiles, newfiles)
+		})
+	}
+}