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

"Smart and None Smart Options" breaks Metrics #1417

Closed
softberry opened this issue Mar 3, 2022 · 2 comments
Closed

"Smart and None Smart Options" breaks Metrics #1417

softberry opened this issue Mar 3, 2022 · 2 comments

Comments

@softberry
Copy link

softberry commented Mar 3, 2022

Thumbor request URL

Thumbor can not build

Expected behaviour

Thumbor should run

Actual behaviour

Gives following error on build:

ValueError: Duplicated timeseries in CollectorRegistry: 
{
        'thumbor_response_none__smart_created', 
        'thumbor_response_none__smart'
}

Operating system

All operationg systems

Your thumbor.conf

....
PROMETHEUS_SCRAPE_PORT = 4001
METRICS = 'tc_prometheus.metrics.prometheus_metrics'
...

Description :

This change possibly introduces a braking issue for metrics usages :

I could not find the discussions on this issue and documentation about smart & none smart options. Could you please explain what is intended for those smart and none_smart options? Maybe I could implement that usage for my implementation to fix below explained bug.

In my case,
Sample usage of prometheus metrics with thumbor community plugin tc_prometheus

self.context.metrics.incr('response.smart')
self.context.metrics.timing('response.smart', total_time)
....
self.context.metrics.incr('response.none_smart')
self.context.metrics.timing('response.none_smart', total_time)

as well as : thumbor/loaders/http_loader.py
PS: This code is now slightly changed but keeps the same buggy behavior:

    context.metrics.timing(
            f"original_image.fetch.{code}.{netloc}",
            (finish - req_start).total_seconds() * 1000,
        )
        context.metrics.incr(
            f"original_image.fetch.{code}.{netloc}",
        )

Above lines from from : thumbor/handlers/init.py are trying to create duplicate entry in CollectorRegistry which resulting following errors:

ValueError: Duplicated timeseries in CollectorRegistry: 
{
        'thumbor_response_none__smart_created', 
        'thumbor_response_none__smart'
}

In all other implementations of metrics.timing and metrics.incr metric names are always different. For example:

        self.context.metrics.timing("response.time.{0}".format(status), total_time)
        self.context.metrics.incr("response.status.{0}".format(status))

As seen above, Metric entries must be unique. Above implementations can be done by extending BaseMetrics for the specific use cases but not within thumbor. Therefore these changes are hidden breaking issue since v7.0.0b1

Currently I am ignoring those calls to create a clean Metrics Collection as work around.

@heynemann
Copy link
Member

Fix released under thumbor 7.0.8. Thanks for the thorough report!

@pbabilas
Copy link

pbabilas commented Jun 6, 2022

v7.0.10 - problem still occures. :(

  File "/usr/local/lib/python3.9/site-packages/tornado/web.py", line 1704, in _execute
    result = await result
  File "/usr/local/lib/python3.9/site-packages/thumbor/handlers/imaging.py", line 119, in get
    return await self.check_image(kw)
  File "/usr/local/lib/python3.9/site-packages/merce_thumbor/handlers/imaging.py", line 27, in check_image
    return await super(ImagingHandler, self).check_image(kw)
  File "/usr/local/lib/python3.9/site-packages/thumbor/handlers/imaging.py", line 116, in check_image
    return await self.execute_image_operations()
  File "/usr/local/lib/python3.9/site-packages/thumbor/handlers/__init__.py", line 130, in execute_image_operations
    self.context.metrics.timing("response.smart", total_time)
  File "/usr/local/lib/python3.9/site-packages/tc_prometheus/metrics/prometheus_metrics.py", line 51, in timing
    Metrics.summaries[name] = Summary(name, name, labels.keys())
  File "/usr/local/lib/python3.9/site-packages/prometheus_client/metrics.py", line 143, in __init__
    registry.register(self)
  File "/usr/local/lib/python3.9/site-packages/prometheus_client/registry.py", line 43, in register
    raise ValueError(
ValueError: Duplicated timeseries in CollectorRegistry: {'thumbor_response_smart_created', 'thumbor_response_smart'}```

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