Adds to REDIS instrumentation message value as tag #1972
Conversation
|
|
Fix unit tests Fix file format
Codecov Report
@@ Coverage Diff @@
## main #1972 +/- ##
==========================================
+ Coverage 83.57% 83.59% +0.01%
==========================================
Files 192 192
Lines 6162 6169 +7
==========================================
+ Hits 5150 5157 +7
Misses 1012 1012
|
| if (!string.IsNullOrEmpty(message)) | ||
| { | ||
| // Example: "db.operation": [0]:GET key1 | ||
| activity.SetTag(SemanticConventions.AttributeDbOperation, message); |
There was a problem hiding this comment.
From semantic conventions doc, db.operation is required only if db.statement is not applicable. (https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/semantic_conventions/database.md#call-level-attributes)
Since we already populate the db.statement, we need to have good reason for populating db.operation considering its not free to obtain the value for it.
There was a problem hiding this comment.
Thanks a lot for review.
I've tested and had have next results
Read data from REDIS
db.redis.database_index=0
db.redis.flags=1024
db.statement=GET
peer.service=xx.xx.xx.xx:6379
net.peer.ip=xx.xx.xx.xx
db.operation=[0]:GET sso:templates:String:USERRESET (RedisValueProcessor)
Delete key in REDIS
db.redis.database_index=0
db.redis.flags=1028
db.statement=UNLINK
peer.service=xx.xx.xx.xx:6379
net.peer.ip=xx.xx.xx.xx
db.operation=[0]:UNLINK sso:templates:String:USERRESET (DemandZeroOrOneProcessor)
Write JSON to REDIS
db.redis.database_index=0
db.redis.flags=1028
db.statement=SETEX
peer.service=xx.xx.xx.xx:6379
net.peer.ip=xx.xx.xx.xx
db.operation=[0]:SETEX sso:templates:String:USERRESET (BooleanProcessor)
So, db.operation contains REDIS operation, db number and key. Semantic conventions doc contains next text
When setting this to an SQL keyword, it is not recommended to attempt any client-side parsing of
db.statementjust to get this property, but it should be set if the operation name is provided by the library being instrumented.
For Redis, the value provided for db.statement SHOULD correspond to the syntax of the Redis CLI. If, for example, the
HMSET commandis invoked,"HMSET myhash field1 'Hello' field2 'World'"would be a suitable value for db.statement.
db.statement does not contain key and value. db.operation contains key but does not contain value. It is much more information to analyze!
How would you like an idea if I add parsing db.operation to get Redis CLI-like to match the semantic conventions doc?
|
Yes! This would be so useful!! |
|
This PR can be closed as it was implemented in #2198 |
Changes
Current implementation detects database number and command (like
GET,EXISTSand other) but does not present information about key(s). This implementation of theRedisProfilerEntryToActivityConverteranalyzes command and tries extractMessagevalue (like[0]:GET key1) using reflection . In case extract message operation completed successfully the value will be placed to tag with nameSemanticConventions.AttributeDbOperation.