[opentelemetry-instrumentation-redis] Stop sending db.name argument or rename it to redis[0-15]

[luke6Lh43]

Hi! I've been recently playing with OpenTelemetry for Python (Flask) application and noticed that for Redis db.name argument is send to OpenTelemetry collector which seems to be a number of database (integer). This seems to be incorrect as in Redis there is no db name concept (databases are numbered from 0 to 15). Technically, it shouldn't be any problem with that but it may break some OpenTelemetry backends which expects a real DB name not a number. I have done some additional debugging and found that for node.js and .NET db.name argument is not send to collector. Shouldn't we have some consistency here?

Describe your environment

$ python --version
Python 3.8.13
$
$ pip list | grep 'opentelemetry|redis'
opentelemetry-api 1.13.0
opentelemetry-distro 0.34b0
opentelemetry-exporter-otlp 1.13.0
opentelemetry-exporter-otlp-proto-grpc 1.13.0
opentelemetry-exporter-otlp-proto-http 1.13.0
opentelemetry-instrumentation 0.34b0
opentelemetry-instrumentation-aws-lambda 0.34b0
opentelemetry-instrumentation-dbapi 0.34b0
opentelemetry-instrumentation-flask 0.34b0
opentelemetry-instrumentation-grpc 0.34b0
opentelemetry-instrumentation-jinja2 0.34b0
opentelemetry-instrumentation-logging 0.34b0
opentelemetry-instrumentation-redis 0.34b0
opentelemetry-instrumentation-requests 0.34b0
opentelemetry-instrumentation-sqlite3 0.34b0
opentelemetry-instrumentation-urllib 0.34b0
opentelemetry-instrumentation-urllib3 0.34b0
opentelemetry-instrumentation-wsgi 0.34b0
opentelemetry-propagator-aws-xray 1.0.1
opentelemetry-proto 1.13.0
opentelemetry-sdk 1.13.0
opentelemetry-semantic-conventions 0.34b0
opentelemetry-util-http 0.34b0
redis 4.3.4

Steps to reproduce
Any Python app with connection to Redis will show this behavior.

What is the expected behavior?
Stop sending db.name argument or rename it to redis[0-15]

What is the actual behavior?
The db.name argument is send as a number of Redis database.

Additional context

Please see below some logs from OpenTelemetry collector for python and node.js to see a difference.

===> PYTHON EXAMPLE

ScopeSpans #0
ScopeSpans SchemaURL:
InstrumentationScope opentelemetry.instrumentation.redis 0.34b0
Span #0
Trace ID : 4bc10b43ab0a0d3042f38ebbb32baef1
Parent ID : 79e2aed933827894
ID : 22f4fba607e73a33
Name : HMSET
Kind : SPAN_KIND_CLIENT
Start time : 2022-10-21 09:40:50.606962566 +0000 UTC
End time : 2022-10-21 09:40:50.609568624 +0000 UTC
Status code : STATUS_CODE_UNSET
Status message :
Attributes:
-> db.statement: STRING(HMSET person1-hash name jane age 20)
-> db.system: STRING(redis)
-> db.name: INT(0)
-> db.redis.database_index: INT(0)
-> net.peer.name: STRING(redis-svc)
-> net.peer.port: STRING(6379)
-> net.transport: STRING(ip_tcp)
-> db.redis.args_length: INT(6)

===> NODEJS EXAMPLE

ScopeSpans #0
ScopeSpans SchemaURL:
InstrumentationScope @opentelemetry/instrumentation-redis-4 0.33.0
Span #0
Trace ID : 21a071f4d1d7c860ecb758398d304f60
Parent ID : 1bbf5328c079ceda
ID : 13dc47b2521f7f82
Name : redis-GET
Kind : SPAN_KIND_CLIENT
Start time : 2022-10-21 09:47:16.9553723 +0000 UTC
End time : 2022-10-21 09:47:16.957585 +0000 UTC
Status code : STATUS_CODE_UNSET
Status message :
Attributes:
-> db.system: STRING(redis)
-> net.peer.name: STRING(redis-svc)
-> net.peer.port: INT(6379)
-> db.statement: STRING(GET)
ResourceSpans #4
Resource SchemaURL:
Resource labels:
-> service.name: STRING(nodejs-redis)
-> telemetry.sdk.language: STRING(nodejs)
-> telemetry.sdk.name: STRING(opentelemetry)
-> telemetry.sdk.version: STRING(0.24.0)

I am happy to contribute to it by reviewing the code fix and testing the behavior.

@svrnm @sanketmehta28

[srikanthccv]

I guess the best way to provide this information is through db.redis.database_index than db.name. Feel free to send a pull request.

[sanketmehta28]

Yes @luke6Lh43. This makes sense. I have verified the behavior and while instrumenting the redis, it tries to fetch the "db" attribute from connection_pool object and in case of its absence (which is true in case of redis), it will by default set to '0'.
AFAIK this is not the right behavior (as reflected in jode js or .net agents). You should create a PR and send it for review.

[luke6Lh43]

Thank you @sanketmehta28 and @srikanthccv for your comments! I will start working on it in upcoming days :)