From 60735486a29d44773baf5182d2ce9f63d4071ed5 Mon Sep 17 00:00:00 2001 From: "o.omahony" Date: Mon, 21 Nov 2022 16:39:17 +0000 Subject: [PATCH 1/5] see the test --- src/plugins/advanced_metrics.go | 6 +- .../core/metrics/sources/nginx_access_log.go | 57 +++++++++++++++---- .../monitoring/processor/nap.go | 6 +- .../agent/v2/src/plugins/advanced_metrics.go | 6 +- .../agent/v2/src/plugins/nap_monitoring.go | 2 +- 5 files changed, 61 insertions(+), 16 deletions(-) diff --git a/src/plugins/advanced_metrics.go b/src/plugins/advanced_metrics.go index 9839b4ea1..0fd61d5d7 100644 --- a/src/plugins/advanced_metrics.go +++ b/src/plugins/advanced_metrics.go @@ -226,7 +226,11 @@ func (m *AdvancedMetrics) run() { return } now := types.TimestampNow() - m.pipeline.Process(core.NewMessage(core.CommMetrics, []core.Payload{toMetricReport(mr, now, commonDimensions)})) + report := toMetricReport(mr, now, commonDimensions) + log.Infof("%v", report) + if len(report.Data) != 0 { + m.pipeline.Process(core.NewMessage(core.CommMetrics, []core.Payload{report})) + } case <-m.pipeline.Context().Done(): return } diff --git a/test/performance/vendor/github.com/nginx/agent/v2/src/core/metrics/sources/nginx_access_log.go b/test/performance/vendor/github.com/nginx/agent/v2/src/core/metrics/sources/nginx_access_log.go index f18dc9fb7..69d0dcf43 100644 --- a/test/performance/vendor/github.com/nginx/agent/v2/src/core/metrics/sources/nginx_access_log.go +++ b/test/performance/vendor/github.com/nginx/agent/v2/src/core/metrics/sources/nginx_access_log.go @@ -18,6 +18,10 @@ import ( "github.com/nginx/agent/v2/src/core/metrics/sources/tailer" ) +const ( + spaceDelim = " " +) + // This metrics source is used to tail the NGINX access logs to retrieve http metrics. type NginxAccessLog struct { @@ -207,26 +211,30 @@ func (c *NginxAccessLog) logStats(ctx context.Context, logFile, logFormat string } if access.Request != "" { - splitRequest := strings.Split(access.Request, " ") - n := fmt.Sprintf("method.%s", strings.ToLower(splitRequest[0])) + method, _, protocol := getParsedRequest(access.Request) + n := fmt.Sprintf("method.%s", strings.ToLower(method)) if isOtherMethod(n) { n = "method.others" } counters[n] = counters[n] + 1 if access.ServerProtocol == "" { - httpProtocolVersion := strings.Split(splitRequest[2], "/")[1] - httpProtocolVersion = strings.ReplaceAll(httpProtocolVersion, ".", "_") - n = fmt.Sprintf("v%s", httpProtocolVersion) - counters[n] = counters[n] + 1 + if strings.Count(protocol, "/") == 1 { + httpProtocolVersion := strings.Split(protocol, "/")[1] + httpProtocolVersion = strings.ReplaceAll(httpProtocolVersion, ".", "_") + n = fmt.Sprintf("v%s", httpProtocolVersion) + counters[n] = counters[n] + 1 + } } } if access.ServerProtocol != "" { - httpProtocolVersion := strings.Split(access.ServerProtocol, "/")[1] - httpProtocolVersion = strings.ReplaceAll(httpProtocolVersion, ".", "_") - n := fmt.Sprintf("v%s", httpProtocolVersion) - counters[n] = counters[n] + 1 + if strings.Count(access.ServerProtocol, "/") == 1 { + httpProtocolVersion := strings.Split(access.ServerProtocol, "/")[1] + httpProtocolVersion = strings.ReplaceAll(httpProtocolVersion, ".", "_") + n := fmt.Sprintf("v%s", httpProtocolVersion) + counters[n] = counters[n] + 1 + } } // don't need the http status for NGINX Plus @@ -294,6 +302,35 @@ func (c *NginxAccessLog) logStats(ctx context.Context, logFile, logFormat string } } +func getParsedRequest(request string) (method string, uri string, protocol string) { + if len(request) == 0 { + return + } + + startURIIdx := strings.Index(request, spaceDelim) + if startURIIdx == -1 { + return + } + + endURIIdx := strings.LastIndex(request, spaceDelim) + // Ideally, endURIIdx should never be -1 here, as startURIIdx should have handled it already + if endURIIdx == -1 { + return + } + + // For Example: GET /user/register?ahrefp' or ' HTTP/1.1 + + // method -> GET + method = request[:startURIIdx] + + // uri -> /user/register?ahrefp' or ' + uri = request[startURIIdx+1 : endURIIdx] + + // protocol -> HTTP/1.1 + protocol = request[endURIIdx+1:] + return +} + func getRequestLengthMetricValue(requestLengths []float64) float64 { value := 0.0 diff --git a/test/performance/vendor/github.com/nginx/agent/v2/src/extensions/nginx-app-protect/monitoring/processor/nap.go b/test/performance/vendor/github.com/nginx/agent/v2/src/extensions/nginx-app-protect/monitoring/processor/nap.go index ea34ce712..9a93efe49 100644 --- a/test/performance/vendor/github.com/nginx/agent/v2/src/extensions/nginx-app-protect/monitoring/processor/nap.go +++ b/test/performance/vendor/github.com/nginx/agent/v2/src/extensions/nginx-app-protect/monitoring/processor/nap.go @@ -81,7 +81,6 @@ var ( subViolations, supportID, threatCampaignNames, - httpURI, violationRating, httpHostname, xForwardedForHeaderVal, @@ -97,6 +96,7 @@ var ( clientApplication, clientApplicationVersion, transportProtocol, + httpURI, } ) @@ -464,7 +464,7 @@ func setValue(napConfig *NAPConfig, key, value string, logger *logrus.Entry) err case sigSetNames: napConfig.SigSetNames = replaceEncodedList(value, listSeperator) case threatCampaignNames: - napConfig.ThreatCampaignNames = value + napConfig.ThreatCampaignNames = replaceEncodedList(value, listSeperator) case violationDetails: napConfig.ViolationDetailsXML = func(data string) *BADMSG { var xmlData BADMSG @@ -513,7 +513,7 @@ func setValue(napConfig *NAPConfig, key, value string, logger *logrus.Entry) err case sigCVEs: napConfig.SignatureCVEs = replaceEncodedList(value, listSeperator) case subViolations: - napConfig.SubViolations = value + napConfig.SubViolations = replaceEncodedList(value, listSeperator) case supportID: napConfig.SupportID = value case violations: diff --git a/test/performance/vendor/github.com/nginx/agent/v2/src/plugins/advanced_metrics.go b/test/performance/vendor/github.com/nginx/agent/v2/src/plugins/advanced_metrics.go index 9839b4ea1..0fd61d5d7 100644 --- a/test/performance/vendor/github.com/nginx/agent/v2/src/plugins/advanced_metrics.go +++ b/test/performance/vendor/github.com/nginx/agent/v2/src/plugins/advanced_metrics.go @@ -226,7 +226,11 @@ func (m *AdvancedMetrics) run() { return } now := types.TimestampNow() - m.pipeline.Process(core.NewMessage(core.CommMetrics, []core.Payload{toMetricReport(mr, now, commonDimensions)})) + report := toMetricReport(mr, now, commonDimensions) + log.Infof("%v", report) + if len(report.Data) != 0 { + m.pipeline.Process(core.NewMessage(core.CommMetrics, []core.Payload{report})) + } case <-m.pipeline.Context().Done(): return } diff --git a/test/performance/vendor/github.com/nginx/agent/v2/src/plugins/nap_monitoring.go b/test/performance/vendor/github.com/nginx/agent/v2/src/plugins/nap_monitoring.go index 832b88562..e10d28b1b 100644 --- a/test/performance/vendor/github.com/nginx/agent/v2/src/plugins/nap_monitoring.go +++ b/test/performance/vendor/github.com/nginx/agent/v2/src/plugins/nap_monitoring.go @@ -108,7 +108,7 @@ func (n *NAPMonitoring) run() { } case <-riTicker.C: if len(report.Events) > 0 { - log.Infof("reached a report interval of %vs, sending %d Security Violation Events as a report", n.reportInterval.Seconds(), n.reportCount) + log.Infof("reached a report interval of %vs, sending %d Security Violation Events as a report", n.reportInterval.Seconds(), len(report.Events)) n.send(report) } case <-n.ctx.Done(): From 91f8e256ccfec7c7b1f35d7364162db224dd3df3 Mon Sep 17 00:00:00 2001 From: "o.omahony" Date: Mon, 21 Nov 2022 16:43:26 +0000 Subject: [PATCH 2/5] trace logging --- test/component/advanced_metrics_component_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/component/advanced_metrics_component_test.go b/test/component/advanced_metrics_component_test.go index b3a28a9a5..cd8452d32 100644 --- a/test/component/advanced_metrics_component_test.go +++ b/test/component/advanced_metrics_component_test.go @@ -53,7 +53,7 @@ var ( ) func init() { - logrus.SetLevel(logrus.DebugLevel) + logrus.SetLevel(logrus.TraceLevel) } // TestAdvancedMetricsSimple this is simple test which presents basic logic of advanced_metrics module From 855450f4f194a3999c803d437cb45612f4f68c5c Mon Sep 17 00:00:00 2001 From: "o.omahony" Date: Mon, 21 Nov 2022 18:04:30 +0000 Subject: [PATCH 3/5] added empty metrics report --- .../advanced-metrics/pkg/publisher/publisher_test.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/extensions/advanced-metrics/pkg/publisher/publisher_test.go b/src/extensions/advanced-metrics/pkg/publisher/publisher_test.go index fd6b4f04b..6e6126f19 100644 --- a/src/extensions/advanced-metrics/pkg/publisher/publisher_test.go +++ b/src/extensions/advanced-metrics/pkg/publisher/publisher_test.go @@ -20,6 +20,16 @@ func TestPublisher(t *testing.T) { expectedMetrics []*MetricSet dimensionsLookups map[int]map[int]string }{ + { + name: "no metric with no dimension", + schema: schema.NewSchema([]*schema.Field{ + schema.NewDimensionField("dim1", 0, schema.WithKeyBitSize(8)), + schema.NewMetricField("metric1"), + }...), + samples: map[string]*sample.Sample{}, + dimensionsLookups: map[int]map[int]string{}, + expectedMetrics: []*MetricSet{}, + }, { name: "single metric with single dimension", schema: schema.NewSchema([]*schema.Field{ From 9da93f7aac65dc24a7a5a06b0af9c66716747175 Mon Sep 17 00:00:00 2001 From: "o.omahony" Date: Mon, 21 Nov 2022 18:05:34 +0000 Subject: [PATCH 4/5] tidied test --- .../advanced-metrics/pkg/publisher/publisher_test.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/extensions/advanced-metrics/pkg/publisher/publisher_test.go b/src/extensions/advanced-metrics/pkg/publisher/publisher_test.go index 6e6126f19..186432328 100644 --- a/src/extensions/advanced-metrics/pkg/publisher/publisher_test.go +++ b/src/extensions/advanced-metrics/pkg/publisher/publisher_test.go @@ -22,10 +22,7 @@ func TestPublisher(t *testing.T) { }{ { name: "no metric with no dimension", - schema: schema.NewSchema([]*schema.Field{ - schema.NewDimensionField("dim1", 0, schema.WithKeyBitSize(8)), - schema.NewMetricField("metric1"), - }...), + schema: schema.NewSchema([]*schema.Field{}...), samples: map[string]*sample.Sample{}, dimensionsLookups: map[int]map[int]string{}, expectedMetrics: []*MetricSet{}, From 9b2cdeb473323096a0ebfa6555335c23b9c3b458 Mon Sep 17 00:00:00 2001 From: "o.omahony" Date: Wed, 30 Nov 2022 12:18:05 +0000 Subject: [PATCH 5/5] removed debug --- sdk/proto/events/event.pb.go | 1 + src/plugins/advanced_metrics.go | 1 - .../tonistiigi/fsutil/.golangci.yml | 20 +++++++++++++++++++ .../agent/sdk/v2/proto/events/event.pb.go | 1 + .../agent/v2/src/plugins/advanced_metrics.go | 1 - .../agent/sdk/v2/proto/events/event.pb.go | 1 + 6 files changed, 23 insertions(+), 2 deletions(-) create mode 100644 test/integration/vendor/github.com/tonistiigi/fsutil/.golangci.yml diff --git a/sdk/proto/events/event.pb.go b/sdk/proto/events/event.pb.go index 0d8ac2fec..903408281 100644 --- a/sdk/proto/events/event.pb.go +++ b/sdk/proto/events/event.pb.go @@ -123,6 +123,7 @@ func (m *Metadata) GetCategory() string { type Event struct { Metadata *Metadata `protobuf:"bytes,1,opt,name=Metadata,proto3" json:"metadata"` // Types that are valid to be assigned to Data: + // // *Event_ActivityEvent // *Event_SecurityViolationEvent Data isEvent_Data `protobuf_oneof:"data"` diff --git a/src/plugins/advanced_metrics.go b/src/plugins/advanced_metrics.go index 0fd61d5d7..3c3c3ac3a 100644 --- a/src/plugins/advanced_metrics.go +++ b/src/plugins/advanced_metrics.go @@ -227,7 +227,6 @@ func (m *AdvancedMetrics) run() { } now := types.TimestampNow() report := toMetricReport(mr, now, commonDimensions) - log.Infof("%v", report) if len(report.Data) != 0 { m.pipeline.Process(core.NewMessage(core.CommMetrics, []core.Payload{report})) } diff --git a/test/integration/vendor/github.com/tonistiigi/fsutil/.golangci.yml b/test/integration/vendor/github.com/tonistiigi/fsutil/.golangci.yml new file mode 100644 index 000000000..788dfd53d --- /dev/null +++ b/test/integration/vendor/github.com/tonistiigi/fsutil/.golangci.yml @@ -0,0 +1,20 @@ +run: + timeout: 10m + skip-files: + - ".*\\.pb\\.go$" + +linters: + enable: + - gofmt + - govet + - deadcode + - goimports + - ineffassign + - misspell + - unused + - varcheck + - golint + - staticcheck + - typecheck + - structcheck + disable-all: true diff --git a/test/performance/vendor/github.com/nginx/agent/sdk/v2/proto/events/event.pb.go b/test/performance/vendor/github.com/nginx/agent/sdk/v2/proto/events/event.pb.go index 0d8ac2fec..903408281 100644 --- a/test/performance/vendor/github.com/nginx/agent/sdk/v2/proto/events/event.pb.go +++ b/test/performance/vendor/github.com/nginx/agent/sdk/v2/proto/events/event.pb.go @@ -123,6 +123,7 @@ func (m *Metadata) GetCategory() string { type Event struct { Metadata *Metadata `protobuf:"bytes,1,opt,name=Metadata,proto3" json:"metadata"` // Types that are valid to be assigned to Data: + // // *Event_ActivityEvent // *Event_SecurityViolationEvent Data isEvent_Data `protobuf_oneof:"data"` diff --git a/test/performance/vendor/github.com/nginx/agent/v2/src/plugins/advanced_metrics.go b/test/performance/vendor/github.com/nginx/agent/v2/src/plugins/advanced_metrics.go index 0fd61d5d7..3c3c3ac3a 100644 --- a/test/performance/vendor/github.com/nginx/agent/v2/src/plugins/advanced_metrics.go +++ b/test/performance/vendor/github.com/nginx/agent/v2/src/plugins/advanced_metrics.go @@ -227,7 +227,6 @@ func (m *AdvancedMetrics) run() { } now := types.TimestampNow() report := toMetricReport(mr, now, commonDimensions) - log.Infof("%v", report) if len(report.Data) != 0 { m.pipeline.Process(core.NewMessage(core.CommMetrics, []core.Payload{report})) } diff --git a/vendor/github.com/nginx/agent/sdk/v2/proto/events/event.pb.go b/vendor/github.com/nginx/agent/sdk/v2/proto/events/event.pb.go index 0d8ac2fec..903408281 100644 --- a/vendor/github.com/nginx/agent/sdk/v2/proto/events/event.pb.go +++ b/vendor/github.com/nginx/agent/sdk/v2/proto/events/event.pb.go @@ -123,6 +123,7 @@ func (m *Metadata) GetCategory() string { type Event struct { Metadata *Metadata `protobuf:"bytes,1,opt,name=Metadata,proto3" json:"metadata"` // Types that are valid to be assigned to Data: + // // *Event_ActivityEvent // *Event_SecurityViolationEvent Data isEvent_Data `protobuf_oneof:"data"`