Skip to content

Commit dcc096a

Browse files
jorbaumjorbaum
committed
Introduce logging to binding config
1 parent 4fb3117 commit dcc096a

4 files changed

Lines changed: 69 additions & 26 deletions

File tree

‎src/cmd/syslog-agent/app/syslog_agent.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -113,13 +113,13 @@ func NewSyslogAgent(
113113
cfg.WarnOnInvalidDrains,
114114
l,
115115
)
116-
cupsFetcher = bindings.NewDrainParamParser(cupsFetcher, cfg.DefaultDrainMetadata)
116+
cupsFetcher = bindings.NewDrainParamParser(cupsFetcher, cfg.DefaultDrainMetadata, l)
117117
}
118118

119119
aggregateFetcher := bindings.NewAggregateDrainFetcher(cfg.AggregateDrainURLs, cacheClient)
120120
bindingManager := binding.NewManager(
121121
cupsFetcher,
122-
bindings.NewDrainParamParser(aggregateFetcher, cfg.DefaultDrainMetadata),
122+
bindings.NewDrainParamParser(aggregateFetcher, cfg.DefaultDrainMetadata, l),
123123
connector,
124124
m,
125125
cfg.Cache.PollingInterval,

‎src/pkg/egress/syslog/filtering_drain_writer.go‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,10 @@ const (
1919
LOGS_AND_METRICS
2020
)
2121

22+
// LogType defines the log types used within Cloud Foundry
23+
// Their order in the code is as documented in https://docs.cloudfoundry.org/devguide/deploy-apps/streaming-logs.html#format
2224
type LogType int
2325

24-
// Ordered as in https://docs.cloudfoundry.org/devguide/deploy-apps/streaming-logs.html#format
2526
const (
2627
API LogType = iota
2728
STG
@@ -32,6 +33,7 @@ const (
3233
CELL
3334
)
3435

36+
// logTypePrefixes maps string prefixes to LogType values for efficient lookup
3537
var logTypePrefixes = map[string]LogType{
3638
"API": API,
3739
"STG": STG,
@@ -42,7 +44,7 @@ var logTypePrefixes = map[string]LogType{
4244
"CELL": CELL,
4345
}
4446

45-
// A set of LogTypes for efficient membership checking
47+
// LogTypeSet is a set of LogTypes for efficient membership checking
4648
type LogTypeSet map[LogType]struct{}
4749

4850
type FilteringDrainWriter struct {
@@ -80,7 +82,8 @@ func (w *FilteringDrainWriter) Write(env *loggregator_v2.Envelope) error {
8082
// Default to sending logs if no source_type tag is present
8183
value, ok := env.GetTags()["source_type"]
8284
if !ok {
83-
// TODO Log
85+
// TODO add unit test for case where source_type tag is missing
86+
// source_type tag is missing, default to sending logs
8487
value = ""
8588
}
8689
if sendsLogs(w.binding.DrainData, w.binding.LogFilter, value) {
@@ -96,6 +99,7 @@ func (w *FilteringDrainWriter) Write(env *loggregator_v2.Envelope) error {
9699
return nil
97100
}
98101

102+
// shouldIncludeLog determines if a log with the given sourceTypeTag should be forwarded
99103
func shouldIncludeLog(logFilter *LogTypeSet, sourceTypeTag string) bool {
100104
// Empty filter or missing source type means no filtering
101105
if logFilter == nil || sourceTypeTag == "" {
@@ -113,7 +117,7 @@ func shouldIncludeLog(logFilter *LogTypeSet, sourceTypeTag string) bool {
113117
logType, known := logTypePrefixes[prefix]
114118
if !known {
115119
// Unknown log type, default to not filtering
116-
// TODO log
120+
// TODO unit test
117121
return true
118122
}
119123

‎src/pkg/ingress/bindings/binding_config.go‎

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package bindings
22

33
import (
44
"errors"
5+
"log"
56
"net/url"
67
"strings"
78

@@ -12,12 +13,14 @@ import (
1213
type DrainParamParser struct {
1314
fetcher binding.Fetcher
1415
defaultDrainMetadata bool
16+
log *log.Logger
1517
}
1618

17-
func NewDrainParamParser(f binding.Fetcher, defaultDrainMetadata bool) *DrainParamParser {
19+
func NewDrainParamParser(f binding.Fetcher, defaultDrainMetadata bool, l *log.Logger) *DrainParamParser {
1820
return &DrainParamParser{
1921
fetcher: f,
2022
defaultDrainMetadata: defaultDrainMetadata,
23+
log: l,
2124
}
2225
}
2326

@@ -37,7 +40,7 @@ func (d *DrainParamParser) FetchBindings() ([]syslog.Binding, error) {
3740
b.OmitMetadata = getOmitMetadata(urlParsed, d.defaultDrainMetadata)
3841
b.InternalTls = getInternalTLS(urlParsed)
3942
b.DrainData = getBindingType(urlParsed)
40-
b.LogFilter, err = getLogFilter(urlParsed)
43+
b.LogFilter, err = d.getLogFilter(urlParsed)
4144
if err != nil {
4245
return nil, err
4346
}
@@ -90,8 +93,8 @@ func getBindingType(u *url.URL) syslog.DrainData {
9093
return drainData
9194
}
9295

93-
// Parse HTML query parameter into a Set of LogTypes
94-
func NewLogTypeSet(logTypeList string, isExclude bool) *syslog.LogTypeSet {
96+
// NewLogTypeSet parses an HTML query parameter into a Set of LogTypes
97+
func (d *DrainParamParser) NewLogTypeSet(logTypeList string, isExclude bool) *syslog.LogTypeSet {
9598
if logTypeList == "" {
9699
set := make(syslog.LogTypeSet)
97100
return &set
@@ -119,7 +122,9 @@ func NewLogTypeSet(logTypeList string, isExclude bool) *syslog.LogTypeSet {
119122
case "cell":
120123
t = syslog.CELL
121124
default:
122-
// TODO Log unknown log type
125+
// Unknown log type, skip it
126+
// TODO add unit test for unknown log type
127+
d.log.Printf("Unknown log type '%s' in log type filter, ignoring", logType)
123128
continue
124129
}
125130
set[t] = struct{}{}
@@ -142,18 +147,18 @@ func NewLogTypeSet(logTypeList string, isExclude bool) *syslog.LogTypeSet {
142147
return &set
143148
}
144149

145-
func getLogFilter(u *url.URL) (*syslog.LogTypeSet, error) {
150+
func (d *DrainParamParser) getLogFilter(u *url.URL) (*syslog.LogTypeSet, error) {
146151
includeLogTypes := u.Query().Get("include-log-types")
147152
excludeLogTypes := u.Query().Get("exclude-log-types")
148153

149154
if excludeLogTypes != "" && includeLogTypes != "" {
150155
return nil, errors.New("include-log-types and exclude-log-types can not be used at the same time")
151156
} else if excludeLogTypes != "" {
152-
return NewLogTypeSet(excludeLogTypes, true), nil
157+
return d.NewLogTypeSet(excludeLogTypes, true), nil
153158
} else if includeLogTypes != "" {
154-
return NewLogTypeSet(includeLogTypes, false), nil
159+
return d.NewLogTypeSet(includeLogTypes, false), nil
155160
}
156-
return NewLogTypeSet("", false), nil
161+
return d.NewLogTypeSet("", false), nil
157162
}
158163

159164
func getRemoveMetadataQuery(u *url.URL) string {

‎src/pkg/ingress/bindings/binding_config_test.go‎

Lines changed: 45 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@ package bindings_test
22

33
import (
44
"errors"
5+
"log"
6+
"strings"
57

68
"code.cloudfoundry.org/loggregator-agent-release/src/pkg/egress/syslog"
79
"code.cloudfoundry.org/loggregator-agent-release/src/pkg/ingress/bindings"
@@ -10,12 +12,15 @@ import (
1012
)
1113

1214
var _ = Describe("Drain Param Config", func() {
15+
var (
16+
logger = log.New(GinkgoWriter, "", 0)
17+
)
1318
It("sets OmitMetadata to false if the drain doesn't contain 'disable-metadata=true'", func() {
1419
bs := []syslog.Binding{
1520
{Drain: syslog.Drain{Url: "https://test.org/drain"}},
1621
}
1722
f := newStubFetcher(bs, nil)
18-
wf := bindings.NewDrainParamParser(f, true)
23+
wf := bindings.NewDrainParamParser(f, true, logger)
1924

2025
configedBindings, _ := wf.FetchBindings()
2126
Expect(configedBindings[0].OmitMetadata).To(BeFalse())
@@ -27,7 +32,7 @@ var _ = Describe("Drain Param Config", func() {
2732
{Drain: syslog.Drain{Url: "https://test.org/drain?omit-metadata=true"}},
2833
}
2934
f := newStubFetcher(bs, nil)
30-
wf := bindings.NewDrainParamParser(f, true)
35+
wf := bindings.NewDrainParamParser(f, true, logger)
3136

3237
configedBindings, _ := wf.FetchBindings()
3338
Expect(configedBindings[0].OmitMetadata).To(BeTrue())
@@ -39,7 +44,7 @@ var _ = Describe("Drain Param Config", func() {
3944
{Drain: syslog.Drain{Url: "https://test.org/drain"}},
4045
}
4146
f := newStubFetcher(bs, nil)
42-
wf := bindings.NewDrainParamParser(f, false)
47+
wf := bindings.NewDrainParamParser(f, false, logger)
4348

4449
configedBindings, _ := wf.FetchBindings()
4550
Expect(configedBindings[0].OmitMetadata).To(BeTrue())
@@ -51,7 +56,7 @@ var _ = Describe("Drain Param Config", func() {
5156
{Drain: syslog.Drain{Url: "https://test.org/drain?omit-metadata=false"}},
5257
}
5358
f := newStubFetcher(bs, nil)
54-
wf := bindings.NewDrainParamParser(f, false)
59+
wf := bindings.NewDrainParamParser(f, false, logger)
5560

5661
configedBindings, _ := wf.FetchBindings()
5762
Expect(configedBindings[0].OmitMetadata).To(BeFalse())
@@ -63,7 +68,7 @@ var _ = Describe("Drain Param Config", func() {
6368
{Drain: syslog.Drain{Url: "https://test.org/drain?ssl-strict-internal=true"}},
6469
}
6570
f := newStubFetcher(bs, nil)
66-
wf := bindings.NewDrainParamParser(f, true)
71+
wf := bindings.NewDrainParamParser(f, true, logger)
6772

6873
configedBindings, _ := wf.FetchBindings()
6974
Expect(configedBindings[0].InternalTls).To(BeTrue())
@@ -78,7 +83,7 @@ var _ = Describe("Drain Param Config", func() {
7883
{Drain: syslog.Drain{Url: "https://test.org/drain?drain-data=all"}},
7984
}
8085
f := newStubFetcher(bs, nil)
81-
wf := bindings.NewDrainParamParser(f, true)
86+
wf := bindings.NewDrainParamParser(f, true, logger)
8287

8388
configedBindings, _ := wf.FetchBindings()
8489
Expect(configedBindings[0].DrainData).To(Equal(syslog.LOGS))
@@ -97,7 +102,7 @@ var _ = Describe("Drain Param Config", func() {
97102
{Drain: syslog.Drain{Url: "https://test.org/drain?exclude-log-types=rtr"}},
98103
}
99104
f := newStubFetcher(bs, nil)
100-
wf := bindings.NewDrainParamParser(f, true)
105+
wf := bindings.NewDrainParamParser(f, true, logger)
101106

102107
configedBindings, _ := wf.FetchBindings()
103108
Expect(configedBindings[0].LogFilter).To(Equal(NewLogTypeSet())) // Empty map defaults to all types
@@ -112,13 +117,42 @@ var _ = Describe("Drain Param Config", func() {
112117
{Drain: syslog.Drain{Url: "https://test.org/drain?include-log-types=app&exclude-log-types=rtr"}},
113118
}
114119
f := newStubFetcher(bs, nil)
115-
wf := bindings.NewDrainParamParser(f, true)
120+
wf := bindings.NewDrainParamParser(f, true, logger)
116121

117122
configedBindings, err := wf.FetchBindings()
118123
Expect(err).To(HaveOccurred())
119124
Expect(configedBindings).To(HaveLen(0))
120125
})
121126

127+
It("logs a warning when an unknown log type is provided", func() {
128+
var logOutput strings.Builder
129+
testLogger := log.New(&logOutput, "", log.LstdFlags)
130+
parser := bindings.NewDrainParamParser(newStubFetcher(nil, nil), false, testLogger)
131+
132+
result := parser.NewLogTypeSet("app,unknown,rtr", false)
133+
134+
// Should only contain APP and RTR, not the unknown type
135+
Expect(result).To(Equal(NewLogTypeSet(syslog.APP, syslog.RTR)))
136+
137+
// Should have logged a warning
138+
Expect(logOutput.String()).To(ContainSubstring("ignoring"))
139+
})
140+
141+
It("handles unknown log types in exclude mode", func() {
142+
var logOutput strings.Builder
143+
testLogger := log.New(&logOutput, "", log.LstdFlags)
144+
parser := bindings.NewDrainParamParser(newStubFetcher(nil, nil), false, testLogger)
145+
146+
result := parser.NewLogTypeSet("rtr,unknown", true)
147+
148+
// Should exclude only RTR (unknown type is ignored)
149+
expectedSet := NewLogTypeSet(syslog.API, syslog.STG, syslog.LGR, syslog.APP, syslog.SSH, syslog.CELL)
150+
Expect(result).To(Equal(expectedSet))
151+
152+
// Should have logged a warning
153+
Expect(logOutput.String()).To(ContainSubstring("ignoring"))
154+
})
155+
122156
It("sets drain data for old parameter appropriately'", func() {
123157
bs := []syslog.Binding{
124158
{Drain: syslog.Drain{Url: "https://test.org/drain?drain-type=metrics"}},
@@ -128,7 +162,7 @@ var _ = Describe("Drain Param Config", func() {
128162
{Drain: syslog.Drain{Url: "https://test.org/drain?include-metrics-deprecated=true"}},
129163
}
130164
f := newStubFetcher(bs, nil)
131-
wf := bindings.NewDrainParamParser(f, true)
165+
wf := bindings.NewDrainParamParser(f, true, logger)
132166

133167
configedBindings, _ := wf.FetchBindings()
134168
Expect(configedBindings[0].DrainData).To(Equal(syslog.METRICS))
@@ -145,7 +179,7 @@ var _ = Describe("Drain Param Config", func() {
145179
{Drain: syslog.Drain{Url: "https://test.org/drain?omit-metadata=true"}},
146180
}
147181
f := newStubFetcher(bs, nil)
148-
wf := bindings.NewDrainParamParser(f, true)
182+
wf := bindings.NewDrainParamParser(f, true, logger)
149183

150184
configedBindings, err := wf.FetchBindings()
151185
Expect(err).ToNot(HaveOccurred())
@@ -156,7 +190,7 @@ var _ = Describe("Drain Param Config", func() {
156190

157191
It("Returns a error when fetching fails", func() {
158192
f := newStubFetcher(nil, errors.New("Ahhh an error"))
159-
wf := bindings.NewDrainParamParser(f, true)
193+
wf := bindings.NewDrainParamParser(f, true, logger)
160194

161195
_, err := wf.FetchBindings()
162196
Expect(err).To(MatchError("Ahhh an error"))

0 commit comments

Comments
 (0)