Skip to content

Commit 46a404c

Browse files
AchoArnoldCopilot
andcommitted
refactor(api): mark spans for sampling without copying logs
Keep standalone log output intact and simplify telemetry conditions and test fixtures. Propagate heartbeat update contexts and defer SQL formatting until a log is emitted. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 60fb2b83-6e9d-4656-9f60-6183c61b716d
1 parent b0f0f22 commit 46a404c

9 files changed

Lines changed: 492 additions & 259 deletions

‎api/pkg/di/container.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -262,9 +262,9 @@ func (container *Container) DedicatedDB() (db *gorm.DB) {
262262

263263
config := &gorm.Config{
264264
TranslateError: true,
265-
Logger: telemetry.NewGormLoggerWithConfig(
265+
Logger: telemetry.NewGormSpanLogger(
266266
container.Tracer(),
267-
container.Logger(6),
267+
gormLogger.Default,
268268
gormLogger.Config{
269269
SlowThreshold: 200 * time.Millisecond,
270270
LogLevel: gormLogger.Warn,

‎api/pkg/repositories/gorm_heartbeat_monitor_repository.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ func (repository *gormHeartbeatMonitorRepository) UpdatePhoneOnline(ctx context.
5353
ctx, cancel := context.WithTimeout(ctx, dbOperationDuration)
5454
defer cancel()
5555

56-
err := repository.db.
56+
err := repository.db.WithContext(ctx).
5757
Model(&entities.HeartbeatMonitor{}).
5858
Where("id = ?", monitorID).
5959
Where("user_id = ?", userID).
@@ -75,7 +75,7 @@ func (repository *gormHeartbeatMonitorRepository) UpdateQueueID(ctx context.Cont
7575
ctx, cancel := context.WithTimeout(ctx, dbOperationDuration)
7676
defer cancel()
7777

78-
err := repository.db.
78+
err := repository.db.WithContext(ctx).
7979
Model(&entities.HeartbeatMonitor{}).
8080
Where("id = ?", monitorID).
8181
Updates(map[string]any{
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
package repositories
2+
3+
import (
4+
"context"
5+
"testing"
6+
7+
"github.com/NdoleStudio/httpsms/pkg/telemetry"
8+
"github.com/google/uuid"
9+
"github.com/stretchr/testify/require"
10+
"gorm.io/driver/postgres"
11+
"gorm.io/gorm"
12+
)
13+
14+
func TestHeartbeatMonitorUpdatesCarryContext(t *testing.T) {
15+
updates := map[string]func(HeartbeatMonitorRepository, context.Context) error{
16+
"phone online": func(repository HeartbeatMonitorRepository, ctx context.Context) error {
17+
return repository.UpdatePhoneOnline(ctx, "user-id", uuid.New(), true)
18+
},
19+
"queue ID": func(repository HeartbeatMonitorRepository, ctx context.Context) error {
20+
return repository.UpdateQueueID(ctx, uuid.New(), "queue-id")
21+
},
22+
}
23+
for name, update := range updates {
24+
t.Run(name, func(t *testing.T) {
25+
db, err := gorm.Open(postgres.New(postgres.Config{Conn: &messageThreadTestConnPool{}}), &gorm.Config{
26+
DisableAutomaticPing: true,
27+
SkipDefaultTransaction: true,
28+
DryRun: true,
29+
})
30+
require.NoError(t, err)
31+
type contextKey struct{}
32+
ctx := context.WithValue(context.Background(), contextKey{}, "request")
33+
called := false
34+
require.NoError(t, db.Callback().Update().Before("gorm:update").Register("check_context", func(db *gorm.DB) {
35+
called = true
36+
require.Equal(t, "request", db.Statement.Context.Value(contextKey{}))
37+
_, hasDeadline := db.Statement.Context.Deadline()
38+
require.True(t, hasDeadline)
39+
}))
40+
logger := &messageThreadTestLogger{}
41+
repository := NewGormHeartbeatMonitorRepository(logger, telemetry.NewOtelLogger("test", logger), db)
42+
43+
require.NoError(t, update(repository, ctx))
44+
require.True(t, called)
45+
})
46+
}
47+
}

‎api/pkg/telemetry/gorm_logger.go‎

Lines changed: 88 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,11 @@ import (
1414
type gormLogger struct {
1515
tracer Tracer
1616
logger Logger
17-
config logger.Config
17+
config gormLogConfig
18+
}
19+
20+
type gormLogConfig struct {
21+
logger.Config
1822
}
1923

2024
// NewGormLogger creates a new instance of gormLogger
@@ -27,7 +31,7 @@ func NewGormLoggerWithConfig(tracer Tracer, log Logger, config logger.Config) lo
2731
return &gormLogger{
2832
tracer: tracer,
2933
logger: log,
30-
config: config,
34+
config: gormLogConfig{Config: config},
3135
}
3236
}
3337

@@ -57,20 +61,96 @@ func (gorm *gormLogger) Error(ctx context.Context, s string, i ...any) {
5761
}
5862

5963
func (gorm *gormLogger) Trace(ctx context.Context, begin time.Time, fc func() (sql string, rowsAffected int64), err error) {
60-
if gorm.config.LogLevel <= logger.Silent {
64+
elapsed := time.Since(begin)
65+
level := gorm.config.queryLevel(elapsed, err)
66+
if level == logger.Silent {
6167
return
6268
}
63-
elapsed := time.Since(begin)
6469
l := gorm.tracer.CtxLogger(gorm.logger, gorm.tracer.Span(ctx)).WithString("latency", elapsed.String())
6570
sql, rows := fc()
6671
msg := fmt.Sprintf("[ROWS:%d][%s]", rows, sql)
6772

68-
switch {
69-
case err != nil && gorm.config.LogLevel >= logger.Error && (!errors.Is(err, gormdb.ErrRecordNotFound) || !gorm.config.IgnoreRecordNotFoundError):
73+
switch level {
74+
case logger.Error:
7075
l.Error(stacktrace.Propagatef(err, "[ROWS:%d][%s]", rows, sql))
71-
case gorm.config.SlowThreshold > 0 && elapsed > gorm.config.SlowThreshold && gorm.config.LogLevel >= logger.Warn:
76+
case logger.Warn:
7277
l.Warn(stacktrace.NewErrorf("SLOW SQL >= %s [ROWS:%d][%s]", gorm.config.SlowThreshold, rows, sql))
73-
case gorm.config.LogLevel >= logger.Info:
78+
default:
7479
l.Debug(msg)
7580
}
7681
}
82+
83+
func (config gormLogConfig) queryLevel(elapsed time.Duration, err error) logger.LogLevel {
84+
if config.LogLevel <= logger.Silent {
85+
return logger.Silent
86+
}
87+
switch {
88+
case config.shouldLogError(err):
89+
return logger.Error
90+
case config.shouldLogSlowQuery(elapsed):
91+
return logger.Warn
92+
case config.LogLevel >= logger.Info:
93+
return logger.Info
94+
default:
95+
return logger.Silent
96+
}
97+
}
98+
99+
func (config gormLogConfig) shouldLogError(err error) bool {
100+
if err == nil || config.LogLevel < logger.Error {
101+
return false
102+
}
103+
return !config.IgnoreRecordNotFoundError || !errors.Is(err, gormdb.ErrRecordNotFound)
104+
}
105+
106+
func (config gormLogConfig) shouldLogSlowQuery(elapsed time.Duration) bool {
107+
return config.SlowThreshold > 0 &&
108+
elapsed > config.SlowThreshold &&
109+
config.LogLevel >= logger.Warn
110+
}
111+
112+
type gormSpanLogger struct {
113+
logger.Interface
114+
tracer Tracer
115+
config gormLogConfig
116+
}
117+
118+
// NewGormSpanLogger marks warning/error spans without changing the existing logger's output.
119+
func NewGormSpanLogger(tracer Tracer, delegate logger.Interface, config logger.Config) logger.Interface {
120+
return &gormSpanLogger{
121+
Interface: delegate,
122+
tracer: tracer,
123+
config: gormLogConfig{Config: config},
124+
}
125+
}
126+
127+
func (gorm *gormSpanLogger) LogMode(level logger.LogLevel) logger.Interface {
128+
clone := *gorm
129+
clone.Interface = gorm.Interface.LogMode(level)
130+
clone.config.LogLevel = level
131+
return &clone
132+
}
133+
134+
func (gorm *gormSpanLogger) Warn(ctx context.Context, message string, args ...any) {
135+
if gorm.config.LogLevel >= logger.Warn {
136+
gorm.tracer.Span(ctx).SetAttributes(mustSampleAttribute.Bool(true))
137+
}
138+
gorm.Interface.Warn(ctx, message, args...)
139+
}
140+
141+
func (gorm *gormSpanLogger) Error(ctx context.Context, message string, args ...any) {
142+
if gorm.config.LogLevel >= logger.Error {
143+
gorm.tracer.Span(ctx).SetAttributes(mustSampleAttribute.Bool(true))
144+
}
145+
gorm.Interface.Error(ctx, message, args...)
146+
}
147+
148+
func (gorm *gormSpanLogger) Trace(ctx context.Context, begin time.Time, fc func() (string, int64), err error) {
149+
gorm.Interface.Trace(ctx, begin, func() (string, int64) {
150+
switch gorm.config.queryLevel(time.Since(begin), err) {
151+
case logger.Error, logger.Warn:
152+
gorm.tracer.Span(ctx).SetAttributes(mustSampleAttribute.Bool(true))
153+
}
154+
return fc()
155+
}, err)
156+
}

‎api/pkg/telemetry/gorm_logger_test.go‎

Lines changed: 94 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"context"
66
"encoding/json"
77
"errors"
8+
stdlog "log"
89
"testing"
910
"time"
1011

@@ -40,26 +41,103 @@ func TestGormWarningLogger(t *testing.T) {
4041
SlowThreshold: 200 * time.Millisecond,
4142
LogLevel: gormlogger.Warn,
4243
}).LogMode(test.logLevel)
44+
formatted := false
4345
queryLogger.Trace(context.Background(), time.Now().Add(-test.elapsed), func() (string, int64) {
46+
formatted = true
4447
return "SELECT 1", 1
4548
}, test.err)
4649

47-
if test.want == "" {
48-
require.Empty(t, output.String())
49-
return
50-
}
51-
var record struct {
52-
Level string `json:"level"`
53-
Error string `json:"error"`
54-
}
55-
require.NoError(t, json.Unmarshal(output.Bytes(), &record))
56-
require.Equal(t, test.want, record.Level)
57-
require.Contains(t, record.Error, "SELECT 1")
58-
if test.err != nil {
59-
require.Contains(t, record.Error, test.err.Error())
60-
} else {
61-
require.Contains(t, record.Error, "SLOW SQL")
62-
}
50+
require.Equal(t, test.want != "", formatted)
51+
assertGormQueryLog(t, output.Bytes(), test.want, test.err)
6352
})
6453
}
6554
}
55+
56+
func assertGormQueryLog(t *testing.T, output []byte, wantLevel string, err error) {
57+
t.Helper()
58+
if wantLevel == "" {
59+
require.Empty(t, output)
60+
return
61+
}
62+
var record struct {
63+
Level string `json:"level"`
64+
Error string `json:"error"`
65+
}
66+
require.NoError(t, json.Unmarshal(output, &record))
67+
require.Equal(t, wantLevel, record.Level)
68+
require.Contains(t, record.Error, "SELECT 1")
69+
if err != nil {
70+
require.Contains(t, record.Error, err.Error())
71+
return
72+
}
73+
require.Contains(t, record.Error, "SLOW SQL")
74+
}
75+
76+
func TestGormSpanLoggerPreservesDelegate(t *testing.T) {
77+
tests := []struct {
78+
name string
79+
elapsed time.Duration
80+
err error
81+
level gormlogger.LogLevel
82+
message string
83+
spans []string
84+
}{
85+
{name: "fast query", elapsed: time.Millisecond, level: gormlogger.Warn},
86+
{name: "slow query", elapsed: time.Second, level: gormlogger.Warn, message: "SLOW SQL", spans: []string{"test"}},
87+
{name: "query error", err: errors.New("original query error"), level: gormlogger.Warn, message: "original query error", spans: []string{"test"}},
88+
{name: "info query", elapsed: time.Millisecond, level: gormlogger.Info, message: "SELECT 1"},
89+
{name: "silent error", err: errors.New("query error"), level: gormlogger.Silent},
90+
}
91+
for _, spec := range tests {
92+
t.Run(spec.name, func(t *testing.T) {
93+
test := newSamplingTestSpan(t, false)
94+
queryLogger, output := newDelegatedGormLogger(test)
95+
formatted := false
96+
queryLogger.LogMode(spec.level).Trace(test.ctx, time.Now().Add(-spec.elapsed), func() (string, int64) {
97+
formatted = true
98+
return "SELECT 1", 1
99+
}, spec.err)
100+
101+
test.finish(t, spec.spans...)
102+
require.Equal(t, spec.message != "", formatted)
103+
assertDelegatedGormOutput(t, output.String(), spec.message)
104+
})
105+
}
106+
}
107+
108+
func TestGormSpanLoggerDirectWarningsAndErrors(t *testing.T) {
109+
logs := map[string]func(gormlogger.Interface, context.Context){
110+
"warning": func(logger gormlogger.Interface, ctx context.Context) {
111+
logger.Warn(ctx, "original %s", "message")
112+
},
113+
"error": func(logger gormlogger.Interface, ctx context.Context) {
114+
logger.Error(ctx, "original %s", "message")
115+
},
116+
}
117+
for name, log := range logs {
118+
t.Run(name, func(t *testing.T) {
119+
test := newSamplingTestSpan(t, false)
120+
logger, output := newDelegatedGormLogger(test)
121+
log(logger, test.ctx)
122+
test.finish(t, "test")
123+
require.Contains(t, output.String(), "original message")
124+
})
125+
}
126+
}
127+
128+
func newDelegatedGormLogger(test *samplingTestSpan) (gormlogger.Interface, *bytes.Buffer) {
129+
output := &bytes.Buffer{}
130+
config := gormlogger.Config{SlowThreshold: 200 * time.Millisecond, LogLevel: gormlogger.Warn}
131+
delegate := gormlogger.New(stdlog.New(output, "", 0), config)
132+
return NewGormSpanLogger(test.tracer, delegate, config), output
133+
}
134+
135+
func assertDelegatedGormOutput(t *testing.T, output string, message string) {
136+
t.Helper()
137+
if message == "" {
138+
require.Empty(t, output)
139+
return
140+
}
141+
require.Contains(t, output, message)
142+
require.Contains(t, output, "SELECT 1")
143+
}

‎api/pkg/telemetry/sampling_span_processor.go‎

Lines changed: 29 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ import (
66
sdktrace "go.opentelemetry.io/otel/sdk/trace"
77
)
88

9+
const mustSampleAttribute attribute.Key = "httpsms.must_sample"
10+
911
type samplingSpanProcessor struct {
1012
sdktrace.SpanProcessor
1113
sampler sdktrace.Sampler
@@ -31,28 +33,41 @@ func (processor *samplingSpanProcessor) OnEnd(span sdktrace.ReadOnlySpan) {
3133

3234
func shouldRetainSpan(span sdktrace.ReadOnlySpan) bool {
3335
// Drop counters survive buffer limits, unlike severity attributes and events.
34-
if span.DroppedEvents() > 0 || span.DroppedAttributes() > 0 {
35-
return true
36-
}
37-
if span.Status().Code == codes.Error || hasWarningOrErrorSeverity(span.Attributes()) {
38-
return true
39-
}
40-
for _, event := range span.Events() {
41-
if event.Name == "exception" || hasWarningOrErrorSeverity(event.Attributes) {
36+
return span.DroppedEvents() > 0 ||
37+
span.DroppedAttributes() > 0 ||
38+
span.Status().Code == codes.Error ||
39+
hasRetentionAttributes(span.Attributes()) ||
40+
hasExceptionalEvents(span.Events())
41+
}
42+
43+
func hasExceptionalEvents(events []sdktrace.Event) bool {
44+
for _, event := range events {
45+
if event.Name == "exception" || event.DroppedAttributeCount > 0 || hasRetentionAttributes(event.Attributes) {
4246
return true
4347
}
4448
}
4549
return false
4650
}
4751

48-
func hasWarningOrErrorSeverity(attributes []attribute.KeyValue) bool {
52+
func hasRetentionAttributes(attributes []attribute.KeyValue) bool {
4953
for _, value := range attributes {
50-
if value.Key == "log.severity" {
51-
switch value.Value.AsString() {
52-
case "warn", "warning", "error", "fatal":
53-
return true
54-
}
54+
if isRetentionAttribute(value) {
55+
return true
5556
}
5657
}
5758
return false
5859
}
60+
61+
func isRetentionAttribute(value attribute.KeyValue) bool {
62+
if value.Key == mustSampleAttribute {
63+
return value.Value.AsBool()
64+
}
65+
if value.Key != "log.severity" {
66+
return false
67+
}
68+
switch value.Value.AsString() {
69+
case "warn", "warning", "error", "fatal":
70+
return true
71+
}
72+
return false
73+
}

0 commit comments

Comments
 (0)