From 19ac8d199a79369bda36d55ec0c0d166e3201d26 Mon Sep 17 00:00:00 2001 From: Aphral Griffin Date: Wed, 1 Oct 2025 14:41:14 +0100 Subject: [PATCH 01/10] added agent config var --- internal/config/config.go | 9 +++++++++ internal/config/config_test.go | 1 + internal/config/defaults.go | 14 +++++++------- internal/config/flags.go | 1 + internal/config/testdata/nginx-agent.conf | 3 +++ internal/config/types.go | 1 + 6 files changed, 22 insertions(+), 7 deletions(-) diff --git a/internal/config/config.go b/internal/config/config.go index cc72af59c..619a0a1fd 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -755,6 +755,14 @@ func registerCollectorFlags(fs *flag.FlagSet) { "The path to the Opentelemetry Collector configuration file.", ) + fs.StringSlice( + CollectorAdditionalConfigPathsKey, + []string{}, + "Paths to additional OpenTelemetry Collector configuration files. The order of the configuration files"+ + " determines which config file takes priority. The last config file will take precedent over other files "+ + "if they have the same setting. Paths to configuration files must be absolute", + ) + fs.String( CollectorLogLevelKey, DefCollectorLogLevel, @@ -1054,6 +1062,7 @@ func resolveCollector(allowedDirs []string) (*Collector, error) { col := &Collector{ ConfigPath: viperInstance.GetString(CollectorConfigPathKey), + AdditionalPaths: viperInstance.GetStringSlice(CollectorAdditionalConfigPathsKey), Exporters: exporters, Processors: resolveProcessors(), Receivers: receivers, diff --git a/internal/config/config_test.go b/internal/config/config_test.go index f55afee23..4c4ba1278 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -1112,6 +1112,7 @@ func createConfig() *Config { }, Collector: &Collector{ ConfigPath: "/etc/nginx-agent/nginx-agent-otelcol.yaml", + AdditionalPaths: []string{"/configs/my_config.yaml", "/etc/nginx-agent/nginx-agent-otelcol.yaml"}, Exporters: Exporters{ OtlpExporters: map[string]*OtlpExporter{ "default": { diff --git a/internal/config/defaults.go b/internal/config/defaults.go index 615c7bc8b..7174ac1ca 100644 --- a/internal/config/defaults.go +++ b/internal/config/defaults.go @@ -85,13 +85,13 @@ const ( DefFileWatcherMonitoringFrequency = 5 * time.Second // Collector defaults - DefCollectorConfigPath = "/etc/nginx-agent/opentelemetry-collector-agent.yaml" - DefCollectorLogLevel = "INFO" - DefCollectorLogPath = "/var/log/nginx-agent/opentelemetry-collector-agent.log" - DefCollectorTLSCertPath = "/var/lib/nginx-agent/cert.pem" - DefCollectorTLSKeyPath = "/var/lib/nginx-agent/key.pem" - DefCollectorTLSCAPath = "/var/lib/nginx-agent/ca.pem" - DefCollectorTLSSANNames = "127.0.0.1,::1,localhost" + DefCollectorConfigPath = "/etc/nginx-agent/opentelemetry-collector-agent.yaml" + DefCollectorLogLevel = "INFO" + DefCollectorLogPath = "/var/log/nginx-agent/opentelemetry-collector-agent.log" + DefCollectorTLSCertPath = "/var/lib/nginx-agent/cert.pem" + DefCollectorTLSKeyPath = "/var/lib/nginx-agent/key.pem" + DefCollectorTLSCAPath = "/var/lib/nginx-agent/ca.pem" + DefCollectorTLSSANNames = "127.0.0.1,::1,localhost" DefCollectorMetricsBatchProcessorSendBatchSize = 1000 DefCollectorMetricsBatchProcessorSendBatchMaxSize = 1000 diff --git a/internal/config/flags.go b/internal/config/flags.go index 3e51eb52b..697c2906e 100644 --- a/internal/config/flags.go +++ b/internal/config/flags.go @@ -48,6 +48,7 @@ var ( ClientBackoffMultiplierKey = pre(ClientRootKey) + "backoff_multiplier" CollectorConfigPathKey = pre(CollectorRootKey) + "config_path" + CollectorAdditionalConfigPathsKey = pre(CollectorRootKey) + "additional_config_paths" CollectorExportersKey = pre(CollectorRootKey) + "exporters" CollectorDebugExporterKey = pre(CollectorExportersKey) + "debug" CollectorPrometheusExporterKey = pre(CollectorExportersKey) + "prometheus" diff --git a/internal/config/testdata/nginx-agent.conf b/internal/config/testdata/nginx-agent.conf index 9df36198d..6e247a585 100644 --- a/internal/config/testdata/nginx-agent.conf +++ b/internal/config/testdata/nginx-agent.conf @@ -94,6 +94,9 @@ auxiliary_command: collector: config_path: "/etc/nginx-agent/nginx-agent-otelcol.yaml" + additional_config_paths: + - "/configs/my_config.yaml" + - /etc/nginx-agent/nginx-agent-otelcol.yaml receivers: otlp: "default": diff --git a/internal/config/types.go b/internal/config/types.go index a5d07a210..63fc976c9 100644 --- a/internal/config/types.go +++ b/internal/config/types.go @@ -106,6 +106,7 @@ type ( Collector struct { ConfigPath string `yaml:"config_path" mapstructure:"config_path"` + AdditionalPaths []string `yaml:"additional_config_paths" mapstructure:"additional_config_paths"` Log *Log `yaml:"log" mapstructure:"log"` Exporters Exporters `yaml:"exporters" mapstructure:"exporters"` Extensions Extensions `yaml:"extensions" mapstructure:"extensions"` From 69b27ae08a53449e124635742748a065fa166d27 Mon Sep 17 00:00:00 2001 From: Aphral Griffin Date: Wed, 1 Oct 2025 17:24:06 +0100 Subject: [PATCH 02/10] embedded otel collector --- .../internal/config/config.go | 1 + .../scraper/accesslog/nginx_log_scraper.go | 2 +- .../scraper/stubstatus/stub_status_scraper.go | 2 +- .../collector/nginxplusreceiver/config.go | 1 + .../collector/nginxplusreceiver/scraper.go | 2 +- internal/collector/otel_collector_plugin.go | 35 +++++++++++++++++++ internal/collector/otelcol.tmpl | 26 +++++++++++--- internal/collector/settings.go | 6 +++- internal/collector/settings_test.go | 28 ++++++++++++++- internal/config/config.go | 14 ++++---- internal/config/config_test.go | 2 +- internal/config/defaults.go | 14 ++++---- internal/config/types.go | 16 ++++----- .../test-opentelemetry-collector-agent.yaml | 23 ++++++++++-- 14 files changed, 138 insertions(+), 34 deletions(-) diff --git a/internal/collector/nginxossreceiver/internal/config/config.go b/internal/collector/nginxossreceiver/internal/config/config.go index 3d272c1a4..67b90c68d 100644 --- a/internal/collector/nginxossreceiver/internal/config/config.go +++ b/internal/collector/nginxossreceiver/internal/config/config.go @@ -24,6 +24,7 @@ const ( type Config struct { confighttp.ClientConfig `mapstructure:",squash"` APIDetails APIDetails `mapstructure:"api_details"` + InstanceID string `mapstructure:"instance_id"` AccessLogs []AccessLog `mapstructure:"access_logs"` MetricsBuilderConfig metadata.MetricsBuilderConfig `mapstructure:",squash"` scraperhelper.ControllerConfig `mapstructure:",squash"` diff --git a/internal/collector/nginxossreceiver/internal/scraper/accesslog/nginx_log_scraper.go b/internal/collector/nginxossreceiver/internal/scraper/accesslog/nginx_log_scraper.go index a1e3cf64f..92fbce0a2 100644 --- a/internal/collector/nginxossreceiver/internal/scraper/accesslog/nginx_log_scraper.go +++ b/internal/collector/nginxossreceiver/internal/scraper/accesslog/nginx_log_scraper.go @@ -171,7 +171,7 @@ func (nls *NginxLogScraper) Scrape(_ context.Context) (pmetric.Metrics, error) { nls.entries = make([]*entry.Entry, 0) timeNow := pcommon.NewTimestampFromTime(time.Now()) - nls.rb.SetInstanceID(nls.settings.ID.Name()) + nls.rb.SetInstanceID(nls.cfg.InstanceID) nls.rb.SetInstanceType("nginx") nls.logger.Debug("NGINX OSS access log resource info", zap.Any("resource", nls.rb)) diff --git a/internal/collector/nginxossreceiver/internal/scraper/stubstatus/stub_status_scraper.go b/internal/collector/nginxossreceiver/internal/scraper/stubstatus/stub_status_scraper.go index 15b61ec93..4d3002243 100644 --- a/internal/collector/nginxossreceiver/internal/scraper/stubstatus/stub_status_scraper.go +++ b/internal/collector/nginxossreceiver/internal/scraper/stubstatus/stub_status_scraper.go @@ -138,7 +138,7 @@ func (s *NginxStubStatusScraper) Scrape(context.Context) (pmetric.Metrics, error return pmetric.Metrics{}, err } - s.rb.SetInstanceID(s.settings.ID.Name()) + s.rb.SetInstanceID(s.cfg.InstanceID) s.rb.SetInstanceType("nginx") s.settings.Logger.Debug("NGINX OSS stub status resource info", zap.Any("resource", s.rb)) diff --git a/internal/collector/nginxplusreceiver/config.go b/internal/collector/nginxplusreceiver/config.go index 6e622aaea..d633d4fee 100644 --- a/internal/collector/nginxplusreceiver/config.go +++ b/internal/collector/nginxplusreceiver/config.go @@ -21,6 +21,7 @@ const defaultCollectInterval = 10 * time.Second type Config struct { confighttp.ClientConfig `mapstructure:",squash"` APIDetails APIDetails `mapstructure:"api_details"` + InstanceID string `mapstructure:"instance_id"` MetricsBuilderConfig metadata.MetricsBuilderConfig `mapstructure:",squash"` scraperhelper.ControllerConfig `mapstructure:",squash"` } diff --git a/internal/collector/nginxplusreceiver/scraper.go b/internal/collector/nginxplusreceiver/scraper.go index 165545e26..4aa151c9e 100644 --- a/internal/collector/nginxplusreceiver/scraper.go +++ b/internal/collector/nginxplusreceiver/scraper.go @@ -127,7 +127,7 @@ func (nps *NginxPlusScraper) Scrape(ctx context.Context) (pmetric.Metrics, error return pmetric.Metrics{}, fmt.Errorf("failed to get stats from plus API: %w", err) } - nps.rb.SetInstanceID(nps.settings.ID.Name()) + nps.rb.SetInstanceID(nps.cfg.InstanceID) nps.rb.SetInstanceType("nginxplus") nps.logger.Debug("NGINX Plus resource info", zap.Any("resource", nps.rb)) diff --git a/internal/collector/otel_collector_plugin.go b/internal/collector/otel_collector_plugin.go index 5112e68be..9daad0ccd 100644 --- a/internal/collector/otel_collector_plugin.go +++ b/internal/collector/otel_collector_plugin.go @@ -16,7 +16,9 @@ import ( "sync" "time" + "github.com/goccy/go-yaml" pkgConfig "github.com/nginx/agent/v3/pkg/config" + "go.opentelemetry.io/collector/confmap" "github.com/nginx/agent/v3/api/grpc/mpi/v1" "github.com/nginx/agent/v3/internal/backoff" @@ -384,6 +386,31 @@ func (oc *Collector) updateHeadersSetterExtension( return headersSetterExtensionUpdated } +func (oc *Collector) writeRunningConfig(ctx context.Context, settings otelcol.CollectorSettings) error { + slog.DebugContext(ctx, "Writing running OTel collector config", "path", + "/var/lib/nginx-agent/opentelemetry-collector-agent-debug.yaml") + resolver, err := confmap.NewResolver(settings.ConfigProviderSettings.ResolverSettings) + if err != nil { + return fmt.Errorf("unable to create resolver: %w", err) + } + + con, err := resolver.Resolve(ctx) + if err != nil { + return fmt.Errorf("error while resolving config: %w", err) + } + b, err := yaml.Marshal(con.ToStringMap()) + if err != nil { + return fmt.Errorf("error while marshaling to YAML: %w", err) + } + + writeErr := os.WriteFile("/var/lib/nginx-agent/opentelemetry-collector-agent-debug.yaml", b, filePermission) + if writeErr != nil { + return fmt.Errorf("error while writing debug config: %w", err) + } + + return nil +} + func (oc *Collector) restartCollector(ctx context.Context) { err := oc.Close(ctx) if err != nil { @@ -392,6 +419,14 @@ func (oc *Collector) restartCollector(ctx context.Context) { } settings := OTelCollectorSettings(oc.config) + + if strings.ToLower(oc.config.Log.Level) == "debug" { + writeErr := oc.writeRunningConfig(ctx, settings) + if writeErr != nil { + slog.ErrorContext(ctx, "Failed to write debug OTel Collector config", "error", writeErr) + } + } + oTelCollector, err := otelcol.NewCollector(settings) if err != nil { slog.ErrorContext(ctx, "Failed to create OTel Collector", "error", err) diff --git a/internal/collector/otelcol.tmpl b/internal/collector/otelcol.tmpl index 49e46cf23..e0b1015f1 100644 --- a/internal/collector/otelcol.tmpl +++ b/internal/collector/otelcol.tmpl @@ -79,7 +79,12 @@ receivers: {{- end }} {{- range .Receivers.NginxReceivers }} +{{- if gt (len $.Receivers.NginxReceivers) 1 }} nginx/{{- .InstanceID -}}: +{{- else }} + nginx: +{{- end}} + instance_id: "{{- .InstanceID -}}" api_details: url: "{{- .StubStatus.URL -}}" listen: "{{- .StubStatus.Listen -}}" @@ -98,12 +103,17 @@ receivers: {{- end }} {{- range .Receivers.NginxPlusReceivers }} +{{- if gt (len $.Receivers.NginxPlusReceivers) 1 }} nginxplus/{{- .InstanceID -}}: +{{- else }} + nginxplus: +{{- end}} + instance_id: "{{- .InstanceID -}}" api_details: - url: "{{- .PlusAPI.URL -}}" - listen: "{{- .PlusAPI.Listen -}}" - location: "{{- .PlusAPI.Location -}}" - ca: "{{- .PlusAPI.Ca -}}" + url: "{{- .PlusAPI.URL -}}" + listen: "{{- .PlusAPI.Listen -}}" + location: "{{- .PlusAPI.Location -}}" + ca: "{{- .PlusAPI.Ca -}}" {{- if .CollectionInterval }} collection_interval: {{ .CollectionInterval }} {{- end }} @@ -274,10 +284,18 @@ service: {{- end }} {{- else if eq $receiver "nginx_metrics" }} {{- range $.Receivers.NginxReceivers }} + {{- if gt (len $.Receivers.NginxReceivers) 1 }} - nginx/{{- .InstanceID -}} + {{- else }} + - nginx + {{- end }} {{- end }} {{- range $.Receivers.NginxPlusReceivers }} + {{- if gt (len $.Receivers.NginxReceivers) 1 }} - nginxplus/{{- .InstanceID -}} + {{- else }} + - nginxplus + {{- end }} {{- end }} {{- else }} - {{ $receiver }} diff --git a/internal/collector/settings.go b/internal/collector/settings.go index e89bb89d6..11a7fc8a4 100644 --- a/internal/collector/settings.go +++ b/internal/collector/settings.go @@ -71,7 +71,11 @@ func createConverterFactories() []confmap.ConverterFactory { } func createURIs(cfg *config.Config) []string { - return []string{cfg.Collector.ConfigPath} + configFiles := []string{cfg.Collector.ConfigPath} + configFiles = slices.Concat(configFiles, cfg.Collector.AdditionalPaths) + slog.Info("Additional config files:", "", configFiles) + + return configFiles } func createFile(confPath string) error { diff --git a/internal/collector/settings_test.go b/internal/collector/settings_test.go index 0d7b5ce34..d152d7bfe 100644 --- a/internal/collector/settings_test.go +++ b/internal/collector/settings_test.go @@ -7,6 +7,7 @@ package collector import ( "os" "path/filepath" + "slices" "testing" "time" @@ -107,6 +108,28 @@ func TestTemplateWrite(t *testing.T) { }, }, }) + + cfg.Collector.Receivers.NginxPlusReceivers = slices.Concat(cfg.Collector.Receivers.NginxPlusReceivers, + []config.NginxPlusReceiver{ + { + InstanceID: "456", + PlusAPI: config.APIDetails{ + URL: "http://localhost:80/api", + Listen: "", + Location: "", + }, + CollectionInterval: 20 * time.Second, + }, + { + InstanceID: "789", + PlusAPI: config.APIDetails{ + URL: "http://localhost:90/api", + Listen: "", + Location: "", + }, + CollectionInterval: 20 * time.Second, + }, + }) // Clear default config and test collector with TLS enabled cfg.Collector.Receivers.OtlpReceivers["default"] = &config.OtlpReceiver{ Server: &config.ServerConfig{ @@ -165,7 +188,10 @@ func TestTemplateWrite(t *testing.T) { cfg.Collector.Pipelines.Metrics = make(map[string]*config.Pipeline) cfg.Collector.Pipelines.Metrics["default"] = &config.Pipeline{ - Receivers: []string{"hostmetrics", "containermetrics", "otlp/default", "nginx/123"}, + Receivers: []string{ + "hostmetrics", "containermetrics", + "otlp/default", "nginx", "nginxplus/456", "nginxplus/789", + }, Processors: []string{"resource/default", "batch/default"}, Exporters: []string{"otlp/default", "prometheus", "debug"}, } diff --git a/internal/config/config.go b/internal/config/config.go index 619a0a1fd..5c1a0a0f1 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -1061,14 +1061,14 @@ func resolveCollector(allowedDirs []string) (*Collector, error) { } col := &Collector{ - ConfigPath: viperInstance.GetString(CollectorConfigPathKey), + ConfigPath: viperInstance.GetString(CollectorConfigPathKey), AdditionalPaths: viperInstance.GetStringSlice(CollectorAdditionalConfigPathsKey), - Exporters: exporters, - Processors: resolveProcessors(), - Receivers: receivers, - Extensions: resolveExtensions(), - Log: resolveCollectorLog(), - Pipelines: resolvePipelines(), + Exporters: exporters, + Processors: resolveProcessors(), + Receivers: receivers, + Extensions: resolveExtensions(), + Log: resolveCollectorLog(), + Pipelines: resolvePipelines(), } // Check for self-signed certificate true in Agent conf diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 4c4ba1278..6bb94633c 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -1111,7 +1111,7 @@ func createConfig() *Config { }, }, Collector: &Collector{ - ConfigPath: "/etc/nginx-agent/nginx-agent-otelcol.yaml", + ConfigPath: "/etc/nginx-agent/nginx-agent-otelcol.yaml", AdditionalPaths: []string{"/configs/my_config.yaml", "/etc/nginx-agent/nginx-agent-otelcol.yaml"}, Exporters: Exporters{ OtlpExporters: map[string]*OtlpExporter{ diff --git a/internal/config/defaults.go b/internal/config/defaults.go index 7174ac1ca..615c7bc8b 100644 --- a/internal/config/defaults.go +++ b/internal/config/defaults.go @@ -85,13 +85,13 @@ const ( DefFileWatcherMonitoringFrequency = 5 * time.Second // Collector defaults - DefCollectorConfigPath = "/etc/nginx-agent/opentelemetry-collector-agent.yaml" - DefCollectorLogLevel = "INFO" - DefCollectorLogPath = "/var/log/nginx-agent/opentelemetry-collector-agent.log" - DefCollectorTLSCertPath = "/var/lib/nginx-agent/cert.pem" - DefCollectorTLSKeyPath = "/var/lib/nginx-agent/key.pem" - DefCollectorTLSCAPath = "/var/lib/nginx-agent/ca.pem" - DefCollectorTLSSANNames = "127.0.0.1,::1,localhost" + DefCollectorConfigPath = "/etc/nginx-agent/opentelemetry-collector-agent.yaml" + DefCollectorLogLevel = "INFO" + DefCollectorLogPath = "/var/log/nginx-agent/opentelemetry-collector-agent.log" + DefCollectorTLSCertPath = "/var/lib/nginx-agent/cert.pem" + DefCollectorTLSKeyPath = "/var/lib/nginx-agent/key.pem" + DefCollectorTLSCAPath = "/var/lib/nginx-agent/ca.pem" + DefCollectorTLSSANNames = "127.0.0.1,::1,localhost" DefCollectorMetricsBatchProcessorSendBatchSize = 1000 DefCollectorMetricsBatchProcessorSendBatchMaxSize = 1000 diff --git a/internal/config/types.go b/internal/config/types.go index 63fc976c9..59bc4b029 100644 --- a/internal/config/types.go +++ b/internal/config/types.go @@ -105,14 +105,14 @@ type ( } Collector struct { - ConfigPath string `yaml:"config_path" mapstructure:"config_path"` - AdditionalPaths []string `yaml:"additional_config_paths" mapstructure:"additional_config_paths"` - Log *Log `yaml:"log" mapstructure:"log"` - Exporters Exporters `yaml:"exporters" mapstructure:"exporters"` - Extensions Extensions `yaml:"extensions" mapstructure:"extensions"` - Processors Processors `yaml:"processors" mapstructure:"processors"` - Pipelines Pipelines `yaml:"pipelines" mapstructure:"pipelines"` - Receivers Receivers `yaml:"receivers" mapstructure:"receivers"` + ConfigPath string `yaml:"config_path" mapstructure:"config_path"` + AdditionalPaths []string `yaml:"additional_config_paths" mapstructure:"additional_config_paths"` + Log *Log `yaml:"log" mapstructure:"log"` + Exporters Exporters `yaml:"exporters" mapstructure:"exporters"` + Extensions Extensions `yaml:"extensions" mapstructure:"extensions"` + Processors Processors `yaml:"processors" mapstructure:"processors"` + Pipelines Pipelines `yaml:"pipelines" mapstructure:"pipelines"` + Receivers Receivers `yaml:"receivers" mapstructure:"receivers"` } Pipelines struct { diff --git a/test/config/collector/test-opentelemetry-collector-agent.yaml b/test/config/collector/test-opentelemetry-collector-agent.yaml index aa72f4a4b..5cb549bfb 100644 --- a/test/config/collector/test-opentelemetry-collector-agent.yaml +++ b/test/config/collector/test-opentelemetry-collector-agent.yaml @@ -26,7 +26,8 @@ receivers: cert_file: /tmp/cert.pem key_file: /tmp/key.pem ca_file: /tmp/ca.pem - nginx/123: + nginx: + instance_id: "123" api_details: url: "http://localhost:80/status" listen: "" @@ -36,6 +37,22 @@ receivers: access_logs: - log_format: "$remote_addr - $remote_user [$time_local] \"$request\" $status $body_bytes_sent \"$http_referer\" \"$http_user_agent\" \"$http_x_forwarded_for\"\"$upstream_cache_status\"" file_path: "/var/log/nginx/access-custom.conf" + nginxplus/456: + instance_id: "456" + api_details: + url: "http://localhost:80/api" + listen: "" + location: "" + ca: "" + collection_interval: 20s + nginxplus/789: + instance_id: "789" + api_details: + url: "http://localhost:90/api" + listen: "" + location: "" + ca: "" + collection_interval: 20s tcplog/default: listen_address: "localhost:151" operators: @@ -108,7 +125,9 @@ service: - hostmetrics - containermetrics - otlp/default - - nginx/123 + - nginx + - nginxplus/456 + - nginxplus/789 processors: - resource/default - batch/default From 4206ac17b7953911c5729a2ce9874a93a9cd4f95 Mon Sep 17 00:00:00 2001 From: Aphral Griffin Date: Wed, 1 Oct 2025 17:34:44 +0100 Subject: [PATCH 03/10] embedded otel collector --- internal/config/types.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/internal/config/types.go b/internal/config/types.go index 59bc4b029..b599de9cd 100644 --- a/internal/config/types.go +++ b/internal/config/types.go @@ -365,6 +365,14 @@ func (col *Collector) Validate(allowedDirectories []string) error { err = errors.Join(err, nginxReceiver.Validate(allowedDirectories)) } + for _, path := range col.AdditionalPaths { + cleanPath := filepath.Clean(path) + pathAllowed := isAllowedDir(cleanPath, allowedDirectories) + if !pathAllowed { + err = errors.Join(err, fmt.Errorf("additional config path %s not in allowed directories", path)) + } + } + return err } From ee804c8cf0dfc89fee19b23287e8639ca4e6c812 Mon Sep 17 00:00:00 2001 From: Aphral Griffin Date: Thu, 2 Oct 2025 10:01:48 +0100 Subject: [PATCH 04/10] fix test --- internal/config/config_test.go | 3 ++- internal/config/testdata/nginx-agent.conf | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 6bb94633c..59d602aaa 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -64,6 +64,7 @@ func TestResolveConfig(t *testing.T) { actual, err := ResolveConfig() require.NoError(t, err) + t.Logf("Actual: %+v", actual.AllowedDirectories) sort.Slice(actual.Collector.Extensions.HeadersSetter.Headers, func(i, j int) bool { headers := actual.Collector.Extensions.HeadersSetter.Headers return headers[i].Key < headers[j].Key @@ -1094,7 +1095,7 @@ func createConfig() *Config { }, AllowedDirectories: []string{ "/etc/nginx-agent", "/etc/nginx", "/usr/local/etc/nginx", "/var/run/nginx", - "/usr/share/nginx/modules", "/var/log/nginx", + "/usr/share/nginx/modules", "/var/log/nginx", "/configs", }, DataPlaneConfig: &DataPlaneConfig{ Nginx: &NginxDataPlaneConfig{ diff --git a/internal/config/testdata/nginx-agent.conf b/internal/config/testdata/nginx-agent.conf index 6e247a585..f3bd26207 100644 --- a/internal/config/testdata/nginx-agent.conf +++ b/internal/config/testdata/nginx-agent.conf @@ -63,6 +63,7 @@ allowed_directories: - /var/run/nginx - /usr/share/nginx/modules - /var/log/nginx + - /configs command: server: From 51c5847dcbc424ed1c340770d80c49532f2c93dc Mon Sep 17 00:00:00 2001 From: Aphral Griffin Date: Tue, 14 Oct 2025 12:01:51 +0100 Subject: [PATCH 05/10] PR feedback --- api/grpc/mpi/v1/command.pb.go | 2 +- api/grpc/mpi/v1/common.pb.go | 2 +- api/grpc/mpi/v1/files.pb.go | 2 +- internal/collector/otel_collector_plugin.go | 5 +++-- internal/collector/settings.go | 4 ++-- internal/config/config.go | 16 ++++++++-------- internal/config/config_test.go | 4 ++-- internal/config/types.go | 18 +++++++++--------- 8 files changed, 27 insertions(+), 26 deletions(-) diff --git a/api/grpc/mpi/v1/command.pb.go b/api/grpc/mpi/v1/command.pb.go index b5f0346f4..31e94ea7d 100644 --- a/api/grpc/mpi/v1/command.pb.go +++ b/api/grpc/mpi/v1/command.pb.go @@ -8,7 +8,7 @@ // Code generated by protoc-gen-go. DO NOT EDIT. // versions: -// protoc-gen-go v1.36.9 +// protoc-gen-go v1.36.10 // protoc (unknown) // source: mpi/v1/command.proto diff --git a/api/grpc/mpi/v1/common.pb.go b/api/grpc/mpi/v1/common.pb.go index e6d06cf37..b59f47b5a 100644 --- a/api/grpc/mpi/v1/common.pb.go +++ b/api/grpc/mpi/v1/common.pb.go @@ -5,7 +5,7 @@ // Code generated by protoc-gen-go. DO NOT EDIT. // versions: -// protoc-gen-go v1.36.9 +// protoc-gen-go v1.36.10 // protoc (unknown) // source: mpi/v1/common.proto diff --git a/api/grpc/mpi/v1/files.pb.go b/api/grpc/mpi/v1/files.pb.go index 47d1362b7..0223bae3a 100644 --- a/api/grpc/mpi/v1/files.pb.go +++ b/api/grpc/mpi/v1/files.pb.go @@ -5,7 +5,7 @@ // Code generated by protoc-gen-go. DO NOT EDIT. // versions: -// protoc-gen-go v1.36.9 +// protoc-gen-go v1.36.10 // protoc (unknown) // source: mpi/v1/files.proto diff --git a/internal/collector/otel_collector_plugin.go b/internal/collector/otel_collector_plugin.go index 9daad0ccd..fff0e4968 100644 --- a/internal/collector/otel_collector_plugin.go +++ b/internal/collector/otel_collector_plugin.go @@ -45,6 +45,7 @@ const ( `? (let utcTime = ` + `date(timestamp).UTC(); utcTime.Format("Jan 2 15:04:05")) : date(timestamp).Format("Jan 02 15:04:05"); ` + `split(body, ">")[0] + ">" + newTimestamp + " " + split(body, " ", 2)[1])'` + debugOTelConfigPath = "/var/lib/nginx-agent/opentelemetry-collector-agent-debug.yaml" ) type ( @@ -388,7 +389,7 @@ func (oc *Collector) updateHeadersSetterExtension( func (oc *Collector) writeRunningConfig(ctx context.Context, settings otelcol.CollectorSettings) error { slog.DebugContext(ctx, "Writing running OTel collector config", "path", - "/var/lib/nginx-agent/opentelemetry-collector-agent-debug.yaml") + debugOTelConfigPath) resolver, err := confmap.NewResolver(settings.ConfigProviderSettings.ResolverSettings) if err != nil { return fmt.Errorf("unable to create resolver: %w", err) @@ -403,7 +404,7 @@ func (oc *Collector) writeRunningConfig(ctx context.Context, settings otelcol.Co return fmt.Errorf("error while marshaling to YAML: %w", err) } - writeErr := os.WriteFile("/var/lib/nginx-agent/opentelemetry-collector-agent-debug.yaml", b, filePermission) + writeErr := os.WriteFile(debugOTelConfigPath, b, filePermission) if writeErr != nil { return fmt.Errorf("error while writing debug config: %w", err) } diff --git a/internal/collector/settings.go b/internal/collector/settings.go index 11a7fc8a4..3346a0df9 100644 --- a/internal/collector/settings.go +++ b/internal/collector/settings.go @@ -72,8 +72,8 @@ func createConverterFactories() []confmap.ConverterFactory { func createURIs(cfg *config.Config) []string { configFiles := []string{cfg.Collector.ConfigPath} - configFiles = slices.Concat(configFiles, cfg.Collector.AdditionalPaths) - slog.Info("Additional config files:", "", configFiles) + configFiles = slices.Concat(configFiles, cfg.Collector.AdditionalConfigPaths) + slog.Info("Merging additional OTel config files", "config_files", configFiles) return configFiles } diff --git a/internal/config/config.go b/internal/config/config.go index 5c1a0a0f1..7e16f4182 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -1061,14 +1061,14 @@ func resolveCollector(allowedDirs []string) (*Collector, error) { } col := &Collector{ - ConfigPath: viperInstance.GetString(CollectorConfigPathKey), - AdditionalPaths: viperInstance.GetStringSlice(CollectorAdditionalConfigPathsKey), - Exporters: exporters, - Processors: resolveProcessors(), - Receivers: receivers, - Extensions: resolveExtensions(), - Log: resolveCollectorLog(), - Pipelines: resolvePipelines(), + ConfigPath: viperInstance.GetString(CollectorConfigPathKey), + AdditionalConfigPaths: viperInstance.GetStringSlice(CollectorAdditionalConfigPathsKey), + Exporters: exporters, + Processors: resolveProcessors(), + Receivers: receivers, + Extensions: resolveExtensions(), + Log: resolveCollectorLog(), + Pipelines: resolvePipelines(), } // Check for self-signed certificate true in Agent conf diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 59d602aaa..5d040cc71 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -1112,8 +1112,8 @@ func createConfig() *Config { }, }, Collector: &Collector{ - ConfigPath: "/etc/nginx-agent/nginx-agent-otelcol.yaml", - AdditionalPaths: []string{"/configs/my_config.yaml", "/etc/nginx-agent/nginx-agent-otelcol.yaml"}, + ConfigPath: "/etc/nginx-agent/nginx-agent-otelcol.yaml", + AdditionalConfigPaths: []string{"/configs/my_config.yaml", "/etc/nginx-agent/nginx-agent-otelcol.yaml"}, Exporters: Exporters{ OtlpExporters: map[string]*OtlpExporter{ "default": { diff --git a/internal/config/types.go b/internal/config/types.go index b599de9cd..e77e8ee81 100644 --- a/internal/config/types.go +++ b/internal/config/types.go @@ -105,14 +105,14 @@ type ( } Collector struct { - ConfigPath string `yaml:"config_path" mapstructure:"config_path"` - AdditionalPaths []string `yaml:"additional_config_paths" mapstructure:"additional_config_paths"` - Log *Log `yaml:"log" mapstructure:"log"` - Exporters Exporters `yaml:"exporters" mapstructure:"exporters"` - Extensions Extensions `yaml:"extensions" mapstructure:"extensions"` - Processors Processors `yaml:"processors" mapstructure:"processors"` - Pipelines Pipelines `yaml:"pipelines" mapstructure:"pipelines"` - Receivers Receivers `yaml:"receivers" mapstructure:"receivers"` + ConfigPath string `yaml:"config_path" mapstructure:"config_path"` + AdditionalConfigPaths []string `yaml:"additional_config_paths" mapstructure:"additional_config_paths"` + Log *Log `yaml:"log" mapstructure:"log"` + Exporters Exporters `yaml:"exporters" mapstructure:"exporters"` + Extensions Extensions `yaml:"extensions" mapstructure:"extensions"` + Processors Processors `yaml:"processors" mapstructure:"processors"` + Pipelines Pipelines `yaml:"pipelines" mapstructure:"pipelines"` + Receivers Receivers `yaml:"receivers" mapstructure:"receivers"` } Pipelines struct { @@ -365,7 +365,7 @@ func (col *Collector) Validate(allowedDirectories []string) error { err = errors.Join(err, nginxReceiver.Validate(allowedDirectories)) } - for _, path := range col.AdditionalPaths { + for _, path := range col.AdditionalConfigPaths { cleanPath := filepath.Clean(path) pathAllowed := isAllowedDir(cleanPath, allowedDirectories) if !pathAllowed { From b9f0e8f0d2d8defd622e80e2ab06dd6889e47d7d Mon Sep 17 00:00:00 2001 From: Aphral Griffin Date: Wed, 15 Oct 2025 11:58:55 +0100 Subject: [PATCH 06/10] unit test --- internal/collector/otel_collector_plugin.go | 9 ++- .../collector/otel_collector_plugin_test.go | 74 +++++++++++++++++++ .../testdata/custom_otel_config.yaml | 15 ++++ internal/collector/testdata/merge_config.yaml | 70 ++++++++++++++++++ internal/collector/testdata/otel_config.yaml | 64 ++++++++++++++++ 5 files changed, 229 insertions(+), 3 deletions(-) create mode 100644 internal/collector/testdata/custom_otel_config.yaml create mode 100644 internal/collector/testdata/merge_config.yaml create mode 100644 internal/collector/testdata/otel_config.yaml diff --git a/internal/collector/otel_collector_plugin.go b/internal/collector/otel_collector_plugin.go index fff0e4968..aa22eb4bc 100644 --- a/internal/collector/otel_collector_plugin.go +++ b/internal/collector/otel_collector_plugin.go @@ -2,6 +2,7 @@ // // This source code is licensed under the Apache License, Version 2.0 license found in the // LICENSE file in the root directory of this source tree. + package collector import ( @@ -56,6 +57,7 @@ type ( mu *sync.Mutex cancel context.CancelFunc previousNAPSysLogServer string + debugOTelConfigPath string stopped bool } ) @@ -97,6 +99,7 @@ func NewCollector(conf *config.Config) (*Collector, error) { stopped: true, mu: &sync.Mutex{}, previousNAPSysLogServer: "", + debugOTelConfigPath: debugOTelConfigPath, }, nil } @@ -389,7 +392,7 @@ func (oc *Collector) updateHeadersSetterExtension( func (oc *Collector) writeRunningConfig(ctx context.Context, settings otelcol.CollectorSettings) error { slog.DebugContext(ctx, "Writing running OTel collector config", "path", - debugOTelConfigPath) + oc.debugOTelConfigPath) resolver, err := confmap.NewResolver(settings.ConfigProviderSettings.ResolverSettings) if err != nil { return fmt.Errorf("unable to create resolver: %w", err) @@ -404,9 +407,9 @@ func (oc *Collector) writeRunningConfig(ctx context.Context, settings otelcol.Co return fmt.Errorf("error while marshaling to YAML: %w", err) } - writeErr := os.WriteFile(debugOTelConfigPath, b, filePermission) + writeErr := os.WriteFile(oc.debugOTelConfigPath, b, filePermission) if writeErr != nil { - return fmt.Errorf("error while writing debug config: %w", err) + return fmt.Errorf("error while writing debug config: %w", writeErr) } return nil diff --git a/internal/collector/otel_collector_plugin_test.go b/internal/collector/otel_collector_plugin_test.go index ac25ce775..21e5f4d5b 100644 --- a/internal/collector/otel_collector_plugin_test.go +++ b/internal/collector/otel_collector_plugin_test.go @@ -18,6 +18,13 @@ import ( "github.com/nginx/agent/v3/test/stub" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "go.opentelemetry.io/collector/confmap" + "go.opentelemetry.io/collector/confmap/provider/envprovider" + "go.opentelemetry.io/collector/confmap/provider/fileprovider" + "go.opentelemetry.io/collector/confmap/provider/httpprovider" + "go.opentelemetry.io/collector/confmap/provider/httpsprovider" + "go.opentelemetry.io/collector/confmap/provider/yamlprovider" + "go.opentelemetry.io/collector/otelcol" "github.com/nginx/agent/v3/internal/bus" @@ -994,6 +1001,73 @@ func TestCollector_findAvailableSyslogServers(t *testing.T) { } } +func TestCollector_writeRunningConfig(t *testing.T) { + tempDir := t.TempDir() + + tests := []struct { + name string + writeConfigErr error + settings otelcol.CollectorSettings + }{ + { + name: "Test 1: Write Config Success", + settings: otelcol.CollectorSettings{ + ConfigProviderSettings: otelcol.ConfigProviderSettings{ + ResolverSettings: confmap.ResolverSettings{ + URIs: []string{"./testdata/otel_config.yaml", "./testdata/custom_otel_config.yaml"}, + ProviderFactories: []confmap.ProviderFactory{ + envprovider.NewFactory(), + fileprovider.NewFactory(), + httpprovider.NewFactory(), + httpsprovider.NewFactory(), + yamlprovider.NewFactory(), + }, + DefaultScheme: "", + ProviderSettings: confmap.ProviderSettings{}, + ConverterFactories: nil, + ConverterSettings: confmap.ConverterSettings{}, + }, + }, + }, + writeConfigErr: nil, + }, + { + name: "Test 2: Write Config Failed", + settings: otelcol.CollectorSettings{ + ConfigProviderSettings: otelcol.ConfigProviderSettings{ + ResolverSettings: confmap.ResolverSettings{}, + }, + }, + writeConfigErr: errors.New("unable to create resolver: invalid 'confmap.ResolverSettings' configuration: no URIs"), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + conf := types.OTelConfig(t) + conf.Collector.Log.Path = filepath.Join(tempDir, "otel-collector-test.log") + newCollector, err := NewCollector(conf) + newCollector.debugOTelConfigPath = filepath.Join(tempDir, "otel-collector-debug-config.yaml") + require.NoError(t, err) + + writeErr := newCollector.writeRunningConfig(context.Background(), tt.settings) + + if tt.writeConfigErr == nil { + actual, readErr := os.ReadFile(newCollector.debugOTelConfigPath) + require.NoError(t, readErr) + + expected, expectedFileErr := os.ReadFile("./testdata/merge_config.yaml") + require.NoError(t, expectedFileErr) + + assert.Equal(t, string(expected), string(actual)) + } else { + assert.Equal(t, tt.writeConfigErr.Error(), writeErr.Error()) + } + + }) + } +} + func createFakeCollector() *typesfakes.FakeCollectorInterface { fakeCollector := &typesfakes.FakeCollectorInterface{} fakeCollector.RunStub = func(ctx context.Context) error { return nil } diff --git a/internal/collector/testdata/custom_otel_config.yaml b/internal/collector/testdata/custom_otel_config.yaml new file mode 100644 index 000000000..d061c7b93 --- /dev/null +++ b/internal/collector/testdata/custom_otel_config.yaml @@ -0,0 +1,15 @@ +exporters: + prometheus: + endpoint: "127.0.0.1:5643" + resource_to_telemetry_conversion: + enabled: true + +service: + pipelines: + metrics/prometheus-example-pipeline: + receivers: + - nginxplus + processors: + - resource/default + exporters: + - prometheus diff --git a/internal/collector/testdata/merge_config.yaml b/internal/collector/testdata/merge_config.yaml new file mode 100644 index 000000000..54cc87b5d --- /dev/null +++ b/internal/collector/testdata/merge_config.yaml @@ -0,0 +1,70 @@ +exporters: + otlp/default: + endpoint: 127.0.0.1:9091 + retry_on_failure: + enabled: true + initial_interval: 10s + max_elapsed_time: 10m + max_interval: 60s + timeout: 10s + tls: + insecure: true + prometheus: + endpoint: 127.0.0.1:5643 + resource_to_telemetry_conversion: + enabled: true +extensions: null +processors: + batch/default_logs: + send_batch_max_size: 100 + send_batch_size: 100 + timeout: 1m0s + batch/default_metrics: + send_batch_max_size: 1000 + send_batch_size: 1000 + timeout: 30s + logsgzip/default: {} + resource/default: + attributes: + - action: insert + key: resource.id + value: f16dedd7-929f-3642-b823-4ac8d295cc23 +receivers: + hostmetrics: + collection_interval: 1m0s + initial_delay: 1s + scrapers: + cpu: + metrics: + system.cpu.logical.count: + enabled: true + system.cpu.utilization: + enabled: true + disk: null + filesystem: null + memory: + metrics: + system.memory.limit: + enabled: true + network: null +service: + extensions: null + pipelines: + metrics/default: + exporters: + - otlp/default + processors: + - batch/default_metrics + - resource/default + receivers: + - hostmetrics + metrics/prometheus-example-pipeline: + exporters: + - prometheus + processors: + - resource/default + receivers: + - nginxplus + telemetry: + metrics: + level: none diff --git a/internal/collector/testdata/otel_config.yaml b/internal/collector/testdata/otel_config.yaml new file mode 100644 index 000000000..298781551 --- /dev/null +++ b/internal/collector/testdata/otel_config.yaml @@ -0,0 +1,64 @@ +receivers: + hostmetrics: + collection_interval: 1m0s + initial_delay: 1s + scrapers: + cpu: + metrics: + system.cpu.utilization: + enabled: true + system.cpu.logical.count: + enabled: true + disk: + filesystem: + memory: + metrics: + system.memory.limit: + enabled: true + network: + +processors: + resource/default: + attributes: + - key: resource.id + action: insert + value: f16dedd7-929f-3642-b823-4ac8d295cc23 + batch/default_logs: + send_batch_size: 100 + timeout: 1m0s + send_batch_max_size: 100 + batch/default_metrics: + send_batch_size: 1000 + timeout: 30s + send_batch_max_size: 1000 + + logsgzip/default: {} + +exporters: + otlp/default: + endpoint: "127.0.0.1:9091" + timeout: 10s + retry_on_failure: + enabled: true + initial_interval: 10s + max_interval: 60s + max_elapsed_time: 10m + tls: + insecure: true +extensions: + +service: + telemetry: + metrics: + level: none + extensions: + + pipelines: + metrics/default: + receivers: + - hostmetrics + processors: + - batch/default_metrics + - resource/default + exporters: + - otlp/default From 967f8f07494a98a78254fc155814c022ecdab0a6 Mon Sep 17 00:00:00 2001 From: Aphral Griffin Date: Wed, 15 Oct 2025 12:03:35 +0100 Subject: [PATCH 07/10] unit test --- internal/collector/otel_collector_plugin_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/collector/otel_collector_plugin_test.go b/internal/collector/otel_collector_plugin_test.go index 21e5f4d5b..88edeb088 100644 --- a/internal/collector/otel_collector_plugin_test.go +++ b/internal/collector/otel_collector_plugin_test.go @@ -1038,7 +1038,8 @@ func TestCollector_writeRunningConfig(t *testing.T) { ResolverSettings: confmap.ResolverSettings{}, }, }, - writeConfigErr: errors.New("unable to create resolver: invalid 'confmap.ResolverSettings' configuration: no URIs"), + writeConfigErr: errors.New("unable to create resolver: invalid " + + "'confmap.ResolverSettings' configuration: no URIs"), }, } @@ -1063,7 +1064,6 @@ func TestCollector_writeRunningConfig(t *testing.T) { } else { assert.Equal(t, tt.writeConfigErr.Error(), writeErr.Error()) } - }) } } From b78147020d27cc8b07e98329f68c2b057904514a Mon Sep 17 00:00:00 2001 From: Aphral Griffin Date: Wed, 15 Oct 2025 13:11:04 +0100 Subject: [PATCH 08/10] update code coverage settings --- .codecov.yml | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/.codecov.yml b/.codecov.yml index 3421a089c..6e2915076 100644 --- a/.codecov.yml +++ b/.codecov.yml @@ -22,10 +22,9 @@ coverage: # Patch-level coverage settings patch: - default: - - target: 80% + informational: true + target: auto threshold: 0% only_pulls: false From 977601b3b9b980411534524e94da6e9587ba2ce4 Mon Sep 17 00:00:00 2001 From: Aphral Griffin Date: Thu, 16 Oct 2025 15:54:09 +0100 Subject: [PATCH 09/10] fix pipeline --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index b21cbe935..2a270ad8d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -20,7 +20,7 @@ permissions: env: NFPM_VERSION: 'v2.35.3' - GOPROXY: "direct" + GOPROXY: "https://${{ secrets.ARTIFACTORY_USER }}:${{ secrets.ARTIFACTORY_TOKEN }}@azr.artifactory.f5net.com/artifactory/api/go/f5-nginx-go-dev" jobs: proxy-sanity-check: From c6391a8549086076446252f1690c63ddb7ab024d Mon Sep 17 00:00:00 2001 From: Aphral Griffin Date: Wed, 22 Oct 2025 14:15:59 +0100 Subject: [PATCH 10/10] update debug otel config path --- internal/collector/otel_collector_plugin.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/internal/collector/otel_collector_plugin.go b/internal/collector/otel_collector_plugin.go index aa22eb4bc..b57696a5f 100644 --- a/internal/collector/otel_collector_plugin.go +++ b/internal/collector/otel_collector_plugin.go @@ -46,7 +46,7 @@ const ( `? (let utcTime = ` + `date(timestamp).UTC(); utcTime.Format("Jan 2 15:04:05")) : date(timestamp).Format("Jan 02 15:04:05"); ` + `split(body, ">")[0] + ">" + newTimestamp + " " + split(body, " ", 2)[1])'` - debugOTelConfigPath = "/var/lib/nginx-agent/opentelemetry-collector-agent-debug.yaml" + debugOTelConfigFile = "/opentelemetry-collector-agent-debug.yaml" ) type ( @@ -93,6 +93,8 @@ func NewCollector(conf *config.Config) (*Collector, error) { return nil, err } + debugOTelConfigPath := conf.LibDir + debugOTelConfigFile + return &Collector{ config: conf, service: oTelCollector,