From 8f54a186e089928c5b36b6885fc816c413a82287 Mon Sep 17 00:00:00 2001 From: idealism-xxm Date: Thu, 4 Jun 2020 12:50:38 +0800 Subject: [PATCH] fix(apmgoredisv8): rename hook type name; use apm.SpanFromContext to retrieve started span Co-authored-by: Andrew Wilkins --- docs/instrumenting.asciidoc | 4 +-- module/apmgoredisv8/hook.go | 43 +++++++++++--------------------- module/apmgoredisv8/hook_test.go | 6 ++--- 3 files changed, 20 insertions(+), 33 deletions(-) diff --git a/docs/instrumenting.asciidoc b/docs/instrumenting.asciidoc index 61aa0d1b4..2589a7cce 100644 --- a/docs/instrumenting.asciidoc +++ b/docs/instrumenting.asciidoc @@ -523,7 +523,7 @@ func handleRequest(w http.ResponseWriter, req *http.Request) { Package apmgoredisv8 provides a means of instrumenting https://github.com/go-redis/redis[go-redis/redis] for v8 so that Redis commands are reported as spans within the current transaction. -To report Redis commands, you can call `AddHook(apmgoredis.NewGoRedisApmHook())` +To report Redis commands, you can call `AddHook(apmgoredis.NewHook())` from instance of `redis.Client`, `redis.ClusterClient`, or `redis.Ring`. [source,go] @@ -538,7 +538,7 @@ func main() { redisClient := redis.NewClient(&redis.Options{}) // Add apm hook to redisClient. // Redis commands will be reported as spans within the current transaction. - redisClient.AddHook(apmgoredis.NewGoRedisApmHook()) + redisClient.AddHook(apmgoredis.NewHook()) redisClient.Get(ctx, "key") } diff --git a/module/apmgoredisv8/hook.go b/module/apmgoredisv8/hook.go index 5c871bd0f..af7cdaf12 100644 --- a/module/apmgoredisv8/hook.go +++ b/module/apmgoredisv8/hook.go @@ -29,30 +29,30 @@ import ( "go.elastic.co/apm" ) -// goRedisApmHook is an implementation of redis.Hook that reports cmds as spans to Elastic APM. -type goRedisApmHook struct{} +// hook is an implementation of redis.Hook that reports cmds as spans to Elastic APM. +type hook struct{} -// NewGoRedisApmHook returns a redis.Hook that reports cmds as spans to Elastic APM. -func NewGoRedisApmHook() redis.Hook { - return &goRedisApmHook{} +// NewHook returns a redis.Hook that reports cmds as spans to Elastic APM. +func NewHook() redis.Hook { + return &hook{} } // BeforeProcess initiates the span for the redis cmd -func (r *goRedisApmHook) BeforeProcess(ctx context.Context, cmd redis.Cmder) (context.Context, error) { - span, ctx := apm.StartSpan(ctx, getCmdName(cmd), "db.redis") - return withSpan(ctx, span), nil +func (r *hook) BeforeProcess(ctx context.Context, cmd redis.Cmder) (context.Context, error) { + _, ctx = apm.StartSpan(ctx, getCmdName(cmd), "db.redis") + return ctx, nil } // AfterProcess ends the initiated span from BeforeProcess -func (r *goRedisApmHook) AfterProcess(ctx context.Context, cmd redis.Cmder) error { - if span := spanFromContext(ctx); span != nil { +func (r *hook) AfterProcess(ctx context.Context, cmd redis.Cmder) error { + if span := apm.SpanFromContext(ctx); span != nil { span.End() } return nil } // BeforeProcessPipeline initiates the span for the redis cmds -func (r *goRedisApmHook) BeforeProcessPipeline(ctx context.Context, cmds []redis.Cmder) (context.Context, error) { +func (r *hook) BeforeProcessPipeline(ctx context.Context, cmds []redis.Cmder) (context.Context, error) { // Join all cmd names with ", ". var cmdNameBuf bytes.Buffer for i, cmd := range cmds { @@ -62,13 +62,13 @@ func (r *goRedisApmHook) BeforeProcessPipeline(ctx context.Context, cmds []redis cmdNameBuf.WriteString(getCmdName(cmd)) } - span, ctx := apm.StartSpan(ctx, cmdNameBuf.String(), "db.redis") - return withSpan(ctx, span), nil + _, ctx = apm.StartSpan(ctx, cmdNameBuf.String(), "db.redis") + return ctx, nil } // AfterProcess ends the initiated span from BeforeProcessPipeline -func (r *goRedisApmHook) AfterProcessPipeline(ctx context.Context, cmds []redis.Cmder) error { - if span := spanFromContext(ctx); span != nil { +func (r *hook) AfterProcessPipeline(ctx context.Context, cmds []redis.Cmder) error { + if span := apm.SpanFromContext(ctx); span != nil { span.End() } return nil @@ -81,16 +81,3 @@ func getCmdName(cmd redis.Cmder) string { } return cmdName } - -type goRedisApmSpanKey struct{} - -var key = goRedisApmSpanKey{} - -func withSpan(ctx context.Context, span *apm.Span) context.Context { - return context.WithValue(ctx, key, span) -} - -func spanFromContext(ctx context.Context) *apm.Span { - span, _ := ctx.Value(key).(*apm.Span) - return span -} diff --git a/module/apmgoredisv8/hook_test.go b/module/apmgoredisv8/hook_test.go index 56521c27e..a20782a37 100644 --- a/module/apmgoredisv8/hook_test.go +++ b/module/apmgoredisv8/hook_test.go @@ -141,7 +141,7 @@ func redisEmptyClient() *redis.Client { func redisHookedClient() *redis.Client { client := redisEmptyClient() - client.AddHook(apmgoredis.NewGoRedisApmHook()) + client.AddHook(apmgoredis.NewHook()) return client } @@ -151,7 +151,7 @@ func redisEmptyClusterClient() *redis.ClusterClient { func redisHookedClusterClient() *redis.ClusterClient { client := redisEmptyClusterClient() - client.AddHook(apmgoredis.NewGoRedisApmHook()) + client.AddHook(apmgoredis.NewHook()) return client } @@ -161,6 +161,6 @@ func redisEmptyRing() *redis.Ring { func redisHookedRing() *redis.Ring { client := redisEmptyRing() - client.AddHook(apmgoredis.NewGoRedisApmHook()) + client.AddHook(apmgoredis.NewHook()) return client }