grafana / pyroscope-rs

Pyroscope Profiler for Rust. Profile your Rust applications.
Apache License 2.0
139 stars 28 forks source link

Missing log_level option for Python client #57

Open yelinaung opened 2 years ago

yelinaung commented 2 years ago

First of all, thanks for creating an awesome profiling tool. 👏🏾

Describe the bug you encountered: The doc mentioned that we could configure the log_level.

import pyroscope

pyroscope.configure(
....
  log_level           = "info" # default is info, possible values: trace, debug, info, warn, error, and critical 
)

When I configured the log_level, I got the error

    pyroscope.configure(
TypeError: configure() got an unexpected keyword argument 'log_level'

What did you expect to happen instead? I expect the log_level parameter to exist and should only see certain logs.

How did you install pyroscope-rs? I installed it with pip install pyroscope-io -U


pyroscope-rs version and environment

$ pip list | grep pyro
pyroscope-io          0.7.2
$ python3 --version
Python 3.10.1

I took a peak at the code and found that the log_level param is missing from the configure function https://github.com/pyroscope-io/pyroscope-rs/blob/main/pyroscope_ffi/python/pyroscope/__init__.py#L12-L15 Digging a bit deeper, the param log_level was removed in this commit https://github.com/pyroscope-io/pyroscope-rs/commit/1696cd06db53a1d7ebbdcee556334156eed3429e.

siumingdev commented 2 years ago

First thanks a lot for this easy to use and awesome tool.

But I think there is a related but separate issue: seems the log levels mapping is incorrect https://github.com/pyroscope-io/pyroscope-rs/commit/1696cd06db53a1d7ebbdcee556334156eed3429e#diff-9d372ec50bcb337bf0a01889d97d230d6d235bb15487175a17e4c7a1456ce285R51-R70

Here 50 => error, 40 => warn, 30 => info etc, while in python logging 40 => ERROR, 30 => WARNING, 20 => INFO etc