From 8b0907048855d81a338f03d177b827ff31089c18 Mon Sep 17 00:00:00 2001 From: Guillaume Pagnoux Date: Fri, 11 Oct 2024 14:07:10 +0200 Subject: [PATCH 01/18] discovery: add rails service name detector --- .../servicediscovery/module/impl_linux.go | 6 ++ .../corechecks/servicediscovery/usm/ruby.go | 90 +++++++++++++++++++ .../servicediscovery/usm/service.go | 5 +- 3 files changed, 100 insertions(+), 1 deletion(-) create mode 100644 pkg/collector/corechecks/servicediscovery/usm/ruby.go diff --git a/pkg/collector/corechecks/servicediscovery/module/impl_linux.go b/pkg/collector/corechecks/servicediscovery/module/impl_linux.go index 9646bb4d31f28..8f7018a067d94 100644 --- a/pkg/collector/corechecks/servicediscovery/module/impl_linux.go +++ b/pkg/collector/corechecks/servicediscovery/module/impl_linux.go @@ -342,12 +342,18 @@ func (s *discovery) getServiceInfo(proc *process.Process) (*serviceInfo, error) return nil, err } + cwd, err := proc.Cwd() + if err != nil { + return nil, err + } + createTime, err := proc.CreateTime() if err != nil { return nil, err } contextMap := make(usm.DetectorContextMap) + contextMap[usm.ServiceCwd] = cwd root := kernel.HostProc(strconv.Itoa(int(proc.Pid)), "root") lang := language.FindInArgs(exe, cmdline) diff --git a/pkg/collector/corechecks/servicediscovery/usm/ruby.go b/pkg/collector/corechecks/servicediscovery/usm/ruby.go new file mode 100644 index 0000000000000..39c843927598f --- /dev/null +++ b/pkg/collector/corechecks/servicediscovery/usm/ruby.go @@ -0,0 +1,90 @@ +// Unless explicitly stated otherwise all files in this repository are licensed +// under the Apache License Version 2.0. +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// Copyright 2016-present Datadog, Inc. + +package usm + +import ( + "io" + "io/fs" + "regexp" + "strings" + + "github.com/DataDog/datadog-agent/pkg/util/log" +) + +var ( + moduleRegexp = regexp.MustCompile(`module\s+([A-Z][a-zA-Z0-9_]*)`) + matchFirstCap = regexp.MustCompile("(.)([A-Z][a-z]+)") + matchAllCap = regexp.MustCompile("([a-z0-9])([A-Z])") +) + +type railsDetector struct { + ctx DetectionContext +} + +func newRailsDetector(ctx DetectionContext) detector { + return &railsDetector{ctx} +} + +// detect checks if the service is a Rails application by looking for a +// `config/application.rb` file generated by `rails new` when a new rails +// project is created. This file should contain a `module` declaration with the +// application name. +func (r railsDetector) detect(_ []string) (ServiceMetadata, bool) { + cwd := r.ctx.contextMap[ServiceCwd].(string) + absFile := abs("config/application.rb", cwd) + if _, err := fs.Stat(r.ctx.fs, absFile); err != nil { + log.Debugf("Could not stat (cwd: %v) %v: %v", cwd, absFile, err) + return ServiceMetadata{}, false + } + + name, ok := r.findRailsApplicationName(absFile) + if !ok { + return ServiceMetadata{}, false + } + + return NewServiceMetadata(railsUnderscore(name)), true +} + +// findRailsApplicationName scans the `config/application.rb` file to find the +// Rails application name. +func (r railsDetector) findRailsApplicationName(filename string) (string, bool) { + log.Debugf("findRailsApplicationName") + file, err := r.ctx.fs.Open(filename) + if err != nil { + log.Debugf("could not open application.rb") + return "", false + } + defer file.Close() + + reader, err := SizeVerifiedReader(file) + if err != nil { + log.Debugf("skipping application.rb (%q): %v", filename, err) + return "", true // stops here + } + + // Scan the file line by line, instead of reading the whole file into memory + bytes, err := io.ReadAll(reader) + if err != nil { + log.Debugf("unable to read application.rb (%q): %v", filename, err) + return "", true + } + + matches := moduleRegexp.FindSubmatch(bytes) + if matches != nil { + return string(matches[1]), true + } + + // No match found + return "", false +} + +// Converts a PascalCasedWord to a snake_cased_word. +// It keeps uppercase acronyms together when converting (e.g. "HTTPServer" -> "http_server"). +func railsUnderscore(pascalCasedWord string) string { + snake := matchFirstCap.ReplaceAllString(pascalCasedWord, "${1}_${2}") + snake = matchAllCap.ReplaceAllString(snake, "${1}_${2}") + return strings.ToLower(snake) +} diff --git a/pkg/collector/corechecks/servicediscovery/usm/service.go b/pkg/collector/corechecks/servicediscovery/usm/service.go index 36ddc7e804d35..964b07af389f0 100644 --- a/pkg/collector/corechecks/servicediscovery/usm/service.go +++ b/pkg/collector/corechecks/servicediscovery/usm/service.go @@ -34,6 +34,8 @@ const ( NodePackageJSONPath = iota // The SubdirFS instance package.json path is valid in. ServiceSubFS = iota + // The working directory of the service + ServiceCwd = iota ) const ( @@ -180,8 +182,9 @@ var languageDetectors = map[language.Language]detectorCreatorFn{ // Map executables that usually have additional process context of what's // running, to context detectors var executableDetectors = map[string]detectorCreatorFn{ - "sudo": newSimpleDetector, "gunicorn": newGunicornDetector, + "puma": newRailsDetector, + "sudo": newSimpleDetector, } func serviceNameInjected(envs map[string]string) bool { From eb3ca59729b45c0c7b7a4a33d6c9905b99261a7f Mon Sep 17 00:00:00 2001 From: Guillaume Pagnoux Date: Mon, 14 Oct 2024 15:03:41 +0200 Subject: [PATCH 02/18] test: e2e: add check for rails service naming --- test/new-e2e/tests/discovery/linux_test.go | 17 +++- .../discovery/testdata/provision/provision.sh | 89 +++++++++++++------ 2 files changed, 76 insertions(+), 30 deletions(-) diff --git a/test/new-e2e/tests/discovery/linux_test.go b/test/new-e2e/tests/discovery/linux_test.go index d48cc30962b7c..9238137beed99 100644 --- a/test/new-e2e/tests/discovery/linux_test.go +++ b/test/new-e2e/tests/discovery/linux_test.go @@ -33,7 +33,13 @@ type linuxTestSuite struct { e2e.BaseSuite[environments.Host] } -var services = []string{"python-svc", "python-instrumented", "node-json-server", "node-instrumented"} +var services = []string{ + "python-svc", + "python-instrumented", + "node-json-server", + "node-instrumented", + "rails-svc", +} func TestLinuxTestSuite(t *testing.T) { agentParams := []func(*agentparams.Params) error{ @@ -125,6 +131,15 @@ func (s *linuxTestSuite) TestServiceDiscoveryCheck() { assert.NotZero(c, found.Payload.RSSMemory) } + found = foundMap["rails_hello"] + if assert.NotNil(c, found) { + assert.Equal(c, "rails_hello", found.Payload.ServiceName) + assert.Equal(c, "rails_hello", found.Payload.GeneratedServiceName) + assert.Empty(c, found.Payload.DDService) + assert.Empty(c, found.Payload.ServiceNameSource) + assert.NotZero(c, found.Payload.RSSMemory) + } + assert.Contains(c, foundMap, "json-server") }, 3*time.Minute, 10*time.Second) } diff --git a/test/new-e2e/tests/discovery/testdata/provision/provision.sh b/test/new-e2e/tests/discovery/testdata/provision/provision.sh index 93bb7a0deeec1..852a21e3ca789 100755 --- a/test/new-e2e/tests/discovery/testdata/provision/provision.sh +++ b/test/new-e2e/tests/discovery/testdata/provision/provision.sh @@ -2,6 +2,47 @@ set -e +install_systemd_unit() { + while [[ $# -ge 2 ]]; do + case $1 in + --workdir) + shift + workdir="WorkingDirectory=$1" + shift + ;; + *) + break + ;; + esac + done + + name=$1 + command=$2 + port=$3 + extraenv=$4 + + cat > "/etc/systemd/system/${name}.service" <<- EOM +[Unit] +Description=${name} +After=network.target +StartLimitIntervalSec=0 + +[Service] +Type=simple +Restart=always +RestartSec=1 +User=root +ExecStart=${command} +Environment="PORT=${port}" +Environment="NODE_VERSION=20" +Environment="${extraenv}" +${workdir} + +[Install] +WantedBy=multi-user.target +EOM +} + apt-get update apt-get install -y \ ca-certificates \ @@ -28,43 +69,33 @@ nvm install 20 || nvm install 20 || nvm install 20 npm install json-server || npm install json-server npm install /home/ubuntu/e2e-test/node/instrumented -# Install our own services -install_systemd_unit () { - name=$1 - command=$2 - port=$3 - extraenv=$4 - - cat > "/etc/systemd/system/${name}.service" <<- EOM -[Unit] -Description=${name} -After=network.target -StartLimitIntervalSec=0 - -[Service] -Type=simple -Restart=always -RestartSec=1 -User=root -ExecStart=${command} -Environment="PORT=${port}" -Environment="NODE_VERSION=20" -Environment="${extraenv}" +# Install Ruby +## Packages +apt-get install -y \ + ruby \ + ruby-dev \ + ruby-rails \ + sqlite3 \ -[Install] -WantedBy=multi-user.target -EOM -} +## Create new Rails project +pushd /home/ubuntu +rails new rails-hello --minimal +bundle install --gemfile=/home/ubuntu/rails-hello/Gemfile +popd -# Node +# Install our own services +## Node install_systemd_unit "node-json-server" "$NVM_DIR/nvm-exec npx json-server --port 8084 /home/ubuntu/e2e-test/node/json-server/db.json" "8084" "" install_systemd_unit "node-instrumented" "$NVM_DIR/nvm-exec node /home/ubuntu/e2e-test/node/instrumented/server.js" "8085" "" -# Python +## Python install_systemd_unit "python-svc" "/usr/bin/python3 /home/ubuntu/e2e-test/python/server.py" "8082" "DD_SERVICE=python-svc-dd" install_systemd_unit "python-instrumented" "/usr/bin/python3 /home/ubuntu/e2e-test/python/instrumented.py" "8083" "" +## Ruby +install_systemd_unit --workdir "/home/ubuntu/rails-hello" "rails-svc" "rails server" "7777" "" + systemctl daemon-reload -# leave them stopped +## leave them stopped systemctl stop python-svc From fbaf76d2270d1b92ed52009ea24591f143b3a0e4 Mon Sep 17 00:00:00 2001 From: Guillaume Pagnoux Date: Fri, 18 Oct 2024 13:06:55 +0200 Subject: [PATCH 03/18] CR: remove debug log --- pkg/collector/corechecks/servicediscovery/usm/ruby.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/collector/corechecks/servicediscovery/usm/ruby.go b/pkg/collector/corechecks/servicediscovery/usm/ruby.go index 39c843927598f..18e17b7d71522 100644 --- a/pkg/collector/corechecks/servicediscovery/usm/ruby.go +++ b/pkg/collector/corechecks/servicediscovery/usm/ruby.go @@ -36,7 +36,6 @@ func (r railsDetector) detect(_ []string) (ServiceMetadata, bool) { cwd := r.ctx.contextMap[ServiceCwd].(string) absFile := abs("config/application.rb", cwd) if _, err := fs.Stat(r.ctx.fs, absFile); err != nil { - log.Debugf("Could not stat (cwd: %v) %v: %v", cwd, absFile, err) return ServiceMetadata{}, false } From e838cefce0d3c2d60e47da6cba3161d2a1468f17 Mon Sep 17 00:00:00 2001 From: Guillaume Pagnoux Date: Fri, 18 Oct 2024 13:07:50 +0200 Subject: [PATCH 04/18] CR: remove outdated comments --- pkg/collector/corechecks/servicediscovery/usm/ruby.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pkg/collector/corechecks/servicediscovery/usm/ruby.go b/pkg/collector/corechecks/servicediscovery/usm/ruby.go index 18e17b7d71522..ee833ad456110 100644 --- a/pkg/collector/corechecks/servicediscovery/usm/ruby.go +++ b/pkg/collector/corechecks/servicediscovery/usm/ruby.go @@ -61,10 +61,9 @@ func (r railsDetector) findRailsApplicationName(filename string) (string, bool) reader, err := SizeVerifiedReader(file) if err != nil { log.Debugf("skipping application.rb (%q): %v", filename, err) - return "", true // stops here + return "", true } - // Scan the file line by line, instead of reading the whole file into memory bytes, err := io.ReadAll(reader) if err != nil { log.Debugf("unable to read application.rb (%q): %v", filename, err) From d6d1609de4462cb8c1b80e95cd76e37169bbdc50 Mon Sep 17 00:00:00 2001 From: Guillaume Pagnoux Date: Fri, 18 Oct 2024 13:37:55 +0200 Subject: [PATCH 05/18] CR: store Process pointer in contextMap --- .../corechecks/servicediscovery/module/impl_linux.go | 7 +------ pkg/collector/corechecks/servicediscovery/usm/ruby.go | 10 +++++++++- .../corechecks/servicediscovery/usm/service.go | 4 ++-- 3 files changed, 12 insertions(+), 9 deletions(-) diff --git a/pkg/collector/corechecks/servicediscovery/module/impl_linux.go b/pkg/collector/corechecks/servicediscovery/module/impl_linux.go index 8f7018a067d94..70935ff79092b 100644 --- a/pkg/collector/corechecks/servicediscovery/module/impl_linux.go +++ b/pkg/collector/corechecks/servicediscovery/module/impl_linux.go @@ -342,18 +342,13 @@ func (s *discovery) getServiceInfo(proc *process.Process) (*serviceInfo, error) return nil, err } - cwd, err := proc.Cwd() - if err != nil { - return nil, err - } - createTime, err := proc.CreateTime() if err != nil { return nil, err } contextMap := make(usm.DetectorContextMap) - contextMap[usm.ServiceCwd] = cwd + contextMap[usm.ServiceProc] = proc root := kernel.HostProc(strconv.Itoa(int(proc.Pid)), "root") lang := language.FindInArgs(exe, cmdline) diff --git a/pkg/collector/corechecks/servicediscovery/usm/ruby.go b/pkg/collector/corechecks/servicediscovery/usm/ruby.go index ee833ad456110..27b0a91e4ed42 100644 --- a/pkg/collector/corechecks/servicediscovery/usm/ruby.go +++ b/pkg/collector/corechecks/servicediscovery/usm/ruby.go @@ -11,6 +11,8 @@ import ( "regexp" "strings" + "github.com/shirou/gopsutil/v3/process" + "github.com/DataDog/datadog-agent/pkg/util/log" ) @@ -33,7 +35,13 @@ func newRailsDetector(ctx DetectionContext) detector { // project is created. This file should contain a `module` declaration with the // application name. func (r railsDetector) detect(_ []string) (ServiceMetadata, bool) { - cwd := r.ctx.contextMap[ServiceCwd].(string) + proc := r.ctx.contextMap[ServiceProc].(*process.Process) + cwd, err := proc.Cwd() + if err != nil { + log.Debugf("could not get cwd of process: %s", err) + return ServiceMetadata{}, false + } + absFile := abs("config/application.rb", cwd) if _, err := fs.Stat(r.ctx.fs, absFile); err != nil { return ServiceMetadata{}, false diff --git a/pkg/collector/corechecks/servicediscovery/usm/service.go b/pkg/collector/corechecks/servicediscovery/usm/service.go index 964b07af389f0..d34603b284292 100644 --- a/pkg/collector/corechecks/servicediscovery/usm/service.go +++ b/pkg/collector/corechecks/servicediscovery/usm/service.go @@ -34,8 +34,8 @@ const ( NodePackageJSONPath = iota // The SubdirFS instance package.json path is valid in. ServiceSubFS = iota - // The working directory of the service - ServiceCwd = iota + // The pointer to the Process instance of the service + ServiceProc = iota ) const ( From 6b7022872212d29f8f0b72cd6a6834d03bc26b7e Mon Sep 17 00:00:00 2001 From: Guillaume Pagnoux Date: Fri, 18 Oct 2024 15:10:46 +0200 Subject: [PATCH 06/18] CR: add unit tests --- .../servicediscovery/usm/ruby_test.go | 48 +++++++++++++++++++ .../usm/testdata/root/testdata/application.rb | 38 +++++++++++++++ .../root/testdata/application_invalid.rb | 2 + 3 files changed, 88 insertions(+) create mode 100644 pkg/collector/corechecks/servicediscovery/usm/ruby_test.go create mode 100644 pkg/collector/corechecks/servicediscovery/usm/testdata/root/testdata/application.rb create mode 100644 pkg/collector/corechecks/servicediscovery/usm/testdata/root/testdata/application_invalid.rb diff --git a/pkg/collector/corechecks/servicediscovery/usm/ruby_test.go b/pkg/collector/corechecks/servicediscovery/usm/ruby_test.go new file mode 100644 index 0000000000000..c7cc732cf1ac8 --- /dev/null +++ b/pkg/collector/corechecks/servicediscovery/usm/ruby_test.go @@ -0,0 +1,48 @@ +// Unless explicitly stated otherwise all files in this repository are licensed +// under the Apache License Version 2.0. +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// Copyright 2016-present Datadog, Inc. + +//go:build !windows + +package usm + +import ( + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestGenerateNameFromRailsApplicationRb(t *testing.T) { + tests := []struct { + name string + path string + expected string + }{ + { + name: "name is found", + path: "./testdata/application.rb", + expected: "rails_hello", + }, + { + name: "name not found", + path: "./testdata/application_invalid.rb", + expected: "", + }, + } + full, err := filepath.Abs("testdata/root") + require.NoError(t, err) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + instance := &railsDetector{ctx: DetectionContext{ + fs: NewSubDirFS(full), + contextMap: make(DetectorContextMap), + }} + value, ok := instance.findRailsApplicationName(tt.path) + assert.Equal(t, len(tt.expected) > 0, ok) + assert.Equal(t, tt.expected, railsUnderscore(value)) + }) + } +} diff --git a/pkg/collector/corechecks/servicediscovery/usm/testdata/root/testdata/application.rb b/pkg/collector/corechecks/servicediscovery/usm/testdata/root/testdata/application.rb new file mode 100644 index 0000000000000..6af9567680208 --- /dev/null +++ b/pkg/collector/corechecks/servicediscovery/usm/testdata/root/testdata/application.rb @@ -0,0 +1,38 @@ +require_relative "boot" + +require "rails" +# Pick the frameworks you want: +require "active_model/railtie" +# require "active_job/railtie" +require "active_record/railtie" +# require "active_storage/engine" +require "action_controller/railtie" +# require "action_mailer/railtie" +# require "action_mailbox/engine" +# require "action_text/engine" +require "action_view/railtie" +# require "action_cable/engine" +require "sprockets/railtie" +require "rails/test_unit/railtie" + +# Require the gems listed in Gemfile, including any gems +# you've limited to :test, :development, or :production. +Bundler.require(*Rails.groups) + +module RailsHello + class Application < Rails::Application + # Initialize configuration defaults for originally generated Rails version. + config.load_defaults 6.1 + + # Configuration for the application, engines, and railties goes here. + # + # These settings can be overridden in specific environments using the files + # in config/environments, which are processed later. + # + # config.time_zone = "Central Time (US & Canada)" + # config.eager_load_paths << Rails.root.join("extras") + + # Don't generate system test files. + config.generators.system_tests = nil + end +end diff --git a/pkg/collector/corechecks/servicediscovery/usm/testdata/root/testdata/application_invalid.rb b/pkg/collector/corechecks/servicediscovery/usm/testdata/root/testdata/application_invalid.rb new file mode 100644 index 0000000000000..5120fe0be00a0 --- /dev/null +++ b/pkg/collector/corechecks/servicediscovery/usm/testdata/root/testdata/application_invalid.rb @@ -0,0 +1,2 @@ +class SomeRubyClass +end From 5d0c6d050d7d10c5beb827c422da627d8f5e6860 Mon Sep 17 00:00:00 2001 From: Guillaume Pagnoux Date: Tue, 22 Oct 2024 10:13:50 +0200 Subject: [PATCH 07/18] CR: fix copyright --- pkg/collector/corechecks/servicediscovery/usm/ruby_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/collector/corechecks/servicediscovery/usm/ruby_test.go b/pkg/collector/corechecks/servicediscovery/usm/ruby_test.go index c7cc732cf1ac8..1edc3991be0bc 100644 --- a/pkg/collector/corechecks/servicediscovery/usm/ruby_test.go +++ b/pkg/collector/corechecks/servicediscovery/usm/ruby_test.go @@ -1,7 +1,7 @@ // Unless explicitly stated otherwise all files in this repository are licensed // under the Apache License Version 2.0. // This product includes software developed at Datadog (https://www.datadoghq.com/). -// Copyright 2016-present Datadog, Inc. +// Copyright 2024-present Datadog, Inc. //go:build !windows From c1425faa2f701171d7d82a6fcacc298e1f27b877 Mon Sep 17 00:00:00 2001 From: Guillaume Pagnoux Date: Tue, 22 Oct 2024 10:14:15 +0200 Subject: [PATCH 08/18] CR: fix documentation format --- pkg/collector/corechecks/servicediscovery/usm/ruby.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/collector/corechecks/servicediscovery/usm/ruby.go b/pkg/collector/corechecks/servicediscovery/usm/ruby.go index 27b0a91e4ed42..5aa33fe117c54 100644 --- a/pkg/collector/corechecks/servicediscovery/usm/ruby.go +++ b/pkg/collector/corechecks/servicediscovery/usm/ruby.go @@ -87,7 +87,7 @@ func (r railsDetector) findRailsApplicationName(filename string) (string, bool) return "", false } -// Converts a PascalCasedWord to a snake_cased_word. +// railsUnderscore converts a PascalCasedWord to a snake_cased_word. // It keeps uppercase acronyms together when converting (e.g. "HTTPServer" -> "http_server"). func railsUnderscore(pascalCasedWord string) string { snake := matchFirstCap.ReplaceAllString(pascalCasedWord, "${1}_${2}") From c87d97ed0e4d942655ffc1fe3082a2df22339c69 Mon Sep 17 00:00:00 2001 From: Guillaume Pagnoux Date: Tue, 22 Oct 2024 10:14:26 +0200 Subject: [PATCH 09/18] CR: remove extraneous log --- pkg/collector/corechecks/servicediscovery/usm/ruby.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/collector/corechecks/servicediscovery/usm/ruby.go b/pkg/collector/corechecks/servicediscovery/usm/ruby.go index 5aa33fe117c54..53fa2e4d1f3ec 100644 --- a/pkg/collector/corechecks/servicediscovery/usm/ruby.go +++ b/pkg/collector/corechecks/servicediscovery/usm/ruby.go @@ -58,7 +58,6 @@ func (r railsDetector) detect(_ []string) (ServiceMetadata, bool) { // findRailsApplicationName scans the `config/application.rb` file to find the // Rails application name. func (r railsDetector) findRailsApplicationName(filename string) (string, bool) { - log.Debugf("findRailsApplicationName") file, err := r.ctx.fs.Open(filename) if err != nil { log.Debugf("could not open application.rb") From d18d420f46904d438f889760bedcbc56af0a9f57 Mon Sep 17 00:00:00 2001 From: Guillaume Pagnoux Date: Tue, 22 Oct 2024 10:14:44 +0200 Subject: [PATCH 10/18] CR: make map access and cast safe in rails detector --- pkg/collector/corechecks/servicediscovery/usm/ruby.go | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/pkg/collector/corechecks/servicediscovery/usm/ruby.go b/pkg/collector/corechecks/servicediscovery/usm/ruby.go index 53fa2e4d1f3ec..82a0220641328 100644 --- a/pkg/collector/corechecks/servicediscovery/usm/ruby.go +++ b/pkg/collector/corechecks/servicediscovery/usm/ruby.go @@ -35,7 +35,16 @@ func newRailsDetector(ctx DetectionContext) detector { // project is created. This file should contain a `module` declaration with the // application name. func (r railsDetector) detect(_ []string) (ServiceMetadata, bool) { - proc := r.ctx.contextMap[ServiceProc].(*process.Process) + var proc *process.Process + + if procEntry, ok := r.ctx.ContextMap[ServiceProc]; ok { + if p, ok := procEntry.(*process.Process); ok { + proc = p + } else { + log.Error("could not get process object in rails detector") + } + } + cwd, err := proc.Cwd() if err != nil { log.Debugf("could not get cwd of process: %s", err) From 3f545e3ba6ff93231c86a3379a4feab6bd8d3299 Mon Sep 17 00:00:00 2001 From: Guillaume Pagnoux Date: Tue, 22 Oct 2024 10:22:36 +0200 Subject: [PATCH 11/18] CR: improve error handling in ruby detector --- .../corechecks/servicediscovery/usm/ruby.go | 21 +++++++++---------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/pkg/collector/corechecks/servicediscovery/usm/ruby.go b/pkg/collector/corechecks/servicediscovery/usm/ruby.go index 82a0220641328..3b305a8471b59 100644 --- a/pkg/collector/corechecks/servicediscovery/usm/ruby.go +++ b/pkg/collector/corechecks/servicediscovery/usm/ruby.go @@ -6,6 +6,7 @@ package usm import ( + "fmt" "io" "io/fs" "regexp" @@ -56,8 +57,9 @@ func (r railsDetector) detect(_ []string) (ServiceMetadata, bool) { return ServiceMetadata{}, false } - name, ok := r.findRailsApplicationName(absFile) - if !ok { + name, err := r.findRailsApplicationName(absFile) + if err != nil { + log.Debugf("could not find ruby application name: %s", err) return ServiceMetadata{}, false } @@ -66,33 +68,30 @@ func (r railsDetector) detect(_ []string) (ServiceMetadata, bool) { // findRailsApplicationName scans the `config/application.rb` file to find the // Rails application name. -func (r railsDetector) findRailsApplicationName(filename string) (string, bool) { +func (r railsDetector) findRailsApplicationName(filename string) (string, error) { file, err := r.ctx.fs.Open(filename) if err != nil { - log.Debugf("could not open application.rb") - return "", false + return "", fmt.Errorf("could not open application.rb: %w", err) } defer file.Close() reader, err := SizeVerifiedReader(file) if err != nil { - log.Debugf("skipping application.rb (%q): %v", filename, err) - return "", true + return "", fmt.Errorf("skipping application.rb (%q): %w", filename, err) } bytes, err := io.ReadAll(reader) if err != nil { - log.Debugf("unable to read application.rb (%q): %v", filename, err) - return "", true + return "", fmt.Errorf("unable to read application.rb (%q): %w", filename, err) } matches := moduleRegexp.FindSubmatch(bytes) if matches != nil { - return string(matches[1]), true + return string(matches[1]), nil } // No match found - return "", false + return "", fmt.Errorf("could not find Ruby module name") } // railsUnderscore converts a PascalCasedWord to a snake_cased_word. From 3d19532c236d9b5b6bfc295eaa949c8767286597 Mon Sep 17 00:00:00 2001 From: Guillaume Pagnoux Date: Tue, 22 Oct 2024 10:23:17 +0200 Subject: [PATCH 12/18] CR: fix copyright in ruby.go --- pkg/collector/corechecks/servicediscovery/usm/ruby.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/collector/corechecks/servicediscovery/usm/ruby.go b/pkg/collector/corechecks/servicediscovery/usm/ruby.go index 3b305a8471b59..f831914ed0e64 100644 --- a/pkg/collector/corechecks/servicediscovery/usm/ruby.go +++ b/pkg/collector/corechecks/servicediscovery/usm/ruby.go @@ -1,7 +1,7 @@ // Unless explicitly stated otherwise all files in this repository are licensed // under the Apache License Version 2.0. // This product includes software developed at Datadog (https://www.datadoghq.com/). -// Copyright 2016-present Datadog, Inc. +// Copyright 2024-present Datadog, Inc. package usm From 414f1bea020d361a618901f3f4981cb82dddd9d0 Mon Sep 17 00:00:00 2001 From: Guillaume Pagnoux Date: Tue, 22 Oct 2024 10:26:02 +0200 Subject: [PATCH 13/18] CR: fix Submatch call --- pkg/collector/corechecks/servicediscovery/usm/ruby.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/collector/corechecks/servicediscovery/usm/ruby.go b/pkg/collector/corechecks/servicediscovery/usm/ruby.go index f831914ed0e64..65686e7c70a37 100644 --- a/pkg/collector/corechecks/servicediscovery/usm/ruby.go +++ b/pkg/collector/corechecks/servicediscovery/usm/ruby.go @@ -86,12 +86,12 @@ func (r railsDetector) findRailsApplicationName(filename string) (string, error) } matches := moduleRegexp.FindSubmatch(bytes) - if matches != nil { - return string(matches[1]), nil + if len(matches) < 2 { + // No match found + return "", fmt.Errorf("could not find Ruby module name") } - // No match found - return "", fmt.Errorf("could not find Ruby module name") + return string(matches[1]), nil } // railsUnderscore converts a PascalCasedWord to a snake_cased_word. From 0bf6cc5e5ea23b3c6424dd6543b2c33cf604906a Mon Sep 17 00:00:00 2001 From: Guillaume Pagnoux Date: Tue, 22 Oct 2024 10:27:31 +0200 Subject: [PATCH 14/18] CR: test: run test on Linux only --- pkg/collector/corechecks/servicediscovery/usm/ruby_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/collector/corechecks/servicediscovery/usm/ruby_test.go b/pkg/collector/corechecks/servicediscovery/usm/ruby_test.go index 1edc3991be0bc..eac7d4ec2bfc0 100644 --- a/pkg/collector/corechecks/servicediscovery/usm/ruby_test.go +++ b/pkg/collector/corechecks/servicediscovery/usm/ruby_test.go @@ -3,7 +3,7 @@ // This product includes software developed at Datadog (https://www.datadoghq.com/). // Copyright 2024-present Datadog, Inc. -//go:build !windows +//go:build linux package usm From 23e9270135a35ce6a36ef15cfa4c8990d0f0654c Mon Sep 17 00:00:00 2001 From: Guillaume Pagnoux Date: Tue, 22 Oct 2024 10:45:02 +0200 Subject: [PATCH 15/18] CR: detector: log type in case of case error --- pkg/collector/corechecks/servicediscovery/usm/ruby.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/collector/corechecks/servicediscovery/usm/ruby.go b/pkg/collector/corechecks/servicediscovery/usm/ruby.go index 65686e7c70a37..6617cb12c6545 100644 --- a/pkg/collector/corechecks/servicediscovery/usm/ruby.go +++ b/pkg/collector/corechecks/servicediscovery/usm/ruby.go @@ -42,7 +42,7 @@ func (r railsDetector) detect(_ []string) (ServiceMetadata, bool) { if p, ok := procEntry.(*process.Process); ok { proc = p } else { - log.Error("could not get process object in rails detector") + log.Errorf("could not get process object in rails detector: got type %T", procEntry) } } From f5c967517a47bb3e2abd6d7deb42ced81551f728 Mon Sep 17 00:00:00 2001 From: Guillaume Pagnoux Date: Tue, 22 Oct 2024 10:45:44 +0200 Subject: [PATCH 16/18] CR: use errors.New instead of fmt.Errorf when regex does not match --- pkg/collector/corechecks/servicediscovery/usm/ruby.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/collector/corechecks/servicediscovery/usm/ruby.go b/pkg/collector/corechecks/servicediscovery/usm/ruby.go index 6617cb12c6545..69122052ec277 100644 --- a/pkg/collector/corechecks/servicediscovery/usm/ruby.go +++ b/pkg/collector/corechecks/servicediscovery/usm/ruby.go @@ -6,6 +6,7 @@ package usm import ( + "errors" "fmt" "io" "io/fs" @@ -88,7 +89,7 @@ func (r railsDetector) findRailsApplicationName(filename string) (string, error) matches := moduleRegexp.FindSubmatch(bytes) if len(matches) < 2 { // No match found - return "", fmt.Errorf("could not find Ruby module name") + return "", errors.New("could not find Ruby module name") } return string(matches[1]), nil From 548757c5b2c7c950af5d58ebd09633130fccc9a3 Mon Sep 17 00:00:00 2001 From: Guillaume Pagnoux Date: Tue, 22 Oct 2024 17:23:08 +0200 Subject: [PATCH 17/18] CR: add additional test cases --- .../servicediscovery/usm/ruby_test.go | 21 ++++++++++++++----- .../root/testdata/{ => ruby}/application.rb | 0 .../testdata/ruby/application_accronym.rb | 1 + .../{ => ruby}/application_invalid.rb | 0 4 files changed, 17 insertions(+), 5 deletions(-) rename pkg/collector/corechecks/servicediscovery/usm/testdata/root/testdata/{ => ruby}/application.rb (100%) create mode 100644 pkg/collector/corechecks/servicediscovery/usm/testdata/root/testdata/ruby/application_accronym.rb rename pkg/collector/corechecks/servicediscovery/usm/testdata/root/testdata/{ => ruby}/application_invalid.rb (100%) diff --git a/pkg/collector/corechecks/servicediscovery/usm/ruby_test.go b/pkg/collector/corechecks/servicediscovery/usm/ruby_test.go index eac7d4ec2bfc0..40f6323027a80 100644 --- a/pkg/collector/corechecks/servicediscovery/usm/ruby_test.go +++ b/pkg/collector/corechecks/servicediscovery/usm/ruby_test.go @@ -23,12 +23,22 @@ func TestGenerateNameFromRailsApplicationRb(t *testing.T) { }{ { name: "name is found", - path: "./testdata/application.rb", + path: "./testdata/ruby/application.rb", expected: "rails_hello", }, { name: "name not found", - path: "./testdata/application_invalid.rb", + path: "./testdata/ruby/application_invalid.rb", + expected: "", + }, + { + name: "accronym in module name", + path: "./testdata/ruby/application_accronym.rb", + expected: "http_server", + }, + { + name: "file does not exists", + path: "./testdata/ruby/application_does_not_exist.rb", expected: "", }, } @@ -38,10 +48,11 @@ func TestGenerateNameFromRailsApplicationRb(t *testing.T) { t.Run(tt.name, func(t *testing.T) { instance := &railsDetector{ctx: DetectionContext{ fs: NewSubDirFS(full), - contextMap: make(DetectorContextMap), + ContextMap: make(DetectorContextMap), }} - value, ok := instance.findRailsApplicationName(tt.path) - assert.Equal(t, len(tt.expected) > 0, ok) + value, err := instance.findRailsApplicationName(tt.path) + t.Log(err) + assert.Equal(t, len(tt.expected) > 0, err == nil) assert.Equal(t, tt.expected, railsUnderscore(value)) }) } diff --git a/pkg/collector/corechecks/servicediscovery/usm/testdata/root/testdata/application.rb b/pkg/collector/corechecks/servicediscovery/usm/testdata/root/testdata/ruby/application.rb similarity index 100% rename from pkg/collector/corechecks/servicediscovery/usm/testdata/root/testdata/application.rb rename to pkg/collector/corechecks/servicediscovery/usm/testdata/root/testdata/ruby/application.rb diff --git a/pkg/collector/corechecks/servicediscovery/usm/testdata/root/testdata/ruby/application_accronym.rb b/pkg/collector/corechecks/servicediscovery/usm/testdata/root/testdata/ruby/application_accronym.rb new file mode 100644 index 0000000000000..5e33caace3fc1 --- /dev/null +++ b/pkg/collector/corechecks/servicediscovery/usm/testdata/root/testdata/ruby/application_accronym.rb @@ -0,0 +1 @@ +module HTTPServer diff --git a/pkg/collector/corechecks/servicediscovery/usm/testdata/root/testdata/application_invalid.rb b/pkg/collector/corechecks/servicediscovery/usm/testdata/root/testdata/ruby/application_invalid.rb similarity index 100% rename from pkg/collector/corechecks/servicediscovery/usm/testdata/root/testdata/application_invalid.rb rename to pkg/collector/corechecks/servicediscovery/usm/testdata/root/testdata/ruby/application_invalid.rb From bf02b556d21f971da801449a9d678c59921fba1c Mon Sep 17 00:00:00 2001 From: Guillaume Pagnoux Date: Tue, 22 Oct 2024 17:24:46 +0200 Subject: [PATCH 18/18] CR: use path.Join instead of abs --- pkg/collector/corechecks/servicediscovery/usm/ruby.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/collector/corechecks/servicediscovery/usm/ruby.go b/pkg/collector/corechecks/servicediscovery/usm/ruby.go index 69122052ec277..8df8b59991e8d 100644 --- a/pkg/collector/corechecks/servicediscovery/usm/ruby.go +++ b/pkg/collector/corechecks/servicediscovery/usm/ruby.go @@ -10,6 +10,7 @@ import ( "fmt" "io" "io/fs" + "path" "regexp" "strings" @@ -53,7 +54,7 @@ func (r railsDetector) detect(_ []string) (ServiceMetadata, bool) { return ServiceMetadata{}, false } - absFile := abs("config/application.rb", cwd) + absFile := path.Join(cwd, "config/application.rb") if _, err := fs.Stat(r.ctx.fs, absFile); err != nil { return ServiceMetadata{}, false }