Fix file handle check on Darwin (#21013)

* refactor: rename freebsd file_handle check files to bsd

* chore(file_handle): add package comment

* feat(file_handles): fix bsd implementation for darwin

* fix(file_handles): fix bsd tests

* chore: add release note

* fix: write proper oid name in logs and errors

pkg/collector/corechecks/system/filehandles/docs.go

@@ -0,0 +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.
+
+// Package filehandles defines the file_handle core check
+package filehandles

pkg/collector/corechecks/system/filehandles/file_handles.go

@@ -2,7 +2,7 @@
 // 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 && !freebsd
+//go:build !windows && !freebsd && !darwin
 
 package filehandles
 

pkg/collector/corechecks/system/filehandles/file_handles_bsd.go

@@ -0,0 +1,71 @@
+// 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 freebsd || darwin
+
+package filehandles
+
+import (
+	"github.com/blabber/go-freebsd-sysctl/sysctl"
+
+	"github.com/DataDog/datadog-agent/pkg/aggregator/sender"
+	"github.com/DataDog/datadog-agent/pkg/autodiscovery/integration"
+	"github.com/DataDog/datadog-agent/pkg/collector/check"
+	core "github.com/DataDog/datadog-agent/pkg/collector/corechecks"
+	"github.com/DataDog/datadog-agent/pkg/util/log"
+)
+
+// For testing purpose
+var getInt64 = sysctl.GetInt64
+
+const fileHandlesCheckName = "file_handle"
+
+type fhCheck struct {
+	core.CheckBase
+}
+
+// Run executes the check
+func (c *fhCheck) Run() error {
+
+	sender, err := c.GetSender()
+	if err != nil {
+		return err
+	}
+	openFh, err := getInt64(openfilesOID)
+	if err != nil {
+		log.Warnf("Error getting %s value %v", openfilesOID, err)
+		return err
+	}
+	maxFh, err := getInt64("kern.maxfiles")
+	if err != nil {
+		log.Warnf("Error getting kern.maxfiles value %v", err)
+		return err
+	}
+	log.Debugf("Submitting %s %v", openfilesOID, openFh)
+	log.Debugf("Submitting kern.maxfiles %v", maxFh)
+	sender.Gauge("system.fs.file_handles.used", float64(openFh), "", nil)
+	sender.Gauge("system.fs.file_handles.max", float64(maxFh), "", nil)
+	sender.Commit()
+
+	return nil
+}
+
+// The check doesn't need configuration
+func (c *fhCheck) Configure(senderManager sender.SenderManager, integrationConfigDigest uint64, data integration.Data, initConfig integration.Data, source string) (err error) {
+	if err := c.CommonConfigure(senderManager, integrationConfigDigest, initConfig, data, source); err != nil {
+		return err
+	}
+
+	return err
+}
+
+func fhFactory() check.Check {
+	return &fhCheck{
+		CheckBase: core.NewCheckBase(fileHandlesCheckName),
+	}
+}
+
+func init() {
+	core.RegisterCheck(fileHandlesCheckName, fhFactory)
+}

pkg/collector/corechecks/system/filehandles/file_handles_bsd_test.go

@@ -0,0 +1,45 @@
+// 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 freebsd || darwin
+
+package filehandles
+
+import (
+	"testing"
+
+	"github.com/DataDog/datadog-agent/pkg/aggregator/mocksender"
+	"github.com/DataDog/datadog-agent/pkg/autodiscovery/integration"
+)
+
+func GetInt64(_ string) (value int64, err error) {
+	value = 65534
+	err = nil
+	return
+}
+
+func TestFhCheckFreeBSD(t *testing.T) {
+	getInt64 = GetInt64
+
+	// we have to init the mocked sender here before fileHandleCheck.Configure(mock.GetSenderManager(), integration.FakeConfigHash, ...)
+	// (and append it to the aggregator, which is automatically done in NewMockSender)
+	// because the FinalizeCheckServiceTag is called in Configure.
+	// Hopefully, the check ID is an empty string while running unit tests;
+	mock := mocksender.NewMockSender("")
+
+	fileHandleCheck := new(fhCheck)
+	fileHandleCheck.Configure(mock.GetSenderManager(), integration.FakeConfigHash, nil, nil, "test")
+
+	// reset the check ID for the sake of correctness
+	mocksender.SetSender(mock, fileHandleCheck.ID())
+
+	mock.On("Gauge", "system.fs.file_handles.used", float64(65534), "", []string(nil)).Return().Times(1)
+	mock.On("Gauge", "system.fs.file_handles.max", float64(65534), "", []string(nil)).Return().Times(1)
+	mock.On("Commit").Return().Times(1)
+	fileHandleCheck.Run()
+
+	mock.AssertExpectations(t)
+	mock.AssertNumberOfCalls(t, "Gauge", 2)
+	mock.AssertNumberOfCalls(t, "Commit", 1)
+}

pkg/collector/corechecks/system/filehandles/file_handles_darwin.go

@@ -0,0 +1,8 @@
+// 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 filehandles
+
+const openfilesOID = "kern.num_files"

pkg/collector/corechecks/system/filehandles/file_handles_freebsd.go

@@ -2,70 +2,7 @@
 // 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 freebsd
 
 package filehandles
 
-import (
-	"github.com/blabber/go-freebsd-sysctl/sysctl"
-
-	"github.com/DataDog/datadog-agent/pkg/aggregator/sender"
-	"github.com/DataDog/datadog-agent/pkg/autodiscovery/integration"
-	"github.com/DataDog/datadog-agent/pkg/collector/check"
-	core "github.com/DataDog/datadog-agent/pkg/collector/corechecks"
-	"github.com/DataDog/datadog-agent/pkg/util/log"
-)
-
-// For testing purpose
-var getInt64 = sysctl.GetInt64
-
-const fileHandlesCheckName = "file_handle"
-
-type fhCheck struct {
-	core.CheckBase
-}
-
-// Run executes the check
-func (c *fhCheck) Run() error {
-
-	sender, err := c.GetSender()
-	if err != nil {
-		return err
-	}
-	openFh, err := getInt64("kern.openfiles")
-	if err != nil {
-		log.Warnf("Error getting kern.openfiles value %v", err)
-		return err
-	}
-	maxFh, err := getInt64("kern.maxfiles")
-	if err != nil {
-		log.Warnf("Error getting kern.maxfiles value %v", err)
-		return err
-	}
-	log.Debugf("Submitting kern.openfiles %v", openFh)
-	log.Debugf("Submitting kern.maxfiles %v", maxFh)
-	sender.Gauge("system.fs.file_handles.used", float64(openFh), "", nil)
-	sender.Gauge("system.fs.file_handles.max", float64(maxFh), "", nil)
-	sender.Commit()
-
-	return nil
-}
-
-// The check doesn't need configuration
-func (c *fhCheck) Configure(senderManager sender.SenderManager, integrationConfigDigest uint64, data integration.Data, initConfig integration.Data, source string) (err error) {
-	if err := c.CommonConfigure(senderManager, integrationConfigDigest, initConfig, data, source); err != nil {
-		return err
-	}
-
-	return err
-}
-
-func fhFactory() check.Check {
-	return &fhCheck{
-		CheckBase: core.NewCheckBase(fileHandlesCheckName),
-	}
-}
-
-func init() {
-	core.RegisterCheck(fileHandlesCheckName, fhFactory)
-}
+const openfilesOID = "kern.openfiles"

pkg/collector/corechecks/system/filehandles/file_handles_test.go

@@ -2,7 +2,7 @@
 // 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
+//go:build !windows && !freebsd && !darwin
 
 package filehandles
 

releasenotes/notes/file-check-on-darwin-ea525cc9369307a4.yaml

@@ -0,0 +1,11 @@
+# Each section from every release note are combined when the
+# CHANGELOG.rst is rendered. So the text needs to be worded so that
+# it does not depend on any information only available in another
+# section. This may mean repeating some details, but each section
+# must be readable independently of the other.
+#
+# Each section note must be formatted as reStructuredText.
+---
+fixes:
+  - |
+    Fix `file_handle` core check on Darwin by using `sysctl` system call.