From 3877a66f6a373a3526121a414f5e770e1a4cf1ca Mon Sep 17 00:00:00 2001 From: Aphral Griffin Date: Thu, 17 Apr 2025 16:21:22 +0100 Subject: [PATCH 1/9] enabled disable features --- .../collector/nginxplusreceiver/scraper.go | 36 +++---- internal/collector/otel_collector_plugin.go | 2 + internal/command/command_plugin.go | 10 +- internal/plugin/plugin_manager.go | 15 +++ internal/plugin/plugin_manager_test.go | 100 +++++++++++++++++- .../watcher/instance/nginx_config_parser.go | 13 ++- internal/watcher/watcher_plugin.go | 3 + pkg/config/features.go | 1 - 8 files changed, 151 insertions(+), 29 deletions(-) diff --git a/internal/collector/nginxplusreceiver/scraper.go b/internal/collector/nginxplusreceiver/scraper.go index 7b6cb0ad5f..b35c607398 100644 --- a/internal/collector/nginxplusreceiver/scraper.go +++ b/internal/collector/nginxplusreceiver/scraper.go @@ -34,7 +34,7 @@ const ( peerStateUnhealthy = "unhealthy" ) -type nginxPlusScraper struct { +type NginxPlusScraper struct { previousServerZoneResponses map[string]ResponseStatuses previousLocationZoneResponses map[string]ResponseStatuses plusClient *plusapi.NginxClient @@ -57,7 +57,7 @@ type ResponseStatuses struct { func newNginxPlusScraper( settings receiver.Settings, cfg *Config, -) (*nginxPlusScraper, error) { +) (*NginxPlusScraper, error) { endpoint := strings.TrimPrefix(cfg.APIDetails.URL, "unix:") logger := settings.Logger logger.Info("Creating NGINX Plus scraper") @@ -79,7 +79,7 @@ func newNginxPlusScraper( return nil, err } - return &nginxPlusScraper{ + return &NginxPlusScraper{ plusClient: plusClient, settings: settings, cfg: cfg, @@ -89,7 +89,7 @@ func newNginxPlusScraper( }, nil } -func (nps *nginxPlusScraper) createPreviousLocationZoneResponses(stats *plusapi.Stats) { +func (nps *NginxPlusScraper) createPreviousLocationZoneResponses(stats *plusapi.Stats) { previousLocationZoneResponses := make(map[string]ResponseStatuses) for lzName, lz := range stats.LocationZones { respStatus := ResponseStatuses{ @@ -106,7 +106,7 @@ func (nps *nginxPlusScraper) createPreviousLocationZoneResponses(stats *plusapi. nps.previousLocationZoneResponses = previousLocationZoneResponses } -func (nps *nginxPlusScraper) createPreviousServerZoneResponses(stats *plusapi.Stats) { +func (nps *NginxPlusScraper) createPreviousServerZoneResponses(stats *plusapi.Stats) { previousServerZoneResponses := make(map[string]ResponseStatuses) for szName, sz := range stats.ServerZones { respStatus := ResponseStatuses{ @@ -123,7 +123,7 @@ func (nps *nginxPlusScraper) createPreviousServerZoneResponses(stats *plusapi.St nps.previousServerZoneResponses = previousServerZoneResponses } -func (nps *nginxPlusScraper) scrape(ctx context.Context) (pmetric.Metrics, error) { +func (nps *NginxPlusScraper) scrape(ctx context.Context) (pmetric.Metrics, error) { // nps.init.Do is ran only once, it is only ran the first time scrape is called to set the previous responses // metric value nps.init.Do(func() { @@ -151,7 +151,7 @@ func (nps *nginxPlusScraper) scrape(ctx context.Context) (pmetric.Metrics, error return nps.mb.Emit(metadata.WithResource(nps.rb.Emit())), nil } -func (nps *nginxPlusScraper) recordMetrics(stats *plusapi.Stats) { +func (nps *NginxPlusScraper) recordMetrics(stats *plusapi.Stats) { now := pcommon.NewTimestampFromTime(time.Now()) // NGINX config reloads @@ -193,7 +193,7 @@ func (nps *nginxPlusScraper) recordMetrics(stats *plusapi.Stats) { nps.recordStreamMetrics(stats, now) } -func (nps *nginxPlusScraper) recordStreamMetrics(stats *plusapi.Stats, now pcommon.Timestamp) { +func (nps *NginxPlusScraper) recordStreamMetrics(stats *plusapi.Stats, now pcommon.Timestamp) { for name, streamServerZone := range stats.StreamServerZones { nps.mb.RecordNginxStreamIoDataPoint( now, @@ -437,7 +437,7 @@ func (nps *nginxPlusScraper) recordStreamMetrics(stats *plusapi.Stats, now pcomm } } -func (nps *nginxPlusScraper) recordSSLMetrics(now pcommon.Timestamp, stats *plusapi.Stats) { +func (nps *NginxPlusScraper) recordSSLMetrics(now pcommon.Timestamp, stats *plusapi.Stats) { nps.mb.RecordNginxSslHandshakesDataPoint( now, int64(stats.SSL.HandshakesFailed), @@ -503,7 +503,7 @@ func (nps *nginxPlusScraper) recordSSLMetrics(now pcommon.Timestamp, stats *plus ) } -func (nps *nginxPlusScraper) recordSlabPageMetrics(stats *plusapi.Stats, now pcommon.Timestamp) { +func (nps *NginxPlusScraper) recordSlabPageMetrics(stats *plusapi.Stats, now pcommon.Timestamp) { for name, slab := range stats.Slabs { nps.mb.RecordNginxSlabPageFreeDataPoint(now, int64(slab.Pages.Free), name) nps.mb.RecordNginxSlabPageUsageDataPoint(now, int64(slab.Pages.Used), name) @@ -534,7 +534,7 @@ func (nps *nginxPlusScraper) recordSlabPageMetrics(stats *plusapi.Stats, now pco } } -func (nps *nginxPlusScraper) recordHTTPUpstreamPeerMetrics(stats *plusapi.Stats, now pcommon.Timestamp) { +func (nps *NginxPlusScraper) recordHTTPUpstreamPeerMetrics(stats *plusapi.Stats, now pcommon.Timestamp) { for name, upstream := range stats.Upstreams { nps.mb.RecordNginxHTTPUpstreamKeepaliveCountDataPoint(now, int64(upstream.Keepalive), upstream.Zone, name) @@ -803,7 +803,7 @@ func (nps *nginxPlusScraper) recordHTTPUpstreamPeerMetrics(stats *plusapi.Stats, } } -func (nps *nginxPlusScraper) recordServerZoneMetrics(stats *plusapi.Stats, now pcommon.Timestamp) { +func (nps *NginxPlusScraper) recordServerZoneMetrics(stats *plusapi.Stats, now pcommon.Timestamp) { for szName, sz := range stats.ServerZones { nps.mb.RecordNginxHTTPRequestIoDataPoint( now, @@ -838,7 +838,7 @@ func (nps *nginxPlusScraper) recordServerZoneMetrics(stats *plusapi.Stats, now p // Duplicate of recordLocationZoneHTTPMetrics but same function can not be used due to plusapi.ServerZone // nolint: dupl -func (nps *nginxPlusScraper) recordServerZoneHTTPMetrics(sz plusapi.ServerZone, szName string, now pcommon.Timestamp) { +func (nps *NginxPlusScraper) recordServerZoneHTTPMetrics(sz plusapi.ServerZone, szName string, now pcommon.Timestamp) { nps.mb.RecordNginxHTTPResponseStatusDataPoint(now, int64(sz.Responses.Responses1xx), metadata.AttributeNginxStatusRange1xx, szName, @@ -908,7 +908,7 @@ func (nps *nginxPlusScraper) recordServerZoneHTTPMetrics(sz plusapi.ServerZone, nps.previousServerZoneResponses[szName] = respStatus } -func (nps *nginxPlusScraper) recordLocationZoneMetrics(stats *plusapi.Stats, now pcommon.Timestamp) { +func (nps *NginxPlusScraper) recordLocationZoneMetrics(stats *plusapi.Stats, now pcommon.Timestamp) { for lzName, lz := range stats.LocationZones { nps.mb.RecordNginxHTTPRequestIoDataPoint( now, @@ -943,7 +943,7 @@ func (nps *nginxPlusScraper) recordLocationZoneMetrics(stats *plusapi.Stats, now // Duplicate of recordServerZoneHTTPMetrics but same function can not be used due to plusapi.LocationZone // nolint: dupl -func (nps *nginxPlusScraper) recordLocationZoneHTTPMetrics(lz plusapi.LocationZone, +func (nps *NginxPlusScraper) recordLocationZoneHTTPMetrics(lz plusapi.LocationZone, lzName string, now pcommon.Timestamp, ) { nps.mb.RecordNginxHTTPResponseStatusDataPoint(now, int64(lz.Responses.Responses1xx), @@ -1015,7 +1015,7 @@ func (nps *nginxPlusScraper) recordLocationZoneHTTPMetrics(lz plusapi.LocationZo nps.previousLocationZoneResponses[lzName] = respStatus } -func (nps *nginxPlusScraper) recordHTTPLimitMetrics(stats *plusapi.Stats, now pcommon.Timestamp) { +func (nps *NginxPlusScraper) recordHTTPLimitMetrics(stats *plusapi.Stats, now pcommon.Timestamp) { for name, limitConnection := range stats.HTTPLimitConnections { nps.mb.RecordNginxHTTPLimitConnRequestsDataPoint( now, @@ -1071,7 +1071,7 @@ func (nps *nginxPlusScraper) recordHTTPLimitMetrics(stats *plusapi.Stats, now pc } } -func (nps *nginxPlusScraper) recordCacheMetrics(stats *plusapi.Stats, now pcommon.Timestamp) { +func (nps *NginxPlusScraper) recordCacheMetrics(stats *plusapi.Stats, now pcommon.Timestamp) { for name, cache := range stats.Caches { nps.mb.RecordNginxCacheBytesReadDataPoint( now, @@ -1174,7 +1174,7 @@ func socketClient(socketPath string) *http.Client { } } -func (nps *nginxPlusScraper) Shutdown(ctx context.Context) error { +func (nps *NginxPlusScraper) Shutdown(ctx context.Context) error { return nil } diff --git a/internal/collector/otel_collector_plugin.go b/internal/collector/otel_collector_plugin.go index ea8f1216bd..398a2d854d 100644 --- a/internal/collector/otel_collector_plugin.go +++ b/internal/collector/otel_collector_plugin.go @@ -418,6 +418,8 @@ func (oc *Collector) checkForNewReceivers(nginxConfigContext *model.NginxConfigC if tcplogReceiversFound { reloadCollector = true } + } else { + slog.Warn("NAP logs feature disabled", "enabled_features", oc.config.Features) } return reloadCollector diff --git a/internal/command/command_plugin.go b/internal/command/command_plugin.go index c72aeb74e3..a1b656fa57 100644 --- a/internal/command/command_plugin.go +++ b/internal/command/command_plugin.go @@ -104,7 +104,7 @@ func (cp *CommandPlugin) Process(ctx context.Context, msg *bus.Message) { func (cp *CommandPlugin) processResourceUpdate(ctx context.Context, msg *bus.Message) { if resource, ok := msg.Data.(*mpi.Resource); ok { - if !cp.commandService.IsConnected() && cp.config.IsFeatureEnabled(pkgConfig.FeatureConnection) { + if !cp.commandService.IsConnected() { cp.createConnection(ctx, resource) } else { statusErr := cp.commandService.UpdateDataPlaneStatus(ctx, resource) @@ -237,8 +237,8 @@ func (cp *CommandPlugin) handleAPIActionRequest(ctx context.Context, message *mp } else { slog.WarnContext( ctx, - "API Action Request feature disabled. Unable to process API action request", - "request", message, + "API action feature disabled. Unable to process API action request", + "request", message, "enabled_features", cp.config.Features, ) err := cp.commandService.SendDataPlaneResponse(ctx, &mpi.DataPlaneResponse{ @@ -263,7 +263,7 @@ func (cp *CommandPlugin) handleConfigApplyRequest(newCtx context.Context, messag slog.WarnContext( newCtx, "Configuration feature disabled. Unable to process config apply request", - "request", message, + "request", message, "enabled_features", cp.config.Features, ) err := cp.commandService.SendDataPlaneResponse(newCtx, &mpi.DataPlaneResponse{ @@ -288,7 +288,7 @@ func (cp *CommandPlugin) handleConfigUploadRequest(newCtx context.Context, messa slog.WarnContext( newCtx, "Configuration feature disabled. Unable to process config upload request", - "request", message, + "request", message, "enabled_features", cp.config.Features, ) err := cp.commandService.SendDataPlaneResponse(newCtx, &mpi.DataPlaneResponse{ diff --git a/internal/plugin/plugin_manager.go b/internal/plugin/plugin_manager.go index 7744ce7833..27b3d10c82 100644 --- a/internal/plugin/plugin_manager.go +++ b/internal/plugin/plugin_manager.go @@ -9,6 +9,8 @@ import ( "context" "log/slog" + pkg "github.com/nginx/agent/v3/pkg/config" + "github.com/nginx/agent/v3/internal/collector" "github.com/nginx/agent/v3/internal/command" "github.com/nginx/agent/v3/internal/file" @@ -39,6 +41,13 @@ func addResourcePlugin(plugins []bus.Plugin, agentConfig *config.Config) []bus.P } func addCommandAndFilePlugins(ctx context.Context, plugins []bus.Plugin, agentConfig *config.Config) []bus.Plugin { + if !agentConfig.IsFeatureEnabled(pkg.FeatureConnection) { + slog.WarnContext(ctx, "Connection feature is disabled, no gRPC connection will be created", + "enabled_features", agentConfig.Features) + + return plugins + } + if isGrpcClientConfigured(agentConfig) { grpcConnection, err := grpc.NewGrpcConnection(ctx, agentConfig) if err != nil { @@ -58,6 +67,12 @@ func addCommandAndFilePlugins(ctx context.Context, plugins []bus.Plugin, agentCo } func addCollectorPlugin(ctx context.Context, agentConfig *config.Config, plugins []bus.Plugin) []bus.Plugin { + if !agentConfig.IsFeatureEnabled(pkg.FeatureMetrics) { + slog.WarnContext(ctx, "Metrics feature disabled, no metrics will be collected", + "enabled_features", agentConfig.Features) + + return plugins + } if agentConfig.IsACollectorExporterConfigured() { oTelCollector, err := collector.New(agentConfig) if err == nil { diff --git a/internal/plugin/plugin_manager_test.go b/internal/plugin/plugin_manager_test.go index 89b86668b5..21b3ac6f88 100644 --- a/internal/plugin/plugin_manager_test.go +++ b/internal/plugin/plugin_manager_test.go @@ -9,6 +9,8 @@ import ( "context" "testing" + pkg "github.com/nginx/agent/v3/pkg/config" + "github.com/nginx/agent/v3/internal/collector" "github.com/nginx/agent/v3/internal/command" "github.com/nginx/agent/v3/internal/file" @@ -35,7 +37,8 @@ func TestLoadPlugins(t *testing.T) { &resource.Resource{}, &watcher.Watcher{}, }, - }, { + }, + { name: "Test 2: Load file and command plugins", input: &config.Config{ Command: &config.Command{ @@ -53,7 +56,8 @@ func TestLoadPlugins(t *testing.T) { &file.FilePlugin{}, &watcher.Watcher{}, }, - }, { + }, + { name: "Test 3: Load metrics collector plugin", input: &config.Config{ Collector: &config.Collector{ @@ -61,9 +65,101 @@ func TestLoadPlugins(t *testing.T) { Debug: &config.DebugExporter{}, }, }, + Features: config.DefaultFeatures(), + }, + expected: []bus.Plugin{ + &resource.Resource{}, + &collector.Collector{}, + &watcher.Watcher{}, + }, + }, + { + name: "Test 4: Metrics collector plugin, feature disabled", + input: &config.Config{ + Command: &config.Command{ + Server: &config.ServerConfig{ + Host: "127.0.0.1", + Port: 443, + Type: config.Grpc, + }, + }, + Collector: &config.Collector{ + Exporters: config.Exporters{ + Debug: &config.DebugExporter{}, + }, + }, + Features: []string{ + pkg.FeatureConfiguration, + pkg.FeatureConnection, + pkg.FeatureFileWatcher, + }, }, expected: []bus.Plugin{ &resource.Resource{}, + &command.CommandPlugin{}, + &file.FilePlugin{}, + &watcher.Watcher{}, + }, + }, + { + name: "Test 6: No Connection feature enabled", + input: &config.Config{ + Command: &config.Command{ + Server: &config.ServerConfig{ + Host: "127.0.0.1", + Port: 443, + Type: config.Grpc, + }, + }, + Collector: &config.Collector{ + Exporters: config.Exporters{ + Debug: &config.DebugExporter{}, + }, + }, + Features: []string{ + pkg.FeatureConfiguration, + pkg.FeatureMetrics, + pkg.FeatureFileWatcher, + pkg.FeatureCertificates, + pkg.FeatureAPIAction, + pkg.FeatureLogsNap, + }, + }, + expected: []bus.Plugin{ + &resource.Resource{}, + &collector.Collector{}, + &watcher.Watcher{}, + }, + }, + { + name: "Test 7: All features enabled", + input: &config.Config{ + Command: &config.Command{ + Server: &config.ServerConfig{ + Host: "127.0.0.1", + Port: 443, + Type: config.Grpc, + }, + }, + Collector: &config.Collector{ + Exporters: config.Exporters{ + Debug: &config.DebugExporter{}, + }, + }, + Features: []string{ + pkg.FeatureConfiguration, + pkg.FeatureMetrics, + pkg.FeatureFileWatcher, + pkg.FeatureConnection, + pkg.FeatureCertificates, + pkg.FeatureAPIAction, + pkg.FeatureLogsNap, + }, + }, + expected: []bus.Plugin{ + &resource.Resource{}, + &command.CommandPlugin{}, + &file.FilePlugin{}, &collector.Collector{}, &watcher.Watcher{}, }, diff --git a/internal/watcher/instance/nginx_config_parser.go b/internal/watcher/instance/nginx_config_parser.go index 7f732cbb70..ea6f8a85b3 100644 --- a/internal/watcher/instance/nginx_config_parser.go +++ b/internal/watcher/instance/nginx_config_parser.go @@ -20,6 +20,8 @@ import ( "strconv" "strings" + pkg "github.com/nginx/agent/v3/pkg/config" + mpi "github.com/nginx/agent/v3/api/grpc/mpi/v1" "github.com/nginx/agent/v3/internal/config" "github.com/nginx/agent/v3/internal/model" @@ -147,9 +149,14 @@ func (ncp *NginxConfigParser) createNginxConfigContext( } case "ssl_certificate", "proxy_ssl_certificate", "ssl_client_certificate", "ssl_trusted_certificate": - sslCertFile := ncp.sslCert(ctx, directive.Args[0], rootDir) - if !ncp.isDuplicateFile(nginxConfigContext.Files, sslCertFile) { - nginxConfigContext.Files = append(nginxConfigContext.Files, sslCertFile) + if ncp.agentConfig.IsFeatureEnabled(pkg.FeatureCertificates) { + sslCertFile := ncp.sslCert(ctx, directive.Args[0], rootDir) + if !ncp.isDuplicateFile(nginxConfigContext.Files, sslCertFile) { + nginxConfigContext.Files = append(nginxConfigContext.Files, sslCertFile) + } + } else { + slog.InfoContext(ctx, "Certificate feature is disabled, skipping cert", + "enabled_features", ncp.agentConfig.Features) } case "app_protect_security_log": diff --git a/internal/watcher/watcher_plugin.go b/internal/watcher/watcher_plugin.go index a2c67b0115..892412416d 100644 --- a/internal/watcher/watcher_plugin.go +++ b/internal/watcher/watcher_plugin.go @@ -102,6 +102,9 @@ func (w *Watcher) Init(ctx context.Context, messagePipe bus.MessagePipeInterface if w.agentConfig.IsFeatureEnabled(pkgConfig.FeatureFileWatcher) { go w.fileWatcherService.Watch(watcherContext, w.fileUpdatesChannel) + } else { + slog.InfoContext(watcherContext, "File watcher feature is disabled", + "enabled_features", w.agentConfig.Features) } go w.monitorWatchers(watcherContext) diff --git a/pkg/config/features.go b/pkg/config/features.go index d6a96a9e3a..e3c9364c5e 100644 --- a/pkg/config/features.go +++ b/pkg/config/features.go @@ -14,7 +14,6 @@ const ( FeatureMetricsHost = "metrics-host" FeatureMetricsInstance = "metrics-instance" FeatureFileWatcher = "file-watcher" - FeatureAgentAPI = "agent-api" FeatureAPIAction = "api-action" FeatureLogsNap = "logs-nap" ) From 50e53a43519bce21363c31879ee7fd2167be9c7b Mon Sep 17 00:00:00 2001 From: Aphral Griffin Date: Thu, 17 Apr 2025 16:22:38 +0100 Subject: [PATCH 2/9] enabled disable features --- pkg/config/features.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/config/features.go b/pkg/config/features.go index e3c9364c5e..d33690828b 100644 --- a/pkg/config/features.go +++ b/pkg/config/features.go @@ -15,5 +15,6 @@ const ( FeatureMetricsInstance = "metrics-instance" FeatureFileWatcher = "file-watcher" FeatureAPIAction = "api-action" - FeatureLogsNap = "logs-nap" + // experimental feature + FeatureLogsNap = "logs-nap" ) From 35a9994565882c118c7e8f775ac8989f3df437a5 Mon Sep 17 00:00:00 2001 From: Aphral Griffin Date: Fri, 18 Apr 2025 10:35:39 +0100 Subject: [PATCH 3/9] remove connection feature --- internal/command/command_plugin_test.go | 3 --- internal/config/defaults.go | 1 - internal/plugin/plugin_manager.go | 7 ------ internal/plugin/plugin_manager_test.go | 32 ------------------------- pkg/config/features.go | 16 +++++-------- 5 files changed, 6 insertions(+), 53 deletions(-) diff --git a/internal/command/command_plugin_test.go b/internal/command/command_plugin_test.go index 2c532bf654..19c54f92a7 100644 --- a/internal/command/command_plugin_test.go +++ b/internal/command/command_plugin_test.go @@ -204,7 +204,6 @@ func TestCommandPlugin_monitorSubscribeChannel(t *testing.T) { request: "APIActionRequest", configFeatures: []string{ pkg.FeatureConfiguration, - pkg.FeatureConnection, pkg.FeatureMetrics, pkg.FeatureFileWatcher, pkg.FeatureAPIAction, @@ -278,7 +277,6 @@ func TestCommandPlugin_FeatureDisabled(t *testing.T) { expectedLog: "Configuration feature disabled. Unable to process config upload request", request: "UploadRequest", configFeatures: []string{ - pkg.FeatureConnection, pkg.FeatureMetrics, pkg.FeatureFileWatcher, }, @@ -293,7 +291,6 @@ func TestCommandPlugin_FeatureDisabled(t *testing.T) { expectedLog: "Configuration feature disabled. Unable to process config apply request", request: "ApplyRequest", configFeatures: []string{ - pkg.FeatureConnection, pkg.FeatureMetrics, pkg.FeatureFileWatcher, }, diff --git a/internal/config/defaults.go b/internal/config/defaults.go index 4c51eb006e..3d9bb5329b 100644 --- a/internal/config/defaults.go +++ b/internal/config/defaults.go @@ -78,7 +78,6 @@ const ( func DefaultFeatures() []string { return []string{ pkg.FeatureConfiguration, - pkg.FeatureConnection, pkg.FeatureMetrics, pkg.FeatureFileWatcher, } diff --git a/internal/plugin/plugin_manager.go b/internal/plugin/plugin_manager.go index 27b3d10c82..a62310b95e 100644 --- a/internal/plugin/plugin_manager.go +++ b/internal/plugin/plugin_manager.go @@ -41,13 +41,6 @@ func addResourcePlugin(plugins []bus.Plugin, agentConfig *config.Config) []bus.P } func addCommandAndFilePlugins(ctx context.Context, plugins []bus.Plugin, agentConfig *config.Config) []bus.Plugin { - if !agentConfig.IsFeatureEnabled(pkg.FeatureConnection) { - slog.WarnContext(ctx, "Connection feature is disabled, no gRPC connection will be created", - "enabled_features", agentConfig.Features) - - return plugins - } - if isGrpcClientConfigured(agentConfig) { grpcConnection, err := grpc.NewGrpcConnection(ctx, agentConfig) if err != nil { diff --git a/internal/plugin/plugin_manager_test.go b/internal/plugin/plugin_manager_test.go index 21b3ac6f88..376c6b7eeb 100644 --- a/internal/plugin/plugin_manager_test.go +++ b/internal/plugin/plugin_manager_test.go @@ -90,7 +90,6 @@ func TestLoadPlugins(t *testing.T) { }, Features: []string{ pkg.FeatureConfiguration, - pkg.FeatureConnection, pkg.FeatureFileWatcher, }, }, @@ -101,36 +100,6 @@ func TestLoadPlugins(t *testing.T) { &watcher.Watcher{}, }, }, - { - name: "Test 6: No Connection feature enabled", - input: &config.Config{ - Command: &config.Command{ - Server: &config.ServerConfig{ - Host: "127.0.0.1", - Port: 443, - Type: config.Grpc, - }, - }, - Collector: &config.Collector{ - Exporters: config.Exporters{ - Debug: &config.DebugExporter{}, - }, - }, - Features: []string{ - pkg.FeatureConfiguration, - pkg.FeatureMetrics, - pkg.FeatureFileWatcher, - pkg.FeatureCertificates, - pkg.FeatureAPIAction, - pkg.FeatureLogsNap, - }, - }, - expected: []bus.Plugin{ - &resource.Resource{}, - &collector.Collector{}, - &watcher.Watcher{}, - }, - }, { name: "Test 7: All features enabled", input: &config.Config{ @@ -150,7 +119,6 @@ func TestLoadPlugins(t *testing.T) { pkg.FeatureConfiguration, pkg.FeatureMetrics, pkg.FeatureFileWatcher, - pkg.FeatureConnection, pkg.FeatureCertificates, pkg.FeatureAPIAction, pkg.FeatureLogsNap, diff --git a/pkg/config/features.go b/pkg/config/features.go index d33690828b..0e1720a5d3 100644 --- a/pkg/config/features.go +++ b/pkg/config/features.go @@ -6,15 +6,11 @@ package config const ( - FeatureCertificates = "certificates" - FeatureConfiguration = "configuration" - FeatureConnection = "connection" - FeatureMetrics = "metrics" - FeatureMetricsContainer = "metrics-container" - FeatureMetricsHost = "metrics-host" - FeatureMetricsInstance = "metrics-instance" - FeatureFileWatcher = "file-watcher" - FeatureAPIAction = "api-action" - // experimental feature + FeatureCertificates = "certificates" + FeatureConfiguration = "configuration" + FeatureMetrics = "metrics" + FeatureFileWatcher = "file-watcher" + FeatureAPIAction = "api-action" + // FeatureLogsNap experimental feature FeatureLogsNap = "logs-nap" ) From b25bba39ffc96d804e1e969612b440b67f7bbe39 Mon Sep 17 00:00:00 2001 From: Aphral Griffin Date: Fri, 18 Apr 2025 11:16:50 +0100 Subject: [PATCH 4/9] undo change --- .../collector/nginxplusreceiver/scraper.go | 36 +++++++++---------- 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/internal/collector/nginxplusreceiver/scraper.go b/internal/collector/nginxplusreceiver/scraper.go index b35c607398..7b6cb0ad5f 100644 --- a/internal/collector/nginxplusreceiver/scraper.go +++ b/internal/collector/nginxplusreceiver/scraper.go @@ -34,7 +34,7 @@ const ( peerStateUnhealthy = "unhealthy" ) -type NginxPlusScraper struct { +type nginxPlusScraper struct { previousServerZoneResponses map[string]ResponseStatuses previousLocationZoneResponses map[string]ResponseStatuses plusClient *plusapi.NginxClient @@ -57,7 +57,7 @@ type ResponseStatuses struct { func newNginxPlusScraper( settings receiver.Settings, cfg *Config, -) (*NginxPlusScraper, error) { +) (*nginxPlusScraper, error) { endpoint := strings.TrimPrefix(cfg.APIDetails.URL, "unix:") logger := settings.Logger logger.Info("Creating NGINX Plus scraper") @@ -79,7 +79,7 @@ func newNginxPlusScraper( return nil, err } - return &NginxPlusScraper{ + return &nginxPlusScraper{ plusClient: plusClient, settings: settings, cfg: cfg, @@ -89,7 +89,7 @@ func newNginxPlusScraper( }, nil } -func (nps *NginxPlusScraper) createPreviousLocationZoneResponses(stats *plusapi.Stats) { +func (nps *nginxPlusScraper) createPreviousLocationZoneResponses(stats *plusapi.Stats) { previousLocationZoneResponses := make(map[string]ResponseStatuses) for lzName, lz := range stats.LocationZones { respStatus := ResponseStatuses{ @@ -106,7 +106,7 @@ func (nps *NginxPlusScraper) createPreviousLocationZoneResponses(stats *plusapi. nps.previousLocationZoneResponses = previousLocationZoneResponses } -func (nps *NginxPlusScraper) createPreviousServerZoneResponses(stats *plusapi.Stats) { +func (nps *nginxPlusScraper) createPreviousServerZoneResponses(stats *plusapi.Stats) { previousServerZoneResponses := make(map[string]ResponseStatuses) for szName, sz := range stats.ServerZones { respStatus := ResponseStatuses{ @@ -123,7 +123,7 @@ func (nps *NginxPlusScraper) createPreviousServerZoneResponses(stats *plusapi.St nps.previousServerZoneResponses = previousServerZoneResponses } -func (nps *NginxPlusScraper) scrape(ctx context.Context) (pmetric.Metrics, error) { +func (nps *nginxPlusScraper) scrape(ctx context.Context) (pmetric.Metrics, error) { // nps.init.Do is ran only once, it is only ran the first time scrape is called to set the previous responses // metric value nps.init.Do(func() { @@ -151,7 +151,7 @@ func (nps *NginxPlusScraper) scrape(ctx context.Context) (pmetric.Metrics, error return nps.mb.Emit(metadata.WithResource(nps.rb.Emit())), nil } -func (nps *NginxPlusScraper) recordMetrics(stats *plusapi.Stats) { +func (nps *nginxPlusScraper) recordMetrics(stats *plusapi.Stats) { now := pcommon.NewTimestampFromTime(time.Now()) // NGINX config reloads @@ -193,7 +193,7 @@ func (nps *NginxPlusScraper) recordMetrics(stats *plusapi.Stats) { nps.recordStreamMetrics(stats, now) } -func (nps *NginxPlusScraper) recordStreamMetrics(stats *plusapi.Stats, now pcommon.Timestamp) { +func (nps *nginxPlusScraper) recordStreamMetrics(stats *plusapi.Stats, now pcommon.Timestamp) { for name, streamServerZone := range stats.StreamServerZones { nps.mb.RecordNginxStreamIoDataPoint( now, @@ -437,7 +437,7 @@ func (nps *NginxPlusScraper) recordStreamMetrics(stats *plusapi.Stats, now pcomm } } -func (nps *NginxPlusScraper) recordSSLMetrics(now pcommon.Timestamp, stats *plusapi.Stats) { +func (nps *nginxPlusScraper) recordSSLMetrics(now pcommon.Timestamp, stats *plusapi.Stats) { nps.mb.RecordNginxSslHandshakesDataPoint( now, int64(stats.SSL.HandshakesFailed), @@ -503,7 +503,7 @@ func (nps *NginxPlusScraper) recordSSLMetrics(now pcommon.Timestamp, stats *plus ) } -func (nps *NginxPlusScraper) recordSlabPageMetrics(stats *plusapi.Stats, now pcommon.Timestamp) { +func (nps *nginxPlusScraper) recordSlabPageMetrics(stats *plusapi.Stats, now pcommon.Timestamp) { for name, slab := range stats.Slabs { nps.mb.RecordNginxSlabPageFreeDataPoint(now, int64(slab.Pages.Free), name) nps.mb.RecordNginxSlabPageUsageDataPoint(now, int64(slab.Pages.Used), name) @@ -534,7 +534,7 @@ func (nps *NginxPlusScraper) recordSlabPageMetrics(stats *plusapi.Stats, now pco } } -func (nps *NginxPlusScraper) recordHTTPUpstreamPeerMetrics(stats *plusapi.Stats, now pcommon.Timestamp) { +func (nps *nginxPlusScraper) recordHTTPUpstreamPeerMetrics(stats *plusapi.Stats, now pcommon.Timestamp) { for name, upstream := range stats.Upstreams { nps.mb.RecordNginxHTTPUpstreamKeepaliveCountDataPoint(now, int64(upstream.Keepalive), upstream.Zone, name) @@ -803,7 +803,7 @@ func (nps *NginxPlusScraper) recordHTTPUpstreamPeerMetrics(stats *plusapi.Stats, } } -func (nps *NginxPlusScraper) recordServerZoneMetrics(stats *plusapi.Stats, now pcommon.Timestamp) { +func (nps *nginxPlusScraper) recordServerZoneMetrics(stats *plusapi.Stats, now pcommon.Timestamp) { for szName, sz := range stats.ServerZones { nps.mb.RecordNginxHTTPRequestIoDataPoint( now, @@ -838,7 +838,7 @@ func (nps *NginxPlusScraper) recordServerZoneMetrics(stats *plusapi.Stats, now p // Duplicate of recordLocationZoneHTTPMetrics but same function can not be used due to plusapi.ServerZone // nolint: dupl -func (nps *NginxPlusScraper) recordServerZoneHTTPMetrics(sz plusapi.ServerZone, szName string, now pcommon.Timestamp) { +func (nps *nginxPlusScraper) recordServerZoneHTTPMetrics(sz plusapi.ServerZone, szName string, now pcommon.Timestamp) { nps.mb.RecordNginxHTTPResponseStatusDataPoint(now, int64(sz.Responses.Responses1xx), metadata.AttributeNginxStatusRange1xx, szName, @@ -908,7 +908,7 @@ func (nps *NginxPlusScraper) recordServerZoneHTTPMetrics(sz plusapi.ServerZone, nps.previousServerZoneResponses[szName] = respStatus } -func (nps *NginxPlusScraper) recordLocationZoneMetrics(stats *plusapi.Stats, now pcommon.Timestamp) { +func (nps *nginxPlusScraper) recordLocationZoneMetrics(stats *plusapi.Stats, now pcommon.Timestamp) { for lzName, lz := range stats.LocationZones { nps.mb.RecordNginxHTTPRequestIoDataPoint( now, @@ -943,7 +943,7 @@ func (nps *NginxPlusScraper) recordLocationZoneMetrics(stats *plusapi.Stats, now // Duplicate of recordServerZoneHTTPMetrics but same function can not be used due to plusapi.LocationZone // nolint: dupl -func (nps *NginxPlusScraper) recordLocationZoneHTTPMetrics(lz plusapi.LocationZone, +func (nps *nginxPlusScraper) recordLocationZoneHTTPMetrics(lz plusapi.LocationZone, lzName string, now pcommon.Timestamp, ) { nps.mb.RecordNginxHTTPResponseStatusDataPoint(now, int64(lz.Responses.Responses1xx), @@ -1015,7 +1015,7 @@ func (nps *NginxPlusScraper) recordLocationZoneHTTPMetrics(lz plusapi.LocationZo nps.previousLocationZoneResponses[lzName] = respStatus } -func (nps *NginxPlusScraper) recordHTTPLimitMetrics(stats *plusapi.Stats, now pcommon.Timestamp) { +func (nps *nginxPlusScraper) recordHTTPLimitMetrics(stats *plusapi.Stats, now pcommon.Timestamp) { for name, limitConnection := range stats.HTTPLimitConnections { nps.mb.RecordNginxHTTPLimitConnRequestsDataPoint( now, @@ -1071,7 +1071,7 @@ func (nps *NginxPlusScraper) recordHTTPLimitMetrics(stats *plusapi.Stats, now pc } } -func (nps *NginxPlusScraper) recordCacheMetrics(stats *plusapi.Stats, now pcommon.Timestamp) { +func (nps *nginxPlusScraper) recordCacheMetrics(stats *plusapi.Stats, now pcommon.Timestamp) { for name, cache := range stats.Caches { nps.mb.RecordNginxCacheBytesReadDataPoint( now, @@ -1174,7 +1174,7 @@ func socketClient(socketPath string) *http.Client { } } -func (nps *NginxPlusScraper) Shutdown(ctx context.Context) error { +func (nps *nginxPlusScraper) Shutdown(ctx context.Context) error { return nil } From 5c38ebaf31ceb587d54aef6ed54319a321096484 Mon Sep 17 00:00:00 2001 From: Aphral Griffin Date: Fri, 18 Apr 2025 15:08:30 +0100 Subject: [PATCH 5/9] PR feedback --- internal/plugin/plugin_manager_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/plugin/plugin_manager_test.go b/internal/plugin/plugin_manager_test.go index 376c6b7eeb..ff0dca6d0c 100644 --- a/internal/plugin/plugin_manager_test.go +++ b/internal/plugin/plugin_manager_test.go @@ -101,7 +101,7 @@ func TestLoadPlugins(t *testing.T) { }, }, { - name: "Test 7: All features enabled", + name: "Test 5: All features enabled", input: &config.Config{ Command: &config.Command{ Server: &config.ServerConfig{ From b7f92de22b9c88f4432eec47b64b0b9fb864c30b Mon Sep 17 00:00:00 2001 From: Aphral Griffin Date: Wed, 23 Apr 2025 13:42:35 +0100 Subject: [PATCH 6/9] update defaults --- internal/config/defaults.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/config/defaults.go b/internal/config/defaults.go index 3d9bb5329b..006b230d59 100644 --- a/internal/config/defaults.go +++ b/internal/config/defaults.go @@ -78,6 +78,7 @@ const ( func DefaultFeatures() []string { return []string{ pkg.FeatureConfiguration, + pkg.FeatureCertificates, pkg.FeatureMetrics, pkg.FeatureFileWatcher, } From f4bdd5ee5f6b68b78b8f358153dcaf42a8ba6f6b Mon Sep 17 00:00:00 2001 From: Aphral Griffin Date: Wed, 23 Apr 2025 14:49:58 +0100 Subject: [PATCH 7/9] log level --- internal/watcher/instance/nginx_config_parser.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/internal/watcher/instance/nginx_config_parser.go b/internal/watcher/instance/nginx_config_parser.go index a0f1ed040f..5020dea268 100644 --- a/internal/watcher/instance/nginx_config_parser.go +++ b/internal/watcher/instance/nginx_config_parser.go @@ -153,14 +153,13 @@ func (ncp *NginxConfigParser) createNginxConfigContext( if ncp.agentConfig.IsFeatureEnabled(pkg.FeatureCertificates) { sslCertFile := ncp.sslCert(ctx, directive.Args[0], rootDir) if sslCertFile != nil && !ncp.isDuplicateFile(nginxConfigContext.Files, sslCertFile) { - slog.InfoContext(ctx, "Adding SSL certificate file", "ssl_cert", sslCertFile) + slog.DebugContext(ctx, "Adding SSL certificate file", "ssl_cert", sslCertFile) nginxConfigContext.Files = append(nginxConfigContext.Files, sslCertFile) } } else { slog.InfoContext(ctx, "Certificate feature is disabled, skipping cert", "enabled_features", ncp.agentConfig.Features) } - case "app_protect_security_log": if len(directive.Args) > 1 { syslogArg := directive.Args[1] From fc4f2affd6728f6e8b483e7461115556e8b1d067 Mon Sep 17 00:00:00 2001 From: Aphral Griffin Date: Wed, 23 Apr 2025 16:19:36 +0100 Subject: [PATCH 8/9] fix --- internal/app.go | 1 + internal/config/config.go | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/app.go b/internal/app.go index 7cbd1a0373..8636b582ee 100644 --- a/internal/app.go +++ b/internal/app.go @@ -53,6 +53,7 @@ func (a *App) Run(ctx context.Context) error { slog.String("version", a.version), slog.String("commit", a.commit), ) + slog.Info("Enabled features", "features", agentConfig.Features) messagePipe := bus.NewMessagePipe(defaultMessagePipeChannelSize) err = messagePipe.Register(defaultQueueSize, plugin.LoadPlugins(ctx, agentConfig)) diff --git a/internal/config/config.go b/internal/config/config.go index d5eaf872cc..efb1c8b20d 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -121,7 +121,6 @@ func ResolveConfig() (*Config, error) { checkCollectorConfiguration(collector, config) slog.Debug("Agent config", "config", config) - slog.Info("Enabled features", "features", config.Features) slog.Info("Excluded files from being watched for file changes", "exclude_files", config.Watchers.FileWatcher.ExcludeFiles) From 210a0ae7ae27606dac41d249588f2b31fc81d285 Mon Sep 17 00:00:00 2001 From: Aphral Griffin Date: Fri, 25 Apr 2025 10:27:00 +0100 Subject: [PATCH 9/9] PR feedback --- internal/app.go | 1 - internal/collector/otel_collector_plugin.go | 2 +- internal/watcher/instance/nginx_config_parser.go | 2 +- internal/watcher/watcher_plugin.go | 2 +- 4 files changed, 3 insertions(+), 4 deletions(-) diff --git a/internal/app.go b/internal/app.go index 8636b582ee..7cbd1a0373 100644 --- a/internal/app.go +++ b/internal/app.go @@ -53,7 +53,6 @@ func (a *App) Run(ctx context.Context) error { slog.String("version", a.version), slog.String("commit", a.commit), ) - slog.Info("Enabled features", "features", agentConfig.Features) messagePipe := bus.NewMessagePipe(defaultMessagePipeChannelSize) err = messagePipe.Register(defaultQueueSize, plugin.LoadPlugins(ctx, agentConfig)) diff --git a/internal/collector/otel_collector_plugin.go b/internal/collector/otel_collector_plugin.go index 398a2d854d..4956bb0c5a 100644 --- a/internal/collector/otel_collector_plugin.go +++ b/internal/collector/otel_collector_plugin.go @@ -419,7 +419,7 @@ func (oc *Collector) checkForNewReceivers(nginxConfigContext *model.NginxConfigC reloadCollector = true } } else { - slog.Warn("NAP logs feature disabled", "enabled_features", oc.config.Features) + slog.Debug("NAP logs feature disabled", "enabled_features", oc.config.Features) } return reloadCollector diff --git a/internal/watcher/instance/nginx_config_parser.go b/internal/watcher/instance/nginx_config_parser.go index 5020dea268..af9a8e3b94 100644 --- a/internal/watcher/instance/nginx_config_parser.go +++ b/internal/watcher/instance/nginx_config_parser.go @@ -157,7 +157,7 @@ func (ncp *NginxConfigParser) createNginxConfigContext( nginxConfigContext.Files = append(nginxConfigContext.Files, sslCertFile) } } else { - slog.InfoContext(ctx, "Certificate feature is disabled, skipping cert", + slog.DebugContext(ctx, "Certificate feature is disabled, skipping cert", "enabled_features", ncp.agentConfig.Features) } case "app_protect_security_log": diff --git a/internal/watcher/watcher_plugin.go b/internal/watcher/watcher_plugin.go index 892412416d..3fb7ec544c 100644 --- a/internal/watcher/watcher_plugin.go +++ b/internal/watcher/watcher_plugin.go @@ -103,7 +103,7 @@ func (w *Watcher) Init(ctx context.Context, messagePipe bus.MessagePipeInterface if w.agentConfig.IsFeatureEnabled(pkgConfig.FeatureFileWatcher) { go w.fileWatcherService.Watch(watcherContext, w.fileUpdatesChannel) } else { - slog.InfoContext(watcherContext, "File watcher feature is disabled", + slog.DebugContext(watcherContext, "File watcher feature is disabled", "enabled_features", w.agentConfig.Features) }