internal/appsec: rework appsec telemetry #3345
Conversation
Datadog ReportBranch report: ✅ 0 Failed, 4472 Passed, 65 Skipped, 3m 37.58s Total Time |
BenchmarksBenchmark execution time: 2025-04-10 09:03:21 Comparing candidate commit 83a543f in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 55 metrics, 1 unstable metrics. |
eliottness
force-pushed
the
eliottness/more-lazy-metrics
branch
3 times, most recently
from
5c6ec98 to
57ea557
Compare
Signed-off-by: Eliott Bouhana <[email protected]>
Signed-off-by: Eliott Bouhana <[email protected]>
Signed-off-by: Eliott Bouhana <[email protected]>
eliottness
force-pushed
the
eliottness/more-lazy-metrics
branch
from
57ea557 to
a3b5339
Compare
| RASPRuleTypeSSRF RASPRuleType = "ssrf" | ||
| RASPRuleTypeSQLI RASPRuleType = "sql_injection" | ||
| RASPRuleTypeCMDI RASPRuleType = "command_injection" | ||
| RASPRuleTypeLFI RASPRuleType = iota |
There was a problem hiding this comment.
Suggest making the 0-value invalid:
| RASPRuleTypeLFI RASPRuleType = iota | |
| _ RASPRuleType = iota | |
| RASPRuleTypeLFI |
There was a problem hiding this comment.
This would make my for loops wrong because I loop on them like a dictionnary but they are arrays 😭
There was a problem hiding this comment.
Like I am doing here with explicit index array placement
| } | ||
|
|
||
| // RASPRuleTypeFromAddressSet returns the RASPRuleType for the given address set if it has a RASP address. | ||
| func RASPRuleTypeFromAddressSet(addressSet waf.RunAddressData) (RASPRuleType, bool) { | ||
| if addressSet.Scope != waf.RASPScope { | ||
| return "", false | ||
| return math.MaxUint8, false |
There was a problem hiding this comment.
If the 0-value is invalid, you could return 0 instead, which looks less weird?
| @@ -46,5 +60,5 @@ func RASPRuleTypeFromAddressSet(addressSet waf.RunAddressData) (RASPRuleType, bo | |||
| } | |||
| } | |||
|
|
|||
| return "", false | |||
| return math.MaxUint8, false | |||
There was a problem hiding this comment.
If the 0-value is invalid, you could return 0 instead, which looks less weird?
| @@ -24,6 +24,8 @@ import ( | |||
| "github.com/DataDog/appsec-internal-go/apisec" | |||
| "github.com/stretchr/testify/assert" | |||
|
|
|||
| "github.com/stretchr/testify/mock" | |||
There was a problem hiding this comment.
Moving this seems unrelated... And the blank spacing there is odd... I'd normalize it or not touch it...
There was a problem hiding this comment.
This was indeed wrong regarding import order so I fixed it even more
Signed-off-by: Eliott Bouhana <[email protected]>
Signed-off-by: Eliott Bouhana <[email protected]>
What does this PR do?
rasp.timeoutmetric.RASPRuleTypetype from string to uint8 to make arrays of static size and use them as a dictionnarywaf.HandleMetricsreplace a lot of makes with keys beingRASPRuleTypewith an arrayxsync.MapOfconfig.Origintype and replace it by the more general purposetelemetry.Origin(That I am thinking of promoting to the packageinternal/globalconfig🤔 )Motivation
Reviewer's Checklist
v2-devbranch and reviewed by @DataDog/apm-go.Unsure? Have a question? Request a review!