Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Option to add new metrics or new lable #34

Closed
BuSHari opened this issue Sep 5, 2019 · 15 comments
Closed

Option to add new metrics or new lable #34

BuSHari opened this issue Sep 5, 2019 · 15 comments

Comments

@BuSHari
Copy link

BuSHari commented Sep 5, 2019

Hello,

I there an option to add a new metrics or new lable to the existing metrics?
for example, i want to add a lable with the requester ip to the default metric

or add a new metric that contain the location data of the requester ip.

I tried to use the group by, but it overwrite the path lable, and i cant add more than one lable.

Thanks,
Idan

@rycus86
Copy link
Owner

rycus86 commented Sep 7, 2019

Hi Idan,

You can add new metrics easily, but you can't add labels to the default metrics.
You could do something like this:

@metrics.summary('requests_by_status', 'Request latencies by status',
                 labels={'requester_id': lambda: get_requester_ip()})
def some_endpoint():
    ...

In this case, the endpoint will be tracked with requests_by_status and also by all of the default ones, like flask_http_request_duration_seconds and flask_http_request_total.

Adding more labels to the built-in metrics is not currently supported.

Does this help?

@BuSHari
Copy link
Author

BuSHari commented Sep 8, 2019

Thanks for the response.
I tried that, but i need it to track all my endpoints that doesn't have the @metrics.do_not_track()
When i tried to create it and duplicate it to some endpoints i got the error

ValueError: Duplicated timeseries in CollectorRegistry:...

@rycus86
Copy link
Owner

rycus86 commented Sep 8, 2019

Yeah, I think it might be nice for this library to support this use case, where you either want to extend the default metrics, or apply a metric collector to each of the endpoints.
I'll have a look when I have time, I don't think it should be too difficult to add this.

@BuSHari
Copy link
Author

BuSHari commented Sep 8, 2019

exactly, it would be great...

@rycus86
Copy link
Owner

rycus86 commented Sep 26, 2019

I've added support for this now. You could either add the same metric to all endpoints:

app = Flask(__name__)
metrics = PrometheusMetrics(app)

@app.route('/simple')
def simple_get():
    pass
    
metrics.register_default(
    metrics.counter(
        'by_path_counter', 'Request count by request paths',
        labels={'path': lambda: request.path}
    )
)

Or just add the same metric to some of the endpoints (not necessarily all of them) :

app = Flask(__name__)
metrics = PrometheusMetrics(app)

by_path_counter = metrics.counter(
    'by_path_counter', 'Request count by request paths',
    labels={'path': lambda: request.path}
)

@app.route('/simple')
@by_path_counter
def simple_get():
    pass
    
@app.route('/plain')
@by_path_counter
def plain():
    pass
    
@app.route('/not/tracked/by/path')
def not_tracked_by_path():
    pass

Let me know if this helps!

@BuSHari
Copy link
Author

BuSHari commented Oct 3, 2019

Thanks for the fix, but i'm getting this error when trying your example:

File "C:\*****\****\******\*******\env\lib\site-packages\prometheus_flask_exporter\__init__.py", line 358, in register_default
    for endpoint, view_func in self.app.view_functions.items():
AttributeError: 'NoneType' object has no attribute 'view_functions'

@rycus86
Copy link
Owner

rycus86 commented Oct 3, 2019

@BuSHari could you provide a small example that fails to test with?

@BuSHari
Copy link
Author

BuSHari commented Oct 3, 2019

Is this helps?

user_info = metrics.register_default(
    metrics.counter(
        'flask_http_user_info', 'Get mote user info',
        labels={'path': lambda: request.path}
    )
)


@api.route('/', methods=['GET', 'POST'])
@user_info
class Serial(Resource):
    @api.login_required(oauth_scopes=['serials:read'])
    def get(self):
        pass

@BuSHari
Copy link
Author

BuSHari commented Oct 3, 2019

And this is an example if i try it for all endpoints:

from flask import Flask, request
from prometheus_flask_exporter import PrometheusMetrics
from prometheus_flask_exporter.multiprocess import GunicornPrometheusMetrics

# Check if app run trough gunicorn
is_gunicorn = "gunicorn" in os.environ.get("SERVER_SOFTWARE", "")

if is_gunicorn:
    metrics = GunicornPrometheusMetrics(app=None)
else:
    metrics = PrometheusMetrics(app=None)


def create_app(flask_config_name=None, **kwargs):
    import threading
    threading.stack_size(2 * 1024 * 1024)
    app = Flask(__name__, **kwargs)

    # Activate metrics '/metrics'
    metrics.init_app(app)
    # static information as metric
    metrics.info('app_info', 'Application info', version='1.0.0')

    gunicorn_error_logger = logging.getLogger('gunicorn.error')
    app.logger.handlers.extend(gunicorn_error_logger.handlers)

    env_flask_config_name = os.getenv('FLASK_CONFIG')

    with app.app_context():

        from . import extensions
        extensions.init_app(app)

        from .templates.db_admin import add_db_admin_cmds
        add_db_admin_cmds(app)

        from . import modules
        modules.init_app(app)

        metrics.register_default(
            metrics.counter(
                'by_path_counter', 'Request count by request paths',
                labels={'path': lambda: request.path}
                )
            )

    return app

@rycus86
Copy link
Owner

rycus86 commented Oct 3, 2019

Yep, so the register_default does not return anything, that should just register a default metric, that's why it complains about the None I believe.

user_info = metrics.register_default(...

Your second example with Gunicorn should work though.
I'll try to have a look later why it doesn't.

@BuSHari
Copy link
Author

BuSHari commented Oct 3, 2019

ok thank you.
The first example is working for individuals endpoints, please let me know if there is a fix for all endpoints.

Thank you...

@rycus86
Copy link
Owner

rycus86 commented Oct 3, 2019

All right, I'll have a look later if I can.
By the looks of it, that should work.
Do you use plain Flask, or some other extension like Restplus or Restful?

@BuSHari
Copy link
Author

BuSHari commented Oct 3, 2019

thank you, I use restplus

@rycus86
Copy link
Owner

rycus86 commented Oct 3, 2019

OK, it should be fixed in version 0.11.0.
There was a problem, that the default registration only worked if the Flask app was passed in at the time of creating PrometheusMetrics object, not when it's init_app is used.
Now changed it so that it works under inside a with app.app_context(): context, otherwise the register_default(..., app=None) function now accepts the Flask app to look for endpoints on.

See https://github.com/rycus86/prometheus_flask_exporter/blob/master/examples/restplus-default-metrics/server.py for an example, though it's pretty much what you had above, your example should now work without any changes.

Let me know how this works out for you!

@BuSHari
Copy link
Author

BuSHari commented Oct 3, 2019

Great!
Thank you its working now... I'll test it further and let you know for any issues

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants