Stop writing channel logs to database and add is_error data field

Author: rowanseymour

core/models/call.go

@@ -364,8 +364,6 @@ func (c *Call) AttachLog(ctx context.Context, db DBorTx, clog *ChannelLog) error
 	if err != nil {
 		return fmt.Errorf("error attaching log to call: %w", err)
 	}
-
-	clog.attached = true
 	return nil
 }
 

core/models/channel_log.go

@@ -2,7 +2,6 @@ package models
 
 import (
 	"context"
-	"encoding/json"
 	"fmt"
 	"log/slog"
 	"slices"
@@ -13,14 +12,10 @@ import (
 	"github.com/aws/aws-sdk-go-v2/service/dynamodb/types"
 	"github.com/nyaruka/gocommon/aws/dynamo"
 	"github.com/nyaruka/gocommon/httpx"
-	"github.com/nyaruka/gocommon/jsonx"
 	"github.com/nyaruka/mailroom/runtime"
 	"github.com/nyaruka/mailroom/utils/clogs"
 )
 
-// ChannelLogID is our type for a channel log id
-type ChannelLogID int64
-
 const (
 	ChannelLogTypeIVRStart    clogs.Type = "ivr_start"
 	ChannelLogTypeIVRIncoming clogs.Type = "ivr_incoming"
@@ -35,8 +30,7 @@ const (
 type ChannelLog struct {
 	*clogs.Log
 
-	channel  *Channel
-	attached bool
+	channel *Channel
 }
 
 // NewChannelLog creates a new channel log with the given type and channel
@@ -56,21 +50,6 @@ func newChannelLog(t clogs.Type, ch *Channel, r *httpx.Recorder, redactVals []st
 	}
 }
 
-// if we have an error or a non 2XX/3XX http response then log is considered an error
-func (l *ChannelLog) isError() bool {
-	if len(l.Errors) > 0 {
-		return true
-	}
-
-	for _, l := range l.HttpLogs {
-		if l.StatusCode < 200 || l.StatusCode >= 400 {
-			return true
-		}
-	}
-
-	return false
-}
-
 func (l *ChannelLog) DynamoKey() runtime.DynamoKey {
 	pk := fmt.Sprintf("cha#%s#%s", l.channel.UUID(), l.UUID[35:36]) // 16 buckets for each channel
 	sk := fmt.Sprintf("log#%s", l.UUID)
@@ -96,29 +75,12 @@ func (l *ChannelLog) DynamoItem() (*runtime.DynamoItem, error) {
 			"type":       l.Type,
 			"elapsed_ms": int(l.Elapsed / time.Millisecond),
 			"created_on": l.CreatedOn,
+			"is_error":   l.IsError(),
 		},
 		DataGZ: dataGZ,
 	}, nil
 }
 
-const sqlInsertChannelLog = `
-INSERT INTO channels_channellog( uuid,  channel_id,  log_type,  http_logs,  errors,  is_error,  elapsed_ms,  created_on)
-                         VALUES(:uuid, :channel_id, :log_type, :http_logs, :errors, :is_error, :elapsed_ms, :created_on)
-  RETURNING id`
-
-// channel log to be inserted into the database
-type dbChannelLog struct {
-	ID        ChannelLogID    `db:"id"`
-	UUID      clogs.UUID      `db:"uuid"`
-	ChannelID ChannelID       `db:"channel_id"`
-	Type      clogs.Type      `db:"log_type"`
-	HTTPLogs  json.RawMessage `db:"http_logs"`
-	Errors    json.RawMessage `db:"errors"`
-	IsError   bool            `db:"is_error"`
-	ElapsedMS int             `db:"elapsed_ms"`
-	CreatedOn time.Time       `db:"created_on"`
-}
-
 // InsertChannelLogs writes the given channel logs to the db
 func InsertChannelLogs(ctx context.Context, rt *runtime.Runtime, logs []*ChannelLog) error {
 	// write all logs to DynamoDB
@@ -150,30 +112,5 @@ func InsertChannelLogs(ctx context.Context, rt *runtime.Runtime, logs []*Channel
 		}
 	}
 
-	unattached := make([]*dbChannelLog, 0, len(logs))
-
-	for _, l := range logs {
-		if !l.attached {
-			// if log isn't attached to a message or call we need to write it to the db so that it's retrievable
-			unattached = append(unattached, &dbChannelLog{
-				UUID:      l.UUID,
-				ChannelID: l.channel.ID(),
-				Type:      l.Type,
-				HTTPLogs:  jsonx.MustMarshal(l.HttpLogs),
-				Errors:    jsonx.MustMarshal(l.Errors),
-				IsError:   l.isError(),
-				CreatedOn: l.CreatedOn,
-				ElapsedMS: int(l.Elapsed / time.Millisecond),
-			})
-		}
-	}
-
-	if len(unattached) > 0 {
-		err := BulkQuery(ctx, "insert channel log", rt.DB, sqlInsertChannelLog, unattached)
-		if err != nil {
-			return fmt.Errorf("error inserting unattached channel logs: %w", err)
-		}
-	}
-
 	return nil
 }

