Skip to content

Commit

Permalink
[rtloader] high-level memory profiling metrics (#3961)
Browse files Browse the repository at this point in the history
* [rtloader] renaming cgo_free.c/h sources to memory.c/h

* [rtloader] adding memory tracking facilities

* [rtloader] actually set callback, dummy

* [collector][memory] add small debug statement

* [collector][memory] tagger should use custom allocator from cgo

* [collector][memory] delete pointer reference on frees

* [rtloader][memory] override strdup

* [rtloader][memory] replace strdup with internal variant

* [rtloader][memory] moving header to non-standard name

* [memory][cgo] lets track cgo memory a little better

* [rtloader][two] cleanup warnings

* [rtloader][three] cleanup warnings

* [rtloader] expvar for untracked frees

* [rtloader][memory] generate cgo stacktrace with symbolizer

* [rtloader][memory] generate cgo stacktrace with symbolizer

* [rtloader] print (risky) free pointer as C-string - debugging

* [go][rtloader] track more CStrings

* [python][memory] sync tracking - order matters

* [rtloader] adding pickle prototypes - disabled

* [rtloader] couple formatting fixes

* [rtloader] fix strdupe unsupported compiler define

* [rtloader] fixing preprocessor directive

* [rtloader] enable check pickling

* [gitlab] trying builders aligned on GCC version 4.7 - suse pending

* [rtloader][three] fixing typo

* [omnibus] datadog-agent: patch needed for SLES11 build

* Remove pickling logic: address in separate PR

Revert "[rtloader] adding pickle prototypes - disabled"

This reverts commit dcf6f4a.

This reverts commit e3b3be8.

This reverts commit 277c2cb.

* [reno] adding release note

* [rtloader] memory: polishing comments a little bit

* [rtloader][python] remove dangerous logging outdated comments
  • Loading branch information
truthbk committed Aug 12, 2019
1 parent f512f07 commit 23ceccc
Show file tree
Hide file tree
Showing 38 changed files with 519 additions and 158 deletions.
19 changes: 18 additions & 1 deletion Gopkg.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 9 additions & 0 deletions Gopkg.toml
Original file line number Diff line number Diff line change
Expand Up @@ -197,3 +197,12 @@
[[override]]
name = "golang.org/x/sys"
revision = "61b9204099cb1bebc803c9ffb9b2d3acd9d457d9"

[[constraint]]
branch = "master"
name = "github.com/benesch/cgosymbolizer"

[[override]]
name = "github.com/ianlancetaylor/cgosymbolizer"
source = "github.com/ianlancetaylor/cgosymbolizer"
revision = "f5072df9c550dc687157e5d7efb50825cdf8f0eb"
2 changes: 2 additions & 0 deletions LICENSE-3rdparty.csv
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
Component,Origin,License
core,bitbucket.org/ww/goautoneg,BSD-3-Clause
core,github.com/benesch/cgosymbolizer,BSD-3-Clause
core,github.com/ianlancetaylor/cgosymbolizer,BSD-3-Clause
core,github.com/DataDog/agent-payload,BSD-3-Clause
core,github.com/DataDog/gohai,MIT
core,github.com/DataDog/mmh3,MIT
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
From 91f7052d0d3a6b06ca72c40d50ae268789a8ea5e Mon Sep 17 00:00:00 2001
From: Jaime Fullaondo <jaime.fullaondo@datadoghq.com>
Date: Wed, 31 Jul 2019 11:40:43 +0200
Subject: [PATCH] [sles] sys/types.h must be included here to build

---
symbolizer.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/symbolizer.c b/symbolizer.c
index aa18aee..a158edf 100644
--- a/symbolizer.c
+++ b/symbolizer.c
@@ -6,6 +6,7 @@

#include <stdint.h>
#include <string.h>
+#include <sys/types.h>

#include "backtrace.h"
#include "internal.h"
--
2.20.1

7 changes: 7 additions & 0 deletions omnibus/config/software/datadog-agent.rb
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,13 @@
# include embedded path (mostly for `pkg-config` binary)
env = with_embedded_path(env)

# cgosymbolizer must be patched on SLES11 builders - PR upstream pending merge
if suse?
patch :source => "0001-sles-sys-types.h-must-be-included-here-to-build.patch", :plevel => 1,
:acceptable_output => "Reversed (or previously applied) patch detected",
:target => "#{gopath.to_path}/src/github.com/DataDog/datadog-agent/vendor/github.com/ianlancetaylor/cgosymbolizer/symbolizer.c"
end

# we assume the go deps are already installed before running omnibus
if windows?
platform = windows_arch_i386? ? "x86" : "x64"
Expand Down
25 changes: 13 additions & 12 deletions pkg/collector/python/check.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,9 @@ import (

/*
#include <stdlib.h>
#include <datadog_agent_rtloader.h>
#include "datadog_agent_rtloader.h"
#include "rtloader_mem.h"
char *getStringAddr(char **array, unsigned int idx);
*/
Expand Down Expand Up @@ -184,15 +186,14 @@ func (c *PythonCheck) Configure(data integration.Data, initConfig integration.Da
}
}

cInitConfig := C.CString(string(initConfig))

cInstance := C.CString(string(data))
cCheckID := C.CString(string(c.id))
cCheckName := C.CString(c.ModuleName)
defer C.free(unsafe.Pointer(cInitConfig))
defer C.free(unsafe.Pointer(cInstance))
defer C.free(unsafe.Pointer(cCheckID))
defer C.free(unsafe.Pointer(cCheckName))
cInitConfig := TrackedCString(string(initConfig))
cInstance := TrackedCString(string(data))
cCheckID := TrackedCString(string(c.id))
cCheckName := TrackedCString(c.ModuleName)
defer C._free(unsafe.Pointer(cInitConfig))
defer C._free(unsafe.Pointer(cInstance))
defer C._free(unsafe.Pointer(cCheckID))
defer C._free(unsafe.Pointer(cCheckName))

var check *C.rtloader_pyobject_t
res := C.get_check(rtloader, c.class, cInitConfig, cInstance, cCheckID, cCheckName, &check)
Expand All @@ -206,8 +207,8 @@ func (c *PythonCheck) Configure(data integration.Data, initConfig integration.Da
log.Errorf("error serializing agent config: %s", err)
return err
}
cAgentConfig := C.CString(string(agentConfig))
defer C.free(unsafe.Pointer(cAgentConfig))
cAgentConfig := TrackedCString(string(agentConfig))
defer C._free(unsafe.Pointer(cAgentConfig))

res := C.get_check_deprecated(rtloader, c.class, cInitConfig, cInstance, cAgentConfig, cCheckID, cCheckName, &check)
if res == 0 {
Expand Down
14 changes: 8 additions & 6 deletions pkg/collector/python/datadog_agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,11 @@ import (
)

/*
#include <datadog_agent_rtloader.h>
#cgo !windows LDFLAGS: -ldatadog-agent-rtloader -ldl
#cgo windows LDFLAGS: -ldatadog-agent-rtloader -lstdc++ -static
#include "datadog_agent_rtloader.h"
#include "rtloader_mem.h"
*/
import (
"C"
Expand All @@ -34,7 +36,7 @@ import (
func GetVersion(agentVersion **C.char) {
av, _ := version.New(version.AgentVersion, version.Commit)
// version will be free by rtloader when it's done with it
*agentVersion = C.CString(av.GetNumber())
*agentVersion = TrackedCString(av.GetNumber())
}

// GetHostname exposes the current hostname of the agent to Python checks.
Expand All @@ -46,15 +48,15 @@ func GetHostname(hostname **C.char) {
goHostname = ""
}
// hostname will be free by rtloader when it's done with it
*hostname = C.CString(goHostname)
*hostname = TrackedCString(goHostname)
}

// GetClusterName exposes the current clustername (if it exists) of the agent to Python checks.
//export GetClusterName
func GetClusterName(clusterName **C.char) {
goClusterName := clustername.GetClusterName()
// clusterName will be free by rtloader when it's done with it
*clusterName = C.CString(goClusterName)
*clusterName = TrackedCString(goClusterName)
}

// Headers returns a basic set of HTTP headers that can be used by clients in Python checks.
Expand All @@ -69,7 +71,7 @@ func Headers(yamlPayload **C.char) {
return
}
// yamlPayload will be free by rtloader when it's done with it
*yamlPayload = C.CString(string(data))
*yamlPayload = TrackedCString(string(data))
}

// GetConfig returns a value from the agent configuration.
Expand All @@ -90,7 +92,7 @@ func GetConfig(key *C.char, yamlPayload **C.char) {
return
}
// yaml Payload will be free by rtloader when it's done with it
*yamlPayload = C.CString(string(data))
*yamlPayload = TrackedCString(string(data))
}

// LogMessage logs a message from python through the agent logger (see
Expand Down
17 changes: 10 additions & 7 deletions pkg/collector/python/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,10 @@ import (

/*
#include <stdlib.h>
#include <datadog_agent_rtloader.h>
#include "datadog_agent_rtloader.h"
#include "rtloader_mem.h"
char *getStringAddr(char **array, unsigned int idx);
*/
import "C"
Expand Down Expand Up @@ -168,12 +171,12 @@ func SetPythonPsutilProcPath(procPath string) error {
glock := newStickyLock()
defer glock.unlock()

module := C.CString(psutilModule)
defer C.free(unsafe.Pointer(module))
attrName := C.CString(psutilProcPath)
defer C.free(unsafe.Pointer(attrName))
attrValue := C.CString(procPath)
defer C.free(unsafe.Pointer(attrValue))
module := TrackedCString(psutilModule)
defer C._free(unsafe.Pointer(module))
attrName := TrackedCString(psutilProcPath)
defer C._free(unsafe.Pointer(attrName))
attrValue := TrackedCString(procPath)
defer C._free(unsafe.Pointer(attrValue))

C.set_module_attr_string(rtloader, module, attrName, attrValue)
return getRtLoaderError()
Expand Down
31 changes: 23 additions & 8 deletions pkg/collector/python/init.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,9 @@ import (
#cgo !windows LDFLAGS: -ldatadog-agent-rtloader -ldl
#cgo windows LDFLAGS: -ldatadog-agent-rtloader -lstdc++ -static
#include <datadog_agent_rtloader.h>
#include "datadog_agent_rtloader.h"
#include "rtloader_mem.h"
#include <stdlib.h>
// helpers
Expand All @@ -39,6 +41,14 @@ char *getStringAddr(char **array, unsigned int idx) {
return array[idx];
}
//
// init memory tracking facilities method
//
void MemoryTracker(void *, size_t, rtloader_mem_ops_t);
void initMemoryTracker(void) {
set_memory_tracker_cb(MemoryTracker);
}
//
// init free method
//
Expand All @@ -47,7 +57,7 @@ char *getStringAddr(char **array, unsigned int idx) {
//
void initCgoFree(rtloader_t *rtloader) {
set_cgo_free_cb(rtloader, free);
set_cgo_free_cb(rtloader, _free);
}
//
Expand Down Expand Up @@ -230,17 +240,20 @@ func Initialize(paths ...string) error {
}
}

// memory related RTLoader-global initialization
C.initMemoryTracker()

var pyErr *C.char = nil
if pythonVersion == "2" {
csPythonHome2 := C.CString(pythonHome2)
csPythonHome2 := TrackedCString(pythonHome2)
rtloader = C.make2(csPythonHome2, &pyErr)
C.free(unsafe.Pointer(csPythonHome2))
C._free(unsafe.Pointer(csPythonHome2))
log.Infof("Initializing rtloader with python2 %s", pythonHome2)
PythonHome = pythonHome2
} else if pythonVersion == "3" {
csPythonHome3 := C.CString(pythonHome3)
csPythonHome3 := TrackedCString(pythonHome3)
rtloader = C.make3(csPythonHome3, &pyErr)
C.free(unsafe.Pointer(csPythonHome3))
C._free(unsafe.Pointer(csPythonHome3))
log.Infof("Initializing rtloader with python3 %s", pythonHome3)
PythonHome = pythonHome3
} else {
Expand All @@ -250,14 +263,16 @@ func Initialize(paths ...string) error {
if rtloader == nil {
err := addExpvarPythonInitErrors(fmt.Sprintf("could not load runtime python for version %s: %s", pythonVersion, C.GoString(pyErr)))
if pyErr != nil {
C.free(unsafe.Pointer(pyErr))
// pyErr tracked when created in rtloader
C._free(unsafe.Pointer(pyErr))
}
return err
}

// Set the PYTHONPATH if needed.
for _, p := range paths {
C.add_python_path(rtloader, C.CString(p))
// bounded but never released allocations with CString
C.add_python_path(rtloader, TrackedCString(p))
}

// Any platform-specific initialization
Expand Down
2 changes: 1 addition & 1 deletion pkg/collector/python/kubeutil.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,5 +76,5 @@ func GetKubeletConnectionInfo(payload **C.char) {
cache.Cache.Set(kubeletCacheKey, creds, 5*time.Minute)
}

*payload = C.CString(creds)
*payload = TrackedCString(creds)
}
26 changes: 18 additions & 8 deletions pkg/collector/python/loader.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,12 @@ import (
"github.com/DataDog/datadog-agent/pkg/util/log"
)

// #include <stdlib.h>
// #include <datadog_agent_rtloader.h>
/*
#include <stdlib.h>
#include "datadog_agent_rtloader.h"
#include "rtloader_mem.h"
*/
import "C"

var (
Expand Down Expand Up @@ -122,8 +126,9 @@ func (cl *PythonCheckLoader) Load(config integration.Config) ([]check.Check, err
var checkModule *C.rtloader_pyobject_t
var checkClass *C.rtloader_pyobject_t
for _, name = range modules {
moduleName := C.CString(name)
defer C.free(unsafe.Pointer(moduleName))
// TrackedCStrings untracked by memory tracker currently
moduleName := TrackedCString(name)
defer C._free(unsafe.Pointer(moduleName))
if res := C.get_class(rtloader, moduleName, &checkModule, &checkClass); res != 0 {
if strings.HasPrefix(name, fmt.Sprintf("%s.", wheelNamespace)) {
loadedAsWheel = true
Expand All @@ -147,8 +152,11 @@ func (cl *PythonCheckLoader) Load(config integration.Config) ([]check.Check, err
wheelVersion := "unversioned"
// getting the wheel version for the check
var version *C.char
versionAttr := C.CString("__version__")
defer C.free(unsafe.Pointer(versionAttr))

// TrackedCStrings untracked by memory tracker currently
versionAttr := TrackedCString("__version__")
defer C._free(unsafe.Pointer(versionAttr))
// get_attr_string allocation tracked by memory tracker
if res := C.get_attr_string(rtloader, checkModule, versionAttr, &version); res != 0 {
wheelVersion = C.GoString(version)
C.rtloader_free(rtloader, unsafe.Pointer(version))
Expand All @@ -161,8 +169,10 @@ func (cl *PythonCheckLoader) Load(config integration.Config) ([]check.Check, err
// Let's use the module namespace to try to decide if this was a
// custom check, check for py3 compatibility
var checkFilePath *C.char
fileAttr := C.CString("__file__")
defer C.free(unsafe.Pointer(fileAttr))

fileAttr := TrackedCString("__file__")
defer C._free(unsafe.Pointer(fileAttr))
// get_attr_string allocation tracked by memory tracker
if res := C.get_attr_string(rtloader, checkModule, fileAttr, &checkFilePath); res != 0 {
reportPy3Warnings(name, C.GoString(checkFilePath))
C.rtloader_free(rtloader, unsafe.Pointer(checkFilePath))
Expand Down
Loading

0 comments on commit 23ceccc

Please sign in to comment.