Metrics instrumentation redis

[srikanthccv]

DB semconv https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/semantic_conventions/database-metrics.md

[dbgoytia]

Hi @srikanthccv ! I'd like to contribute to the project, however i might need some help. May I take on this problem?

[srikanthccv]

Ask any question here you need help with.

[dbgoytia]

Hi @srikanthccv ! I do have a couple of questions regarding this problem. I hope they aren't too obvious. I'm taking as reference MR:1110 on the requests library to get some guidance.

---

1. How do I know which metrics are provided by the Meter Provider?

On the example on the requests library, I see we're testing against this:

expected_attributes = {
            "http.status_code": 200,
            "http.host": "examplehost",
            "net.peer.port": 8000,
            "net.peer.name": "examplehost",
            "http.method": "GET",
            "http.flavor": "1.1",
            "http.scheme": "http",
        }

linking it back to the specification, would the metrics required need to be something like this?

expected_attributes = {
       "db.client.connections.idle.max": XYZ,
       "db.client.connections.idle.min": XYZ,
       "db.client.connections.max": XYZ,
       "db.client.connections.pending_requests": XYZ,
       "db.client.connections.timeouts": XYZ,
       "db.client.connections.create_time": XYZ, 
       "db.client.connections.wait_time": XYZ, 
       "db.client.connections.use_time": XYZ,
}

---

2. I hope this is the right question because I'm new to OTEL, but how do I know that the library actually exposes these metrics? I guess this is on my end to test, but do you have some general guidance?

---

3. Does the Meter Provider need to be initialized in any special form?

Refering back again to the Requests library instrumentation tests, I see that we're testing against what seems to be the default provider:

class TestRequestsIntergrationMetric(TestBase):
    ... 
    def setUp(self):
        super().setUp()
        RequestsInstrumentor().instrument(meter_provider=self.meter_provider)
    ... 

[srikanthccv]

1. Yes, the db semantic conventions has set of recommended attributes. We need to include as many as possible.
2. Did you mean the library we are instrumenting? Yes, we need to find a way to derive the information from the redis library by monkeypatching or wrapping it similar to how we do it for tracing.
3. The TestBase class instantiates the provider which all instrumentation can use. No special form required.

[dbgoytia]

Thanks a lot for taking the time @srikanthccv, I have an idea but let me know if this is heading the correct direction:

I'm thinking for the first value that's required in the semconv for DB "db.client.connections.usage" to wrap a "CLIENT LIST" query to Redis, since we can't infer the rest of the data directly from simply doing a get or set operation, as opposed to the requests.get() for example.

For more clarity, on the requests library, we're asserting the following attributes:

expected_attributes = {
            "http.status_code": 200,
            "http.host": "examplehost",
            "net.peer.port": 8000,
            "net.peer.name": "examplehost",
            "http.method": "GET",
            "http.flavor": "1.1",
            "http.scheme": "http",
        }

all of those could get inferred (or most of them) by parsing the URL that we're consuming, in this case http://examplehost:8000/status/200

As opposed to that, Redis will create a connection pool, and only when we send command will the Redis client actually retrieve a connection from the connection pool, so we can't simply derive that value - I think.

The CLIENT LIST operation returns information and statistics about the client connections:

127.0.0.1:6379> CLIENT LIST
id=11 addr=127.0.0.1:37202 laddr=127.0.0.1:6379 fd=12 name=redisinsight-common-redis-st age=1963 idle=1941 flags=N db=0 sub=0 psub=0 multi=-1 qbuf=0 qbuf-free=0 argv-mem=0 obl=0 oll=0 omem=0 tot-mem=20496 events=r cmd=info user=default redir=-1
id=12 addr=127.0.0.1:37266 laddr=127.0.0.1:6379 fd=13 name=redisinsight-browser-9e5e319b age=1956 idle=1956 flags=N db=0 sub=0 psub=0 multi=-1 qbuf=0 qbuf-free=0 argv-mem=0 obl=0 oll=0 omem=0 tot-mem=20496 events=r cmd=dbsize user=default redir=-1
id=13 addr=127.0.0.1:37658 laddr=127.0.0.1:6379 fd=14 name= age=1908 idle=0 flags=N db=0 sub=0 psub=0 multi=-1 qbuf=26 qbuf-free=40928 argv-mem=10 obl=0 oll=0 omem=0 tot-mem=61466 events=r cmd=client user=default redir=-1

