-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix process agent status provider #22527
Merged
Merged
Changes from all commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
5d16b22
populate the status map properly when Unmarshal the data
GustavoCaso 01e0d34
do not reuse the status map if there is an error afetr trying to Unma…
GustavoCaso 20500da
add unit tests for process agent status provider
GustavoCaso 2ed93f0
re-enabled e2e test for process agent
GustavoCaso dc6c891
Merge branch 'main' into fix-process-agent-status-provider
GustavoCaso 561e276
move fixtures to separate files and use the embed package
GustavoCaso 3535cbe
update tests to make sure the sanatize output for windows and force T…
GustavoCaso 60abf26
re-re-enable e2e tests
GustavoCaso e9e2919
handle windows new lines for the error case
GustavoCaso File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
126 changes: 126 additions & 0 deletions
126
comp/process/status/statusimpl/fixtures/json_response.tmpl
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,126 @@ | ||
{ | ||
"core": { | ||
"build_arch": "arm64", | ||
"config": { | ||
"log_level": "info" | ||
}, | ||
"go_version": "go1.21.5", | ||
"metadata": { | ||
"agent-flavor": "process_agent", | ||
"host-tags": { | ||
"system": [] | ||
}, | ||
"install-method": { | ||
"installer_version": null, | ||
"tool": null, | ||
"tool_version": "undefined" | ||
}, | ||
"logs": { | ||
"auto_multi_line_detection_enabled": false, | ||
"transport": "" | ||
}, | ||
"meta": { | ||
"ec2-hostname": "", | ||
"host_aliases": [], | ||
"hostname": "COMP-VQHPF4W6GY", | ||
"instance-id": "", | ||
"socket-fqdn": "COMP-VQHPF4W6GY", | ||
"socket-hostname": "COMP-VQHPF4W6GY", | ||
"timezones": [ | ||
"CET" | ||
] | ||
}, | ||
"network": null, | ||
"os": "darwin", | ||
"otlp": { | ||
"enabled": false | ||
}, | ||
"proxy-info": { | ||
"no-proxy-nonexact-match": false, | ||
"no-proxy-nonexact-match-explicitly-set": false, | ||
"proxy-behavior-changed": false | ||
}, | ||
"python": "n/a", | ||
"systemStats": { | ||
"cpuCores": 10, | ||
"fbsdV": [ | ||
"", | ||
"", | ||
"" | ||
], | ||
"macV": [ | ||
"14.2.1", | ||
[ | ||
"", | ||
"", | ||
"" | ||
], | ||
"arm64" | ||
], | ||
"machine": "arm64", | ||
"nixV": [ | ||
"", | ||
"", | ||
"" | ||
], | ||
"platform": "darwin", | ||
"processor": "Apple M1 Max", | ||
"pythonV": "n/a", | ||
"winV": [ | ||
"", | ||
"", | ||
"" | ||
] | ||
} | ||
}, | ||
"version": "7.51.0-rc.1+git.416.0d1edc1" | ||
}, | ||
"date": 1706892483712089000, | ||
"expvars": { | ||
"connections_queue_bytes": 0, | ||
"connections_queue_size": 0, | ||
"container_count": 0, | ||
"container_id": "", | ||
"docker_socket": "/var/run/docker.sock", | ||
"drop_check_payloads": [], | ||
"enabled_checks": [ | ||
"process", | ||
"rtprocess" | ||
], | ||
"endpoints": { | ||
"https://process.datadoghq.eu": [ | ||
"72724" | ||
] | ||
}, | ||
"event_queue_bytes": 0, | ||
"event_queue_size": 0, | ||
"language_detection_enabled": false, | ||
"last_collect_time": "2024-02-02 17:47:57", | ||
"log_file": "", | ||
"memstats": { | ||
"alloc": 30387880 | ||
}, | ||
"pid": 72211, | ||
"pod_queue_bytes": 0, | ||
"pod_queue_size": 0, | ||
"process_count": 757, | ||
"process_queue_bytes": 0, | ||
"process_queue_size": 0, | ||
"proxy_url": "", | ||
"rtprocess_queue_bytes": 0, | ||
"rtprocess_queue_size": 0, | ||
"system_probe_process_module_enabled": false, | ||
"uptime": 18, | ||
"uptime_nano": 1706892464835469000, | ||
"version": { | ||
"BuildDate": "", | ||
"GitBranch": "", | ||
"GitCommit": "", | ||
"GoVersion": "", | ||
"Version": "" | ||
}, | ||
"workloadmeta_extractor_cache_size": 0, | ||
"workloadmeta_extractor_diffs_dropped": 0, | ||
"workloadmeta_extractor_stale_diffs": 0 | ||
} | ||
} |
2 changes: 2 additions & 0 deletions
2
comp/process/status/statusimpl/fixtures/text_error_response.tmpl
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
|
||
Status: Not running or unreachable |
48 changes: 48 additions & 0 deletions
48
comp/process/status/statusimpl/fixtures/text_response.tmpl
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,48 @@ | ||
|
||
Version: 7.51.0-rc.1+git.416.0d1edc1 | ||
Status date: 2024-02-02 16:48:03.712 UTC (1706892483712) | ||
Process Agent Start: 2024-02-02 16:47:44.835 UTC (1706892464835) | ||
Pid: 72211 | ||
Go Version: go1.21.5 | ||
Build arch: arm64 | ||
Log Level: info | ||
Enabled Checks: [process rtprocess] | ||
Allocated Memory: 30,387,880 bytes | ||
Hostname: COMP-VQHPF4W6GY | ||
System Probe Process Module Status: Not running | ||
Process Language Detection Enabled: False | ||
|
||
================= | ||
Process Endpoints | ||
================= | ||
https://process.datadoghq.eu - API Key ending with: | ||
- 72724 | ||
|
||
========= | ||
Collector | ||
========= | ||
Last collection time: 2024-02-02 17:47:57 | ||
Docker socket: /var/run/docker.sock | ||
Number of processes: 757 | ||
Number of containers: 0 | ||
Process Queue length: 0 | ||
RTProcess Queue length: 0 | ||
Connections Queue length: 0 | ||
Event Queue length: 0 | ||
Pod Queue length: 0 | ||
Process Bytes enqueued: 0 | ||
RTProcess Bytes enqueued: 0 | ||
Connections Bytes enqueued: 0 | ||
Event Bytes enqueued: 0 | ||
Pod Bytes enqueued: 0 | ||
Drop Check Payloads: [] | ||
|
||
========== | ||
Extractors | ||
========== | ||
|
||
Workloadmeta | ||
============ | ||
Cache size: 0 | ||
Stale diffs discarded: 0 | ||
Diffs dropped: 0 |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
2 changes: 0 additions & 2 deletions
2
comp/process/status/statusimpl/status_templates/processagent.tmpl
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,15 +7,54 @@ package statusimpl | |
|
||
import ( | ||
"bytes" | ||
"embed" | ||
"net/http" | ||
"net/http/httptest" | ||
"os" | ||
"strings" | ||
"testing" | ||
|
||
"github.com/stretchr/testify/assert" | ||
"github.com/stretchr/testify/require" | ||
) | ||
|
||
func TestStatusOut(t *testing.T) { | ||
provides := newStatus() | ||
//go:embed fixtures | ||
var fixturesTemplates embed.FS | ||
|
||
headerProvider := provides.StatusProvider.Provider | ||
func fakeStatusServer(t *testing.T, errCode int, response []byte) *httptest.Server { | ||
handler := func(w http.ResponseWriter, r *http.Request) { | ||
defer r.Body.Close() | ||
|
||
if errCode != 200 { | ||
http.NotFound(w, r) | ||
} else { | ||
_, err := w.Write(response) | ||
require.NoError(t, err) | ||
} | ||
} | ||
|
||
return httptest.NewServer(http.HandlerFunc(handler)) | ||
} | ||
|
||
func TestStatus(t *testing.T) { | ||
originalTZ := os.Getenv("TZ") | ||
os.Setenv("TZ", "UTC") | ||
defer func() { | ||
os.Setenv("TZ", originalTZ) | ||
}() | ||
|
||
jsonBytes, err := fixturesTemplates.ReadFile("fixtures/json_response.tmpl") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💬 suggestion //go:embed fixtures/json_response.json
jsonResponse string // or []bytes if you prefer to have it in bytes It saves you from an error checking and the double There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice 😄 |
||
assert.NoError(t, err) | ||
|
||
textResponse, err := fixturesTemplates.ReadFile("fixtures/text_response.tmpl") | ||
assert.NoError(t, err) | ||
|
||
server := fakeStatusServer(t, 200, jsonBytes) | ||
defer server.Close() | ||
|
||
headerProvider := statusProvider{ | ||
testServerURL: server.URL, | ||
} | ||
|
||
tests := []struct { | ||
name string | ||
|
@@ -24,16 +63,25 @@ func TestStatusOut(t *testing.T) { | |
{"JSON", func(t *testing.T) { | ||
stats := make(map[string]interface{}) | ||
headerProvider.JSON(false, stats) | ||
processStats := stats["processAgentStatus"] | ||
|
||
val, ok := processStats.(map[string]interface{}) | ||
assert.True(t, ok) | ||
|
||
assert.NotEmpty(t, stats) | ||
assert.NotEmpty(t, val["core"]) | ||
assert.Empty(t, val["error"]) | ||
}}, | ||
{"Text", func(t *testing.T) { | ||
b := new(bytes.Buffer) | ||
err := headerProvider.Text(false, b) | ||
|
||
assert.NoError(t, err) | ||
|
||
assert.NotEmpty(t, b.String()) | ||
// We replace windows line break by linux so the tests pass on every OS | ||
expected := strings.Replace(string(textResponse), "\r\n", "\n", -1) | ||
output := strings.Replace(b.String(), "\r\n", "\n", -1) | ||
|
||
assert.Equal(t, expected, output) | ||
}}, | ||
{"HTML", func(t *testing.T) { | ||
b := new(bytes.Buffer) | ||
|
@@ -51,3 +99,55 @@ func TestStatusOut(t *testing.T) { | |
}) | ||
} | ||
} | ||
|
||
func TestStatusError(t *testing.T) { | ||
server := fakeStatusServer(t, 500, []byte{}) | ||
defer server.Close() | ||
|
||
originalTZ := os.Getenv("TZ") | ||
os.Setenv("TZ", "UTC") | ||
defer func() { | ||
os.Setenv("TZ", originalTZ) | ||
}() | ||
|
||
errorResponse, err := fixturesTemplates.ReadFile("fixtures/text_error_response.tmpl") | ||
assert.NoError(t, err) | ||
|
||
headerProvider := statusProvider{ | ||
testServerURL: server.URL, | ||
} | ||
|
||
tests := []struct { | ||
name string | ||
assertFunc func(t *testing.T) | ||
}{ | ||
{"JSON", func(t *testing.T) { | ||
stats := make(map[string]interface{}) | ||
headerProvider.JSON(false, stats) | ||
processStats := stats["processAgentStatus"] | ||
|
||
val, ok := processStats.(map[string]interface{}) | ||
assert.True(t, ok) | ||
|
||
assert.NotEmpty(t, val["error"]) | ||
}}, | ||
{"Text", func(t *testing.T) { | ||
b := new(bytes.Buffer) | ||
err := headerProvider.Text(false, b) | ||
|
||
assert.NoError(t, err) | ||
|
||
// We replace windows line break by linux so the tests pass on every OS | ||
expected := strings.Replace(string(errorResponse), "\r\n", "\n", -1) | ||
output := strings.Replace(b.String(), "\r\n", "\n", -1) | ||
|
||
assert.Equal(t, expected, output) | ||
}}, | ||
} | ||
|
||
for _, test := range tests { | ||
t.Run(test.name, func(t *testing.T) { | ||
test.assertFunc(t) | ||
}) | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💭 thought
Wonder if you could mock the
config.GetProcessAPIAddressPort
instead of setting thetestServerURL
. I'd rather use an interface and mock it in test.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is a totally valid point. I would like to do that in a separate PR, as
GetProcessAPIAddressPort
is used in multiple places and I don't want to change the focus of the fix in the PR 😄There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh I see config is global, ok then !