extension: ecssd Make default config values internal

- Fix typo
- Add more comment on how the matcher merge logic works

extension/observer/ecsobserver/config.go

@@ -22,10 +22,10 @@ import (
 )
 
 const (
-	DefaultRefreshInterval             = 30 * time.Second
-	DefaultJobLabelName                = "prometheus_job"
-	AWSRegionEnvKey                    = "AWS_REGION"
-	DefaultDockerLabelMatcherPortLabel = "ECS_PROMETHEUS_EXPORTER_PORT"
+	defaultRefreshInterval             = 30 * time.Second
+	defaultJobLabelName                = "prometheus_job"
+	awsRegionEnvKey                    = "AWS_REGION"
+	defaultDockerLabelMatcherPortLabel = "ECS_PROMETHEUS_EXPORTER_PORT"
 )
 
 type Config struct {
@@ -60,13 +60,13 @@ func DefaultConfig() Config {
 			NameVal: string(typeStr),
 		},
 		ClusterName:     "default",
-		ClusterRegion:   os.Getenv(AWSRegionEnvKey),
+		ClusterRegion:   os.Getenv(awsRegionEnvKey),
 		ResultFile:      "/etc/ecs_sd_targets.yaml",
-		RefreshInterval: DefaultRefreshInterval,
-		JobLabelName:    DefaultJobLabelName,
+		RefreshInterval: defaultRefreshInterval,
+		JobLabelName:    defaultJobLabelName,
 		DockerLabels: []DockerLabelConfig{
 			{
-				PortLabel: DefaultDockerLabelMatcherPortLabel,
+				PortLabel: defaultDockerLabelMatcherPortLabel,
 			},
 		},
 	}
@@ -80,7 +80,7 @@ func ExampleConfig() Config {
 		ClusterRegion:   "us-west-2",
 		ResultFile:      "/etc/ecs_sd_targets.yaml",
 		RefreshInterval: 15 * time.Second,
-		JobLabelName:    DefaultJobLabelName,
+		JobLabelName:    defaultJobLabelName,
 		Services: []ServiceConfig{
 			{
 				NamePattern: "^retail-.*$",

extension/observer/ecsobserver/fetcher.go

@@ -22,8 +22,8 @@ import (
 )
 
 type Fetcher interface {
-	// FetcherAndDecorate fetches all the tasks and attach addation information
-	// like definition, serivces and container instances.
+	// FetcherAndDecorate fetches all the tasks and attach additional information
+	// like definition, services and container instances.
 	FetchAndDecorate(ctx context.Context) ([]*Task, error)
 }
 

extension/observer/ecsobserver/filter.go

@@ -35,7 +35,9 @@ func NewTaskFilter(c Config, logger *zap.Logger, matchers map[MatcherType][]Matc
 }
 
 // Filter run all the matchers and return all the tasks that including at least one matched container.
+// It updates those matched tasks in place. (i.e. it does not do shallow copy).
 func (f *TaskFilter) Filter(tasks []*Task) ([]*Task, error) {
+	// Gather results from all the matchers
 	matched := make(map[MatcherType][]*MatchResult)
 	var merr error
 	for tpe, matchers := range f.matchers {
@@ -55,12 +57,16 @@ func (f *TaskFilter) Filter(tasks []*Task) ([]*Task, error) {
 		}
 	}
 
-	matchedTasks := make(map[int]bool)
+	// Dedupe, key is task index
+	matchedTasks := make(map[int]struct{})
 	for _, tpe := range matcherOrders() {
 		for _, res := range matched[tpe] {
 			for _, container := range res.Containers {
-				matchedTasks[container.TaskIndex] = true
+				matchedTasks[container.TaskIndex] = struct{}{}
 				task := tasks[container.TaskIndex]
+				// Merge mached containers into the task, this is in place update.
+				// Each task can contain match result from different matchers on different containers.
+				// Same container can also contain multiple targets。
 				task.AddMatchedContainer(container)
 			}
 		}

extension/observer/ecsobserver/matcher.go

@@ -101,7 +101,7 @@ func matcherOrders() []MatcherType {
 }
 
 func newMatchers(c Config, mOpt MatcherOptions) (map[MatcherType][]Matcher, error) {
-	// We can have a registry or factory methods etc. but since we only have three type of metchers in filter.
+	// We can have a registry or factory methods etc. but since we only have three type of matchers in filter.
 	matcherConfigs := map[MatcherType][]MatcherConfig{
 		MatcherTypeDockerLabel: dockerLabelConfigToMatchers(c.DockerLabels),
 	}
@@ -140,7 +140,7 @@ func matchContainers(tasks []*Task, matcher Matcher, matcherIndex int) (*MatchRe
 		var matched []MatchedContainer
 		for cIndex, c := range t.Definition.ContainerDefinitions {
 			targets, err := matcher.MatchTargets(t, c)
-			// NOTE: we don't stop when there is an error becaause it could be one task has invalid docker label.
+			// NOTE: we don't stop when there is an error because it could be one task has invalid docker label.
 			if err != nil {
 				// Keep track of unexpected error
 				if err != errNotMatched {

extension/observer/ecsobserver/sd_test.go

@@ -149,7 +149,7 @@ func TestServiceDiscovery_RunAndWriteFile(t *testing.T) {
 		ClusterRegion:   "us-test-2",
 		RefreshInterval: 100 * time.Millisecond,
 		ResultFile:      outputFile,
-		JobLabelName:    DefaultJobLabelName,
+		JobLabelName:    defaultJobLabelName,
 		DockerLabels: []DockerLabelConfig{
 			{
 				PortLabel: "PROMETHEUS_PORT",

extension/observer/ecsobserver/task.go

@@ -32,13 +32,26 @@ type Task struct {
 	Matched    []MatchedContainer
 }
 
+// AddMatchedContainer merge match result from Matcher with existing containers.
+// Each container can contain multiple targets, it is allowed (but not suggested),
+// that different MatcherConfig matches same container with different targets (even on same port).
+// For example you can use it to workaround multiple metrics_path for same port.
+// services:
+//   - name: nginx
+//	   metrics_path: /foo
+//	   metrics_port: [9000]
+//   - name: nginx
+//     metrics_path: /foo/v2
+//	   metrics_port: [9000]
 func (t *Task) AddMatchedContainer(newContainer MatchedContainer) {
+	// Merge targets for existing containers
 	for i, oldContainer := range t.Matched {
 		if oldContainer.ContainerIndex == newContainer.ContainerIndex {
 			t.Matched[i].MergeTargets(newContainer.Targets)
 			return
 		}
 	}
+	// New container in this task
 	t.Matched = append(t.Matched, newContainer)
 }