fix: put back duration_ext metric as the top-level timer (#85)

- [x] Setup timers to be retro-compatible with the old metrics
- [x] Add a decode metric

Signed-off-by: Eliott Bouhana <[email protected]>

Author: eliottness

context.go

@@ -117,9 +117,8 @@ func (context *Context) Run(addressData RunAddressData, _ time.Duration) (res Re
 
 	runTimer, err := context.timer.NewNode(wafRunTag,
 		timer.WithComponents(
-			wafPersistentEncoderTag,
-			wafEphemeralEncoderTag,
-			wafDurationExtTag,
+			wafEncodeTag,
+			wafDecodeTag,
 			wafDurationTag,
 		),
 	)
@@ -129,22 +128,28 @@ func (context *Context) Run(addressData RunAddressData, _ time.Duration) (res Re
 
 	runTimer.Start()
 	defer func() {
-		context.metrics.add("_dd.appsec.waf.run", runTimer.Stop())
+		context.metrics.add(wafRunTag, runTimer.Stop())
 		context.metrics.merge(runTimer.Stats())
 	}()
 
-	persistentData, persistentEncoder, err := context.encodeOneAddressType(addressData.Persistent, runTimer.MustLeaf(wafPersistentEncoderTag))
+	wafEncodeTimer := runTimer.MustLeaf(wafEncodeTag)
+	wafEncodeTimer.Start()
+	persistentData, persistentEncoder, err := context.encodeOneAddressType(addressData.Persistent, wafEncodeTimer)
 	if err != nil {
+		wafEncodeTimer.Stop()
 		return res, err
 	}
 
 	// The WAF releases ephemeral address data at the max of each run call, so we need not keep the Go values live beyond
 	// that in the same way we need for persistent data. We hence use a separate encoder.
-	ephemeralData, ephemeralEncoder, err := context.encodeOneAddressType(addressData.Ephemeral, runTimer.MustLeaf(wafEphemeralEncoderTag))
+	ephemeralData, ephemeralEncoder, err := context.encodeOneAddressType(addressData.Ephemeral, wafEncodeTimer)
 	if err != nil {
+		wafEncodeTimer.Stop()
 		return res, err
 	}
 
+	wafEncodeTimer.Stop()
+
 	// ddwaf_run cannot run concurrently and we are going to mutate the context.cgoRefs, so we need to lock the context
 	context.mutex.Lock()
 	defer context.mutex.Unlock()
@@ -157,8 +162,8 @@ func (context *Context) Run(addressData RunAddressData, _ time.Duration) (res Re
 	// into C ddwaf_objects. libddwaf's API requires to keep this data for the lifetime of the ddwaf_context.
 	defer context.cgoRefs.append(persistentEncoder.cgoRefs)
 
-	wafExtTimer := runTimer.MustLeaf(wafDurationExtTag)
-	res, err = context.run(persistentData, ephemeralData, wafExtTimer, runTimer.SumRemaining())
+	wafDecodeTimer := runTimer.MustLeaf(wafDecodeTag)
+	res, err = context.run(persistentData, ephemeralData, wafDecodeTimer, runTimer.SumRemaining())
 
 	runTimer.AddTime(wafDurationTag, res.TimeSpent)
 
@@ -207,15 +212,15 @@ func merge[K comparable, V any](a, b map[K][]V) (merged map[K][]V) {
 // If the addressData is empty, it returns nil for the WAF object and an empty ref pool.
 // At this point, if the encoder does not timeout, the only error we can get is an error in case the top level object
 // is a nil map, but this  behaviour is expected since either persistent or ephemeral addresses are allowed to be null
-// one at a time. In this case, EncodeAddresses will return nil contrary to Encode which will return a nil wafObject,
+// one at a time. In this case, Encode will return nil contrary to Encode which will return a nil wafObject,
 // which is what we need to send to ddwaf_run to signal that the address data is empty.
 func (context *Context) encodeOneAddressType(addressData map[string]any, timer timer.Timer) (*bindings.WafObject, encoder, error) {
 	encoder := newLimitedEncoder(timer)
 	if addressData == nil {
 		return nil, encoder, nil
 	}
 
-	data, _ := encoder.EncodeAddresses(addressData)
+	data, _ := encoder.Encode(addressData)
 	if len(encoder.truncations) > 0 {
 		context.mutex.Lock()
 		defer context.mutex.Unlock()
@@ -232,18 +237,18 @@ func (context *Context) encodeOneAddressType(addressData map[string]any, timer t
 
 // run executes the ddwaf_run call with the provided data on this context. The caller is responsible for locking the
 // context appropriately around this call.
-func (context *Context) run(persistentData, ephemeralData *bindings.WafObject, wafTimer timer.Timer, timeBudget time.Duration) (Result, error) {
+func (context *Context) run(persistentData, ephemeralData *bindings.WafObject, wafDecodeTimer timer.Timer, timeBudget time.Duration) (Result, error) {
 	result := new(bindings.WafResult)
 	defer wafLib.WafResultFree(result)
 
-	wafTimer.Start()
-	defer wafTimer.Stop()
-
 	// The value of the timeout cannot exceed 2^55
 	// cf. https://en.cppreference.com/w/cpp/chrono/duration
 	timeout := uint64(timeBudget.Microseconds()) & 0x008FFFFFFFFFFFFF
 	ret := wafLib.WafRun(context.cContext, persistentData, ephemeralData, result, timeout)
 
+	wafDecodeTimer.Start()
+	defer wafDecodeTimer.Stop()
+
 	return unwrapWafResult(ret, result)
 }
 

encoder.go

@@ -106,8 +106,6 @@ func (encoder *encoder) Encode(data any) (wo *bindings.WafObject, err error) {
 	value := reflect.ValueOf(data)
 	wo = &bindings.WafObject{}
 
-	encoder.timer.Start()
-	defer encoder.timer.Stop()
 	err = encoder.encode(value, wo, encoder.objectMaxDepth)
 
 	if len(encoder.truncations[ObjectTooDeep]) != 0 && !encoder.timer.Exhausted() {
@@ -125,21 +123,6 @@ func (encoder *encoder) Truncations() map[TruncationReason][]int {
 	return result
 }
 
-// EncodeAddresses takes a map of Go values and returns a wafObject pointer and an error.
-// The returned wafObject is the root of the tree of nested wafObjects representing the Go values.
-// This function is further optimized from Encode to take addresses as input and avoid further
-// errors in case the top-level map with addresses as keys is nil.
-// Since errors returned by Encode are not sent up between levels of the tree, this means that all errors come from the
-// top layer of encoding, which is the map of addresses. Hence, all errors should be developer errors since the map of
-// addresses is not user defined custom data.
-func (encoder *encoder) EncodeAddresses(addresses map[string]any) (*bindings.WafObject, error) {
-	if addresses == nil {
-		return nil, errors.ErrUnsupportedValue
-	}
-
-	return encoder.Encode(addresses)
-}
-
 func encodeNative[T native](val T, t bindings.WafObjectType, obj *bindings.WafObject) {
 	obj.Type = t
 	obj.Value = (uintptr)(val)

metrics.go

@@ -25,13 +25,12 @@ type Stats struct {
 }
 
 const (
-	wafPersistentEncoderTag = "_dd.appsec.waf.encode.persistent"
-	wafEphemeralEncoderTag  = "_dd.appsec.waf.encode.ephemeral"
-	wafRunTag               = "_dd.appsec.waf.run"
-	wafDurationTag          = "_dd.appsec.waf.duration"
-	wafDurationExtTag       = "_dd.appsec.waf.duration_ext"
-	wafTimeoutTag           = "_dd.appsec.waf.timeouts"
-	wafTruncationTag        = "_dd.appsec.waf.truncations"
+	wafEncodeTag     = "_dd.appsec.waf.encode"
+	wafRunTag        = "_dd.appsec.waf.duration_ext"
+	wafDurationTag   = "_dd.appsec.waf.duration"
+	wafDecodeTag     = "_dd.appsec.waf.decode"
+	wafTimeoutTag    = "_dd.appsec.waf.timeouts"
+	wafTruncationTag = "_dd.appsec.waf.truncations"
 )
 
 // Metrics transform the stats returned by the WAF into a map of key value metrics for datadog backend

waf_test.go

@@ -376,9 +376,8 @@ func TestTimeout(t *testing.T) {
 		_, err := context.Run(RunAddressData{Persistent: normalValue, Ephemeral: normalValue}, 0)
 		require.NoError(t, err)
 		require.NotEmpty(t, context.Stats())
-		require.NotZero(t, context.Stats().Timers["_dd.appsec.waf.run"])
-		require.NotZero(t, context.Stats().Timers["_dd.appsec.waf.encode.persistent"])
-		require.NotZero(t, context.Stats().Timers["_dd.appsec.waf.encode.ephemeral"])
+		require.NotZero(t, context.Stats().Timers["_dd.appsec.waf.decode"])
+		require.NotZero(t, context.Stats().Timers["_dd.appsec.waf.encode"])
 		require.NotZero(t, context.Stats().Timers["_dd.appsec.waf.duration_ext"])
 		require.NotZero(t, context.Stats().Timers["_dd.appsec.waf.duration"])
 	})
@@ -390,9 +389,8 @@ func TestTimeout(t *testing.T) {
 
 		_, err := context.Run(RunAddressData{Persistent: largeValue}, 0)
 		require.Equal(t, errors.ErrTimeout, err)
-		require.GreaterOrEqual(t, context.Stats().Timers["_dd.appsec.waf.run"], time.Millisecond)
-		require.GreaterOrEqual(t, context.Stats().Timers["_dd.appsec.waf.encode.persistent"], time.Millisecond)
-		require.Equal(t, context.Stats().Timers["_dd.appsec.waf.encode.ephemeral"], time.Duration(0))
+		require.GreaterOrEqual(t, context.Stats().Timers["_dd.appsec.waf.duration_ext"], time.Millisecond)
+		require.GreaterOrEqual(t, context.Stats().Timers["_dd.appsec.waf.encode"], time.Millisecond)
 	})
 
 	t.Run("timeout-ephemeral-encoder", func(t *testing.T) {
@@ -402,9 +400,8 @@ func TestTimeout(t *testing.T) {
 
 		_, err := context.Run(RunAddressData{Ephemeral: largeValue}, 0)
 		require.Equal(t, errors.ErrTimeout, err)
-		require.GreaterOrEqual(t, context.Stats().Timers["_dd.appsec.waf.run"], time.Millisecond)
-		require.Equal(t, context.Stats().Timers["_dd.appsec.waf.encode.persistent"], time.Duration(0))
-		require.GreaterOrEqual(t, context.Stats().Timers["_dd.appsec.waf.encode.ephemeral"], time.Millisecond)
+		require.GreaterOrEqual(t, context.Stats().Timers["_dd.appsec.waf.duration_ext"], time.Millisecond)
+		require.GreaterOrEqual(t, context.Stats().Timers["_dd.appsec.waf.encode"], time.Millisecond)
 	})
 
 	t.Run("many-runs", func(t *testing.T) {