Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat(metrics): be able to send new tags on stop functions #7

Merged
merged 1 commit into from
Oct 7, 2024
Merged

feat(metrics): be able to send new tags on stop functions #7

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 14 additions & 11 deletions metrics.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,14 +83,14 @@ func ReportFuncCallAndTimingCtxWithErr(ctx context.Context, tags ...Tags) func(e
}
}

func ReportFuncCallAndTimingWithErr(tags ...Tags) func(err *error) {
func ReportFuncCallAndTimingWithErr(tags ...Tags) func(err *error, tags ...Tags) {
fn := CallerFuncName(1)
reportFunc(fn, "called", tags...)
_, stop := reportTiming(context.Background(), tags...)
return func(err *error) {
stop()
return func(err *error, stopTags ...Tags) {
stop(stopTags...)
if err != nil && *err != nil {
ReportClosureFuncError(fn, tags...)
ReportClosureFuncError(fn, MergeTags(MergeTags(nil, tags...), stopTags...))
}
}
}
Expand All @@ -111,7 +111,7 @@ func reportFunc(fn, action string, tags ...Tags) {
client.Incr(fmt.Sprintf("func.%v", action), tagArray, 0.77)
}

type StopTimerFunc func()
type StopTimerFunc func(tags ...Tags)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Changing StopTimerFunc type may break existing implementations

The type StopTimerFunc has been updated from func() to func(tags ...Tags). This modification can affect all existing implementations and usages of this type, potentially causing compatibility issues.


func ReportFuncTiming(tags ...Tags) StopTimerFunc {
_, stopFn := reportTiming(context.Background(), tags...)
Expand All @@ -127,7 +127,7 @@ func reportTiming(ctx context.Context, tags ...Tags) (context.Context, StopTimer
defer clientMux.RUnlock()

if client == nil {
return ctx, func() {}
return ctx, func(...Tags) {}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Anonymous function signature changed to accept ...Tags

The anonymous function returned now accepts variadic ...Tags as parameters. This change modifies the function signature from func() to func(...Tags), which may disrupt existing code expecting a parameterless function.

}
t := time.Now()
fn := CallerFuncName(2)
Expand Down Expand Up @@ -170,13 +170,15 @@ func reportTiming(ctx context.Context, tags ...Tags) (context.Context, StopTimer
}
}(fn, t)

return spanCtx, func() {
return spanCtx, func(stopTags ...Tags) {
d := time.Since(t)
close(doneC)

stopTagArray := append(tagArray, JoinTags(stopTags...)...)

clientMux.RLock()
defer clientMux.RUnlock()
client.Timing("func.timing", d, tagArray, 1)
client.Timing("func.timing", d, stopTagArray, 1)
if span != nil {
span.End()
}
Expand All @@ -187,7 +189,7 @@ func ReportClosureFuncTiming(name string, tags ...Tags) StopTimerFunc {
clientMux.RLock()
defer clientMux.RUnlock()
if client == nil {
return func() {}
return func(...Tags) {}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Change in returned function's signature may affect callers

The returned function now accepts ...Tags as parameters, changing its signature from func() to func(...Tags). Verify that any callers of this function are updated to accommodate this change.

}
t := time.Now()
tagArray := JoinTags(tags...)
Expand All @@ -212,13 +214,14 @@ func ReportClosureFuncTiming(name string, tags ...Tags) StopTimerFunc {
}
}(name, t)

return func() {
return func(stopTags ...Tags) {
d := time.Since(t)
close(doneC)
stopTagArray := append(tagArray, JoinTags(stopTags...)...)

clientMux.RLock()
defer clientMux.RUnlock()
client.Timing("func.timing", d, tagArray, 1)
client.Timing("func.timing", d, stopTagArray, 1)
}
}

Expand Down
25 changes: 21 additions & 4 deletions metrics_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ func Test_ReportTimedFuncWithError(t *testing.T) {
t.Run("can be deferred and report the error", func(t *testing.T) {
rec.reset()
exec := func() (err error) {
defer ReportFuncCallAndTimingWithErr(Tags{"foo": "bar"})(&err)
defer ReportFuncCallAndTimingWithErr(Tags{"foo": "bar"})(&err, Tags{"stop": "error"})

time.Sleep(5 * time.Millisecond)
return errors.New("this error should be recorder")
Expand All @@ -55,15 +55,16 @@ func Test_ReportTimedFuncWithError(t *testing.T) {
expectedTags := []string{"foo=bar", "func_name=1"}
assert.Equal(t, "Incr", rec.calls[0][0])
assert.Equal(t, "func.called", rec.calls[0][1])
assert.Equal(t, expectedTags, rec.calls[0][2])
assert.ElementsMatch(t, expectedTags, rec.calls[0][2])

expectedTags = []string{"foo=bar", "stop=error", "func_name=1"}
assert.Equal(t, "Count", rec.calls[1][0])
assert.Equal(t, "func.timing", rec.calls[1][1])
assert.Equal(t, expectedTags, rec.calls[1][3])
assert.ElementsMatch(t, expectedTags, rec.calls[1][3])

assert.Equal(t, "Incr", rec.calls[2][0])
assert.Equal(t, "func.error", rec.calls[2][1])
assert.Equal(t, expectedTags, rec.calls[2][2])
assert.ElementsMatch(t, expectedTags, rec.calls[2][2])
})

t.Run("can be deferred and skip error reporting if nil", func(t *testing.T) {
Expand All @@ -79,6 +80,22 @@ func Test_ReportTimedFuncWithError(t *testing.T) {
assert.Equal(t, "func.called", rec.calls[0][1])
assert.Equal(t, "func.timing", rec.calls[1][1])
})

t.Run("can be deferred with new tags and skip error reporting", func(t *testing.T) {
rec.reset()
exec := func() {
var err error
defer ReportFuncCallAndTimingWithErr(Tags{"foo": "bar"})(&err, Tags{"something": "new"})
}

exec()
require.Len(t, rec.calls, 2)

expectedTags := []string{"foo=bar", "something=new", "func_name=1"}
assert.Equal(t, "func.called", rec.calls[0][1])
assert.Equal(t, "func.timing", rec.calls[1][1])
assert.ElementsMatch(t, expectedTags, rec.calls[1][3])
})
}

type statterRecorder struct {
Expand Down