Skip to content
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(server): re-enable prometheus counters. #3304

Merged
merged 9 commits into from
Jul 18, 2023

Conversation

rafaeldtinoco
Copy link
Contributor

Commit: 0213c08 ("tracee: add ready callback") made prometheus stats to stop being registered due to a timing issue. This patch re-enables it by registering prometheus counters in the callback function itself instead of in Tracee.New() (too early).

Fixes: #3303

Still checking more stuff.

@geyslan
Copy link
Member

geyslan commented Jul 11, 2023

Tested and prometheus counters are working again. 👍🏼

I'll redo the panels created in #3297 on top of this working PR and raise the dashboard related one.

image

@geyslan
Copy link
Member

geyslan commented Jul 11, 2023

I'll redo the panels created in #3297 on top of this working PR and raise the dashboard related one.

Ready: https://github.com/rafaeldtinoco/tracee/pull/7

@rafaeldtinoco rafaeldtinoco marked this pull request as ready for review July 12, 2023 13:40
@rafaeldtinoco
Copy link
Contributor Author

Im adding an integration test to test if metrics are enabled and this will be ready.

@rafaeldtinoco
Copy link
Contributor Author

rafaeldtinoco commented Jul 12, 2023

So, if the "test-performance" target does not run (it has a skip if the user can't execute anything using "sudo -n" (with no password)) then I'll have to run the performance tests in a jenkins runner or change the runner config a bit. Lets see.

EDIT:

image

Test passes in regular github runner. It starts tracee using sudo and makes sure it ends at the end of the test as you can see in the code.

@rafaeldtinoco rafaeldtinoco added this to the v0.17.0 milestone Jul 12, 2023
@yanivagman yanivagman removed this from the v0.17.0 milestone Jul 13, 2023
Copy link
Member

@geyslan geyslan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

tests/testutils/tracee.go Outdated Show resolved Hide resolved
rafaeldtinoco and others added 9 commits July 18, 2023 19:42
Commit: 0213c08 ("tracee: add ready callback") made prometheus stats to
stop being registered due to a timing issue. This patch re-enables it
by registering prometheus counters in the callback function itself
instead of in Tracee.New() (too early).

Fixes: #3303
- create testutils/tracee to start/stop tracee using sudo
- test if --healthz works
- test if --metrics works
- test if --pprof works
- test if metrics are enabled on each PR
This brings two new panels to the dashboard:

- Heap
- GC

And makes some minor changes to the existing panels.
@rafaeldtinoco rafaeldtinoco merged commit bf4d762 into aquasecurity:main Jul 18, 2023
26 checks passed
@rafaeldtinoco rafaeldtinoco deleted the perf-fixes branch July 18, 2023 23:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug: metrics cmdline does not enable metrics
3 participants