feat: adds metrics support. #29
Conversation
There was a problem hiding this comment.
Probably should have asked what metrics you're planning sooner 😅
Getting times in wasm is unfortunately fairly expensive so I don't think we can do it by default - one of those cause more harm than help type of situations. The print timings thing is flagged off because of that. Timing per phase is also sort of a performance tuning tool so I don't think it's as important and can be designed in more detail, we should consider adding it to the SDK instead as an extension of tetratelabs/proxy-wasm-go-sdk#327 rather than waf-specific. But if we did want something short-term, it should just be added to the existing function which is flag guarded rather than adding this new wrapper.
The most important metric for WAF seems to be interruption, so I would trim down this PR to just that single metric, it can be added to handleInterruption which can be modified to take a phase parameter if the interruption doesn't already provide that.
|
Do you mind reviewing it again @anuraaga ? |
|
Looks good except for one comment on build file |
Co-authored-by: Anuraag Agrawal <[email protected]>
This PR adds support for metrics. The main challenge for me was to come up with the right design, from named return values and defer to return wrapping methods, this is the best design I could come up.