Adds windows support to process agent #96
Conversation
Adds windows support (w/o wmi) for collecting process stats. Uses specific branch of datadog/gopsutil - Note: hand-edited glide.lock to get correct version. This is bad. Adds prelim windows service processing. Adds version resource on windows Adds message file for reporting to event viewer
available cpu time)
Merge original windows changes. Make it compile w/o docker
so logging goes to file
Initial rev appveyor
|
|
derekwbrown
force-pushed
the
db/new_windows
branch
4 times, most recently
from
e276532
to
498b36e
Compare
Add binary signing
derekwbrown
force-pushed
the
db/new_windows
branch
from
498b36e
to
41ad6eb
Compare
derekwbrown
force-pushed
the
db/new_windows
branch
3 times, most recently
from
430dcc4
to
1053270
Compare
…gent into db/new_windows
derekwbrown
force-pushed
the
db/new_windows
branch
from
1053270
to
329bc77
Compare
There was a problem hiding this comment.
Looks good to me, just a few comments.
If @conorbranagan has time, it'd be nice to have him look over this as well, but I don't think it's a deal-breaker.
glide.yaml
Outdated
| @@ -1,13 +1,15 @@ | |||
| package: github.com/DataDog/datadog-process-agent | |||
| import: | |||
| - package: github.com/DataDog/datadog-agent | |||
| version: f1d63c176a934c8b8aefa31e2c7d938b0e50e348 | |||
| vcs: git | |||
| version: db/merge_no_docker | |||
There was a problem hiding this comment.
Does it still need to be pinned to this branch, or can we change it to the latest master?
glide.yaml
Outdated
| version: fb25fcedf155de94e762080a914adb5436d45b9c | ||
| - package: github.com/DataDog/gopsutil | ||
| vcs: git | ||
| version: db/add_windows_checks |
There was a problem hiding this comment.
Can be pinned to the latest master :)
| if err := NewLoggerLevel(cfg.LogLevel, cfg.LogFile); err != nil { | ||
| return nil, err | ||
| } | ||
| //if err := NewLoggerLevel(cfg.LogLevel, cfg.LogFile); err != nil { |
There was a problem hiding this comment.
I'm not sure this is entirely correct, so the configuration is completely ignored?
There was a problem hiding this comment.
I'll put it back, but IIRC I did that because it's logging to console no matter what, and on windows, when running as a service console == /dev/null
| SystemTime: int64(t2.System), | ||
| } | ||
| } | ||
| func calculatePct(deltaProc, deltaTime, numCPU float64) float32 { |
There was a problem hiding this comment.
Since this is in both nix + windows, it might make sense to throw this into process.go?
| import ( | ||
| "flag" | ||
| "fmt" | ||
| // "log" |
| uninstallService bool | ||
| startService bool | ||
| stopService bool | ||
| } |
There was a problem hiding this comment.
nit: can you wrap all of these in a single var() block and clean up the comments with flag code
There was a problem hiding this comment.
Sure. Something to think about, we use github.com/spf13/cobra on the metrics agent. Might be nice to standardize.
| @@ -7,5 +7,5 @@ import "github.com/DataDog/datadog-agent/pkg/util/docker" | |||
| // GetContainers is the unique method that returns all containers on the host (or in the task) | |||
| // and that other agents can consume so that we don't have to convert all containers to the format. | |||
| func GetContainers() ([]*docker.Container, error) { | |||
| return *docker.Container{}, nil | |||
| return make([]*docker.Container, 0), nil | |||
There was a problem hiding this comment.
this can just be nil, nil without any allocation
There was a problem hiding this comment.
@conorbranagan I did it initially with nil,nil; however, there's other code that iterates the return (and crashes). Needs to be empty (so length is zero) I think.
There was a problem hiding this comment.
Hm you should be able to iterate on a nil map but maybe there's something weird going on. Not a huge deal either way.
derekwbrown
force-pushed
the
db/new_windows
branch
from
f091d91
to
637c69e
Compare
review feedback
derekwbrown
force-pushed
the
db/new_windows
branch
from
637c69e
to
ae915b0
Compare
Generate windows executable for windows process agent.