Skip to content

Commit

Permalink
Change rule expression to match endpoint types for receiver creator (#…
Browse files Browse the repository at this point in the history
…2661)

Change the rule expression in receiver creator for matching endpoints types port, hostport and pod from type.port, type.hostport and type.pod respectively, to type == "port", type == "hostport" and type == "pod".

Testing: Fixed unit tests failing due to change

Documentation: Updated documentation to reflect change.
  • Loading branch information
bjsignalfx authored Mar 23, 2021
1 parent fef5054 commit bf92d2f
Show file tree
Hide file tree
Showing 10 changed files with 47 additions and 54 deletions.
17 changes: 2 additions & 15 deletions extension/observer/endpoints.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,13 +38,6 @@ const (
)

var (
// EndpointTypes is a map of all known endpoint types.
EndpointTypes = map[EndpointType]bool{
PortType: true,
PodType: true,
HostPortType: true,
}

_ EndpointDetails = (*Pod)(nil)
_ EndpointDetails = (*Port)(nil)
_ EndpointDetails = (*HostPort)(nil)
Expand All @@ -71,17 +64,11 @@ func (e *Endpoint) Env() (EndpointEnv, error) {
if e.Details == nil {
return nil, errors.New("endpoint is missing details")
}

env := e.Details.Env()
env["endpoint"] = e.Target
env["type"] = string(e.Details.Type())

// Populate type field for evaluating rules with `type.port && ...`.
// Use string instead of EndpointType for rule evaluation.
types := map[string]bool{}
for endpointType := range EndpointTypes {
types[string(endpointType)] = false
}
types[string(e.Details.Type())] = true
env["type"] = types
return env, nil
}

Expand Down
18 changes: 3 additions & 15 deletions extension/observer/endpoints_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,7 @@ func TestEndpointEnv(t *testing.T) {
},
},
want: EndpointEnv{
"type": map[string]bool{
"pod": true,
"hostport": false,
"port": false,
},
"type": "pod",
"endpoint": "192.68.73.2",
"name": "pod_name",
"labels": map[string]string{
Expand Down Expand Up @@ -85,11 +81,7 @@ func TestEndpointEnv(t *testing.T) {
},
},
want: EndpointEnv{
"type": map[string]bool{
"pod": false,
"hostport": false,
"port": true,
},
"type": "port",
"endpoint": "192.68.73.2",
"name": "port_name",
"port": uint16(2379),
Expand Down Expand Up @@ -122,11 +114,7 @@ func TestEndpointEnv(t *testing.T) {
},
},
want: EndpointEnv{
"type": map[string]bool{
"hostport": true,
"pod": false,
"port": false,
},
"type": "hostport",
"endpoint": "127.0.0.1",
"process_name": "process_name",
"command": "./cmd --config config.yaml",
Expand Down
2 changes: 1 addition & 1 deletion extension/observer/hostobserver/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ Endpoint variables exposed by this observer are as follows.

| Variable | Description |
|-----------|--------------------------------------------------------------------------------------------|
| type.port | `true` |
| type | `"port"` |
| name | name of the process associated to the port |
| port | port number |
| command | full command used to invoke this process, including the executable itself at the beginning |
Expand Down
22 changes: 11 additions & 11 deletions receiver/receivercreator/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -62,39 +62,39 @@ This setting controls what resource attributes are set on metrics emitted from t

Note that the backticks below are not typos--they indicate the value is set dynamically.

`type.pod`
`type == "pod"`

| Resource Attribute | Default |
|--------------------|---------------|
| k8s.pod.name | \`name\` |
| k8s.pod.uid | \`uid\` |
| k8s.namespace.name | \`namespace\` |

`type.port`
`type == "port"`

| Resource Attribute | Default |
|--------------------|-------------------|
| k8s.pod.name | \`pod.name\` |
| k8s.pod.uid | \`pod.uid\` |
| k8s.namespace.name | \`pod.namespace\` |

`type.hostport`
`type == "hostport"`

None

See `redis/2` in [examples](#examples).

## Rule Expressions

Each rule must start with `type.(pod|port|hostport) &&` such that the rule matches
Each rule must start with `type == ("pod"|"port"|"hostport") &&` such that the rule matches
only one endpoint type. Depending on the type of endpoint the rule is
targeting it will have different variables available.

### Pod

| Variable | Description |
|-------------|-----------------------------------|
| type.pod | `true` |
| type | `"pod"` |
| name | name of the pod |
| namespace | namespace of the pod |
| uid | unique id of the pod |
Expand All @@ -105,7 +105,7 @@ targeting it will have different variables available.

| Variable | Description |
|-----------------|-----------------------------------------|
| type.port | `true` |
| type | `"port"` |
| name | container port name |
| port | port number |
| protocol | The transport protocol ("TCP" or "UDP") |
Expand All @@ -119,7 +119,7 @@ targeting it will have different variables available.

| Variable | Description |
|---------------|--------------------------------------------------|
| type.hostport | `true` |
| type | `"hostport"` |
| process_name | Name of the process |
| command | Command line with the used to invoke the process |
| is_ipv6 | true if endpoint is IPv6, otherwise false |
Expand All @@ -141,14 +141,14 @@ receivers:
receivers:
prometheus_simple:
# Configure prometheus scraping if standard prometheus annotations are set on the pod.
rule: type.pod && annotations["prometheus.io/scrape"] == "true"
rule: type == "pod" && annotations["prometheus.io/scrape"] == "true"
config:
metrics_path: '`"prometheus.io/path" in annotations ? annotations["prometheus.io/path"] : "/metrics"`'
endpoint: '`endpoint`:`"prometheus.io/port" in annotations ? annotations["prometheus.io/port"] : 9090`'

redis/1:
# If this rule matches an instance of this receiver will be started.
rule: type.port && port == 6379
rule: type == "port" && port == 6379
config:
# Static receiver-specific config.
password: secret
Expand All @@ -157,7 +157,7 @@ receivers:

redis/2:
# Set a resource attribute based on endpoint value.
rule: type.port && port == 6379
rule: type == "port" && port == 6379
resource_attributes:
# Dynamic value.
app: `pod.labels["app"]`
Expand All @@ -169,7 +169,7 @@ receivers:
receivers:
redis/on_host:
# If this rule matches an instance of this receiver will be started.
rule: type.port && port == 6379 && is_ipv6 == true
rule: type == "port" && port == 6379 && is_ipv6 == true
config:
service_name: redis_on_host

Expand Down
6 changes: 4 additions & 2 deletions receiver/receivercreator/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,9 +84,11 @@ func TestLoadConfig(t *testing.T) {
r1 := cfg.Receivers["receiver_creator/1"].(*Config)

assert.NotNil(t, r1)
assert.Len(t, r1.receiverTemplates, 1)
assert.Len(t, r1.receiverTemplates, 2)
assert.Contains(t, r1.receiverTemplates, "examplereceiver/1")
assert.Equal(t, `type == "port"`, r1.receiverTemplates["examplereceiver/1"].Rule)
assert.Contains(t, r1.receiverTemplates, "nop/1")
assert.Equal(t, `type.port`, r1.receiverTemplates["nop/1"].Rule)
assert.Equal(t, `type == "port"`, r1.receiverTemplates["nop/1"].Rule)
assert.Equal(t, userConfigMap{
endpointConfigKey: "localhost:12345",
}, r1.receiverTemplates["nop/1"].config)
Expand Down
11 changes: 11 additions & 0 deletions receiver/receivercreator/fixtures_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,17 @@ var portEndpoint = observer.Endpoint{
},
}

var hostportEndpoint = observer.Endpoint{
ID: "port-1",
Target: "localhost:1234",
Details: &observer.HostPort{
ProcessName: "splunk",
Command: "./splunk",
Port: 1234,
Transport: observer.ProtocolTCP,
},
}

var unsupportedEndpoint = observer.Endpoint{
ID: "endpoint-1",
Target: "localhost:1234",
Expand Down
8 changes: 4 additions & 4 deletions receiver/receivercreator/observerhandler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ func TestOnAdd(t *testing.T) {
rcvrCfg := receiverConfig{typeStr: configmodels.Type("name"), config: userConfigMap{"foo": "bar"}, fullName: "name/1"}
cfg := createDefaultConfig().(*Config)
cfg.receiverTemplates = map[string]receiverTemplate{
"name/1": {rcvrCfg, "", newRuleOrPanic(`type.port`)},
"name/1": {rcvrCfg, "", newRuleOrPanic(`type == "port"`)},
}
handler := &observerHandler{
config: cfg,
Expand Down Expand Up @@ -104,7 +104,7 @@ func TestOnChange(t *testing.T) {
newRcvr := &nopWithEndpointReceiver{}
cfg := createDefaultConfig().(*Config)
cfg.receiverTemplates = map[string]receiverTemplate{
"name/1": {rcvrCfg, "", newRuleOrPanic(`type.port`)},
"name/1": {rcvrCfg, "", newRuleOrPanic(`type == "port"`)},
}
handler := &observerHandler{
config: cfg,
Expand Down Expand Up @@ -136,8 +136,8 @@ func TestDynamicConfig(t *testing.T) {
cfg.receiverTemplates = map[string]receiverTemplate{
"name/1": {
receiverConfig: receiverConfig{typeStr: configmodels.Type("name"), config: userConfigMap{"endpoint": "`endpoint`:6379"}, fullName: "name/1"},
Rule: "type.pod",
rule: newRuleOrPanic("type.pod"),
Rule: `type == "pod"`,
rule: newRuleOrPanic("type == \"pod\""),
},
}
handler := &observerHandler{
Expand Down
2 changes: 1 addition & 1 deletion receiver/receivercreator/rules.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ type rule struct {
}

// ruleRe is used to verify the rule starts type check.
var ruleRe = regexp.MustCompile(`^type\.(pod|port)`)
var ruleRe = regexp.MustCompile(`^type\s*==\s*("pod"|"port"|"hostport")`)

// newRule creates a new rule instance.
func newRule(ruleStr string) (rule, error) {
Expand Down
11 changes: 7 additions & 4 deletions receiver/receivercreator/rules_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,10 @@ func Test_ruleEval(t *testing.T) {
}{
// Doesn't work yet. See comment in newRule.
// {"unknown variable", args{`type == "port" && unknown_var == 1`, portEndpoint}, false, true},
{"basic port", args{`type.port && name == "http" && pod.labels["app"] == "redis"`, portEndpoint}, true, false},
{"basic pod", args{`type.pod && labels["region"] == "west-1"`, podEndpoint}, true, false},
{"annotations", args{`type.pod && annotations["scrape"] == "true"`, podEndpoint}, true, false},
{"basic port", args{`type == "port" && name == "http" && pod.labels["app"] == "redis"`, portEndpoint}, true, false},
{"basic hostport", args{`type == "hostport" && port == 1234 && process_name == "splunk"`, hostportEndpoint}, true, false},
{"basic pod", args{`type == "pod" && labels["region"] == "west-1"`, podEndpoint}, true, false},
{"annotations", args{`type == "pod" && annotations["scrape"] == "true"`, podEndpoint}, true, false},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
Expand Down Expand Up @@ -81,7 +82,9 @@ func Test_newRule(t *testing.T) {
{"empty rule", args{""}, true},
{"does not start with type", args{"port == 1234"}, true},
{"invalid syntax", args{"port =="}, true},
{"valid", args{`type.port && port_name == "http"`}, false},
{"valid port", args{`type == "port" && port_name == "http"`}, false},
{"valid pod", args{`type=="pod" && port_name == "http"`}, false},
{"valid hostport", args{`type == "hostport" && port_name == "http"`}, false},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
Expand Down
4 changes: 3 additions & 1 deletion receiver/receivercreator/testdata/config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,10 @@ receivers:
receiver_creator/1:
watch_observers: [mock_observer]
receivers:
examplereceiver/1:
rule: type == "port"
nop/1:
rule: type.port
rule: type == "port"
config:
endpoint: localhost:12345

Expand Down

0 comments on commit bf92d2f

Please sign in to comment.