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

"Duplicated timeseries in CollectorRegistry" when testing #17

Closed
maximegregoire opened this issue Jan 11, 2019 · 8 comments
Closed

"Duplicated timeseries in CollectorRegistry" when testing #17

maximegregoire opened this issue Jan 11, 2019 · 8 comments

Comments

@maximegregoire
Copy link

maximegregoire commented Jan 11, 2019

Hi,

Thanks for this awesome module!

When testing my flask application, I instantiate a new instance of my application for every test using fixtures. For the first test, everything goes right, but on subsequent tests, I get a "Duplicated timeseries" error. My understanding is that PrometheusMetrics uses the same registry for every application. Is this the expected behavior?

Here's a minimal snippet that reproduces this behaviour.

from flask import Flask
from prometheus_flask_exporter import PrometheusMetrics

app1 = Flask(__name__)
metrics1 = PrometheusMetrics(app1)
# some test

app2 = Flask(__name__)
metrics2 = PrometheusMetrics(app2)
# some other test

The output:

    metrics2 = PrometheusMetrics(app2)
/usr/local/lib/python3.6/site-packages/prometheus_flask_exporter/__init__.py:115: in __init__
    self.init_app(app)
/usr/local/lib/python3.6/site-packages/prometheus_flask_exporter/__init__.py:137: in init_app
    self._defaults_prefix, app
/usr/local/lib/python3.6/site-packages/prometheus_flask_exporter/__init__.py:253: in export_defaults
    **buckets_as_kwargs
/usr/local/lib/python3.6/site-packages/prometheus_client/metrics.py:494: in __init__
    labelvalues=labelvalues,
/usr/local/lib/python3.6/site-packages/prometheus_client/metrics.py:103: in __init__
    registry.register(self)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <prometheus_client.registry.CollectorRegistry object at 0x7f425acb0d68>, collector = <prometheus_client.metrics.Histogram object at 0x7f424a45c3c8>

    def register(self, collector):
        '''Add a collector to the registry.'''
        with self._lock:
            names = self._get_names(collector)
            duplicates = set(self._names_to_collectors).intersection(names)
            if duplicates:
                raise ValueError(
                    'Duplicated timeseries in CollectorRegistry: {0}'.format(
>                       duplicates))
E               ValueError: Duplicated timeseries in CollectorRegistry: {'flask_http_request_duration_seconds_bucket', 'flask_http_request_duration_seconds_count', 'flask_http_request_duration_seconds_sum', 'flask_http_request_duration_seconds_created'}

I was able to get rid of the error by instantiating a new CollectorRegistry at every app instantiation.

from flask import Flask
from prometheus_flask_exporter import PrometheusMetrics
from prometheus_client import CollectorRegistry

app1 = Flask(__name__)
metrics1 = PrometheusMetrics(app1, registry=CollectorRegistry())

app2 = Flask(__name__)
metrics2 = PrometheusMetrics(app2, registry=CollectorRegistry())

Thanks!

@rycus86
Copy link
Owner

rycus86 commented Jan 12, 2019

Hi,

Thanks a lot for reporting this!

If this is strictly a testing related problem for you, I've solved it exactly the way you described it, by using a new CollectorRegistry for each test: https://github.com/rycus86/prometheus_flask_exporter/blob/master/tests/unittest_helper.py#L22

There's also a way more hacky approach, where you can unregister all the metrics collectors before each tests, like this: https://github.com/rycus86/webhook-proxy/blob/master/tests/unittest_helper.py#L39

Does this help?

Thanks!

@maximegregoire
Copy link
Author

Thanks for your reply! I ended unregistering the metrics collectors in my test fixture, as you suggested.

@andersbogsnes
Copy link

Hi,

I'm having the same issue as maxime, but the solution provided doesn't work for me. I am using pytest and Gunicorn and using the app_factory approach

My pytest fixture looks like this:

@pytest.fixture()
def app() -> Flask:
    app = create_app('for_p_rejse_cov_ranker_api.config.TestConfig')
    prometheus_client.REGISTRY = prometheus_client.CollectorRegistry(auto_describe=True)
    ctx = app.app_context()
    ctx.push()
    yield app
    ctx.pop()

My app factory registers extensions like this:

from prometheus_flask_exporter.multiprocess import GunicornPrometheusMetrics

cache = Cache()
env = EnvironmentDump()
health = HealthCheck()
metrics = GunicornPrometheusMetrics(app=None, group_by="endpoint")



def setup_extensions(app):
    metrics.init_app(app)
    return app
from flask import Flask
from for_p_rejse_cov_ranker_api.extensions import setup_extensions, metrics
from flask_rest_api import Api
from for_p_rejse_cov_ranker_api.model_api import model_bp

metrics.info('app_info', "Model for Prioritizing Travel Covers", version="0.0.1")


def create_app(config):
    app = Flask("Model for Prioritizing Travel Covers")
    app.config.from_object(config)

    api = Api(app)

    api.register_blueprint(model_bp)

    setup_extensions(app)

    return app

Versions:
prometheus-flask-exporter==0.9.1
flask==1.1.1

When I try that, I still get the Duplicated Timeseries, also when I tried using theunregister_metrics approach.

Any ideas on why it doesn't work?

@rycus86
Copy link
Owner

rycus86 commented Sep 11, 2019

Can you try overwriting the registry first then creating the app here?

@pytest.fixture()
def app() -> Flask:
    app = create_app('for_p_rejse_cov_ranker_api.config.TestConfig')
    prometheus_client.REGISTRY = prometheus_client.CollectorRegistry(auto_describe=True)

I think maybe the problem happens when you instantiate the GunicornPrometheusMetrics object before the Prometheus registry is reset.
It could also be the module level metrics object, can you make it so that you get a new instance of that on each test case? Not a 100% sure if that's a problem, but worth looking into.

@andersbogsnes
Copy link

I changed my testfixture to the following, but unfortunately it still fails... I can get it to pass by changing the scope to session, but it's not ideal.

Not sure how I would make sure that the module-level metrics object is reinstantiated, I would have to patch it out and replicate all the functionality...

@pytest.fixture()
def app() -> Flask:
    prometheus_client.REGISTRY = prometheus_client.CollectorRegistry(auto_describe=True)
    app = create_app('for_p_rejse_cov_ranker_api.config.TestConfig')
    ctx = app.app_context()
    ctx.push()
    yield app

    ctx.pop()

rycus86 added a commit that referenced this issue Sep 12, 2019
@rycus86
Copy link
Owner

rycus86 commented Sep 12, 2019

@andersbogsnes have a look at https://github.com/rycus86/prometheus_flask_exporter/blob/master/examples/pytest-app-factory/test/test_example.py where I tried to (mostly) replicate your use-case.

I ended up having to recreate the metrics object (I used the internal Gunicorn one, but shouldn't matter), then it was working OK:

@pytest.fixture()
def app() -> Flask:
    app = create_app('myapp.config.TestConfig')
    prometheus_client.REGISTRY = prometheus_client.CollectorRegistry(auto_describe=True)
    myapp_extensions.metrics = GunicornInternalPrometheusMetrics(app=None, group_by="endpoint")
    ctx = app.app_context()
    ctx.push()
    yield app
    ctx.pop()

Does this help?

@andersbogsnes
Copy link

Sorry for necroing - just remembered that I hadn't thanked you for putting together the test example!

Great work - much appreciated!

@rycus86
Copy link
Owner

rycus86 commented Sep 26, 2019

Thanks a lot for coming back to say this. :)

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

3 participants