Propagate database number, user, and RedisURI into Tracing #3005
Conversation
| @@ -478,6 +482,13 @@ private void attachTracing(ChannelHandlerContext ctx, RedisCommand<?, ?, ?> comm | |||
| Tracer.Span span = tracer.nextSpan(context); | |||
| span.name(command.getType().name()); | |||
|
|
|||
| String redisUriStr = ctx.channel().attr(ConnectionBuilder.REDIS_URI).get(); | |||
| RedisURI redisURI = RedisURI.create(redisUriStr); | |||
| span.tag("db.uri", redisURI.toString()); | |||
There was a problem hiding this comment.
Can you elaborate on how you chose the attribute names?
I had a quick look at the semantic conventions for Redis and seems the recommendations are different? Not an expert myself though, so I may also be wrong.
There was a problem hiding this comment.
I just set the tag using parameters from RedisURI.
I think it is better to follow the semantic convention, thanks for pointing it.
By the way, the user is missing from semantic conventions for Redis, so I removed it, What do you think?
| @@ -478,6 +482,13 @@ private void attachTracing(ChannelHandlerContext ctx, RedisCommand<?, ?, ?> comm | |||
| Tracer.Span span = tracer.nextSpan(context); | |||
| span.name(command.getType().name()); | |||
|
|
|||
| String redisUriStr = ctx.channel().attr(ConnectionBuilder.REDIS_URI).get(); | |||
There was a problem hiding this comment.
I see that there is a chance that the attribute is missing, so it would be good to make this whole logic conditional, e.g.
if (channel.hasAttr(ConnectionBuilder.REDIS_URI)) {
...
}There was a problem hiding this comment.
It makes sense, I updated 🙇
Not yet, I just updated to verify the update. |
Issue: #2232
Make sure that:
mvn formatter:formattarget. Don’t submit any formatting related changes.