Now... the reason I'm not sure if this is the correct path forward, is because we would need to make that extra call, do you think that this is correct?

[srikanthccv]

Does it include all client connections or just this client? Even if it's just the client at hand, I don't think we should make a call to server and parse the data. I haven't really looked at the redis library but from the quick search there is definitely some information we can get from connection pool class https://redis-py.readthedocs.io/en/stable/connections.html#connectionpool. You might want to dig into the lib and see what all can be derived.

[dbgoytia]

Oh awesome ! That's great. Thanks again for taking the time :). I'll review this docs and come back with more questions.

[dbgoytia]

By the way @srikanthccv, forgot to mention but the client_list will include all the connections made from a given connection pool, for example:

# All connections made to the redis server
>>> len(redis_client.client_list())
10

I think this one is more accurate though:

## All active connections to the redis server from a given connection pool
>>> len(redis_client.connection_pool._in_use_connections)
8

[dbgoytia]

So summarizing, for the ConnectionPool class we can get the following attributes:

{
  'connection_class': <class'redis.connection.Connection'>,
  'connection_kwargs': {
    'host': 'localhost',
    'port': 6379,
    'db': 0
  },
  'max_connections': 2147483648,
  '_fork_lock': <unlocked_thread.lockobjectat0x7fdd9d0d3d50>,
  '_lock': <unlocked_thread.lockobjectat0x7fdd9d0d3120>,
  '_created_connections': 1,
  '_available_connections': [
    
  ],
  '_in_use_connections': {
    Connection<host=localhost,
    port=6379,
    db=0>
  },
  'pid': 10366
}

And for each of those Connections we can get the following attributes:

{
  'pid': 10366,
  'host': 'localhost',
  'port': 6379,
  'db': 0,
  'username': None,
  'client_name': None,
  'password': None,
  'socket_timeout': None,
  'socket_connect_timeout': None,
  'socket_keepalive': False,
  'socket_keepalive_options': {
    
  },
  'socket_type': 0,
  'retry_on_timeout': False,
  'retry_on_error': [
    
  ],
  'retry': <redis.retry.Retryobjectat0x7fdda1043a90>,
  'health_check_interval': 0,
  'next_health_check': 0,
  'redis_connect_func': None,
  'encoder': <redis.connection.Encoderobjectat0x7fdd9ffea650>,
  '_sock': <socket.socketfd=6,
  family=AddressFamily.AF_INET6,
  type=SocketKind.SOCK_STREAM,
  proto=6,
  laddr=('::1',
  54750,
  0,
  0),
  raddr=('::1',
  6379,
  0,
  0)>,
  '_socket_read_size': 65536,
  '_parser': <redis.connection.PythonParserobjectat0x7fdd9ffcdd50>,
  '_connect_callbacks': [
    
  ],
  '_buffer_cutoff': 6000
}

I believe this way we can get the first value db.client.connections.usage, but not sure if we can get the db.client.connections.usage.state 🤔.

[srikanthccv]

@dbgoytia let's aim for what can be achieved with the present information. We do not have provide all the attributes mentioned in sem conv except the required attributes.

[dbgoytia]

Hey @srikanthccv, I'd like to get an early review on the feature to see if I'm heading the right direction: #1194

I do have a coulpe of questions around it though:

• I feel that I'm not using the UpDownCounter correctly.
• I'm not sure what name should I give the span.
• I'm not sure I'm setting the metric_labels/attributes correctly.

An there is one thing, it might sound a bit strange, but on the multiple-connection test, the UpDownCounter is not actually "writing" the correct value, maybe I'm using it wrong! I wrote under a log to see how many connections are available and I can see that the _created_connections is holding the right value:

k:db.client.connections.usage, v:1
k:db.client.connections.usage, v:2
k:db.client.connections.usage, v:3

However, I'm not sure why, but the data_point is actually only showing '1':

NumberDataPoint(attributes={'db.client.connections.usage': 1}, start_time_unix_nano=1657998552408972000, time_unix_nano=1657998552411664000, value=1)

Maybe it's my lack of knowledge with the SDK! Can you give me some guidance here?

Oh and by the way, the tests are not mocked correctly, just yet, I'm testing locally to see if I'm heading the right direction.

[zhengkezhou1]

@srikanthccv pls assign this one to me, i will start soon!