core/models/channel_log_test.go

@@ -4,7 +4,7 @@ import (
 	"net/http"
 	"testing"
 
-	"github.com/nyaruka/gocommon/dbutil/assertdb"
+	"github.com/nyaruka/gocommon/aws/dynamo"
 	"github.com/nyaruka/gocommon/httpx"
 	"github.com/nyaruka/mailroom/core/models"
 	"github.com/nyaruka/mailroom/testsuite"
@@ -52,13 +52,15 @@ func TestChannelLogsOutgoing(t *testing.T) {
 	err = models.InsertChannelLogs(ctx, rt, []*models.ChannelLog{clog1, clog2})
 	require.NoError(t, err)
 
-	assertdb.Query(t, rt.DB, `SELECT count(*) FROM channels_channellog`).Returns(2)
-	assertdb.Query(t, rt.DB, `SELECT count(*) FROM channels_channellog WHERE log_type = 'ivr_start' AND http_logs -> 0 ->> 'url' = 'http://ivr.com/start' AND is_error = FALSE AND channel_id = $1`, channel.ID()).Returns(1)
-	assertdb.Query(t, rt.DB, `SELECT count(*) FROM channels_channellog WHERE log_type = 'ivr_hangup' AND http_logs -> 0 ->> 'url' = 'http://ivr.com/hangup' AND is_error = TRUE AND channel_id = $1`, channel.ID()).Returns(1)
-	assertdb.Query(t, rt.DB, `SELECT count(*) FROM channels_channellog WHERE http_logs::text LIKE '%sesame%'`).Returns(0)
-
 	// read log back from DynamoDB
 	item, err := rt.Dynamo.Main.GetItem(ctx, clog1.DynamoKey())
 	require.NoError(t, err)
 	assert.Equal(t, string(models.ChannelLogTypeIVRStart), item.Data["type"])
+
+	var dataGZ map[string]any
+	err = dynamo.UnmarshalJSONGZ(item.DataGZ, &dataGZ)
+	require.NoError(t, err)
+	assert.Len(t, dataGZ["http_logs"], 1)
+
+	assert.NotContains(t, string(item.DataGZ), "sesame", "redacted value should not be present in DynamoDB log")
 }

utils/clogs/clog.go

@@ -76,6 +76,21 @@ func (l *Log) End() {
 	l.Elapsed = time.Since(l.CreatedOn)
 }
 
+// if we have an error or a non 2XX/3XX http response then log is considered an error
+func (l *Log) IsError() bool {
+	if len(l.Errors) > 0 {
+		return true
+	}
+
+	for _, l := range l.HttpLogs {
+		if l.StatusCode < 200 || l.StatusCode >= 400 {
+			return true
+		}
+	}
+
+	return false
+}
+
 func (l *Log) traceToLog(t *httpx.Trace) *httpx.Log {
 	return httpx.NewLog(t, 2048, 50000, l.redactor)
 }

web/ivr/ivr_test.go

@@ -638,10 +638,6 @@ func TestVonageIVR(t *testing.T) {
 	for _, log := range logs {
 		assert.NotContains(t, string(jsonx.MustMarshal(log)), "BEGIN PRIVATE KEY") // private key redacted
 	}
-
-	// and 2 unattached logs in the database
-	assertdb.Query(t, rt.DB, `SELECT count(*) FROM channels_channellog WHERE channel_id = $1`, testdb.VonageChannel.ID).Returns(2)
-	assertdb.Query(t, rt.DB, `SELECT array_agg(log_type ORDER BY id) FROM channels_channellog WHERE channel_id = $1`, testdb.VonageChannel.ID).Returns([]byte(`{ivr_status,ivr_status}`))
 }
 
 func getCallLogs(t *testing.T, ctx context.Context, rt *runtime.Runtime, ch *testdb.Channel) []*httpx.Log {