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

Implement new directive for performance profiling #192

Closed
syinwu opened this issue Mar 3, 2022 · 10 comments
Closed

Implement new directive for performance profiling #192

syinwu opened this issue Mar 3, 2022 · 10 comments

Comments

@syinwu
Copy link
Member

syinwu commented Mar 3, 2022

Summary

I think we need to add some hooks for the rule.

Basic example

Not yet.

Motivation

If we have some hooks of the rule, we can add some custom behaviors before and after rule execution. For example, collect the execution time of each rule(because our system performance requirements are high, it is necessary to do some observable work).

@syinwu syinwu added the enhancement New feature or request label Mar 3, 2022
@jptosso
Copy link
Member

jptosso commented Mar 3, 2022

@fzipi was motivated with this last year, I think we should think about a profiling engine for Coraza capable of creating a detailed report of the performance of the rule. There might be a way to attach ourselves to the profiling API and hook some "tags" https://pkg.go.dev/runtime/pprof, splitting the data by rule. That way we not only get the milliseconds performance per rule, but we will get the memory allocation, processing units, and more.

@jptosso
Copy link
Member

jptosso commented Mar 7, 2022

I was reviewing pprof documentation and it can be done, we can add a special flag to the Transaction instance like

type Transaction struct {
...
// ProfilingStrategy is the strategy used to create profile files
ProfilingStrategy types.ProfilingStrategy

// ProfilingDirectory is the directory where profiling files will be stored
ProfilingDirectory string
}
type ProfilingStrategy int
const (
// No profiling
ProfilingStrategyNone ProfilingStrategy = iota
// One profile will be created per transaction "txid.dump"
ProfilingStrategyFullTransaction
// One profile will be created by transaction and phase
ProfilingStrategyTransactionPhases = iota
// One profile will be created by rule
ProfilingStrategyByRule = iota
)

Something like this, I will create some tests because it seems super cool.

We could also use a contextual seclang approach like

SecProfilingEngine On
SecProfilingPath /some/path

SecRule REMOTE_ADDR "127.0.0.1" "id:1000,phase:1,profiling:start=phase-1"
SecRule ....
SecRule PROFILING "1" "id:1001,phase:1,profiling:stop=phase-1"
# This will store the profile to /some/path/phase-1.dump
# profiling supports macro expansion, you can use timestamps as profile name

Once the profiling files are created we can use pprof tool to build complex performance models like this:
image

@fzipi
Copy link
Member

fzipi commented Mar 7, 2022

Sounds awesome!

Maybe makes sense also to have a SecProfilingEngine <On|Off>?

@jptosso
Copy link
Member

jptosso commented Mar 8, 2022

Sounds awesome!

Maybe makes sense also to have a SecProfilingEngine <On|Off>?

It sounds consistent with other directives like SecAuditEngine and SecRuleEngine.

@syinwu syinwu changed the title Add some hooks for Rule Implement new directive for performance profiling Mar 8, 2022
@jptosso jptosso added roadmap and removed enhancement New feature or request labels Mar 30, 2022
@syinwu syinwu self-assigned this Mar 31, 2022
@github-actions
Copy link

github-actions bot commented May 1, 2022

This issue is stale because it has been open for 30 days with no activity.

@github-actions github-actions bot added the stale label May 1, 2022
@syinwu syinwu removed the stale label May 1, 2022
@syinwu
Copy link
Member Author

syinwu commented May 2, 2022

This is still happening.

@github-actions
Copy link

github-actions bot commented Jun 2, 2022

This issue is stale because it has been open for 30 days with no activity.

@github-actions github-actions bot added the stale label Jun 2, 2022
@github-actions
Copy link

This issue was closed because it has been inactive for 14 days since being marked as stale.

@syinwu syinwu removed the stale label Jun 17, 2022
@syinwu syinwu reopened this Jun 17, 2022
@jptosso jptosso added this to the v3 alpha milestone Jul 11, 2022
@github-actions
Copy link

This issue is stale because it has been open for 30 days with no activity.

@github-actions
Copy link

github-actions bot commented Sep 1, 2022

This issue was closed because it has been inactive for 14 days since being marked as stale.

@github-actions github-actions bot closed this as completed Sep 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants