search

discourse / prometheus_exporter

A framework for collecting and aggregating prometheus metrics
MIT License
538 stars 156 forks source link

Fix MethodProfiler to preserve the target class behavior #254

Closed oakcask closed 2 years ago

oakcask commented 2 years ago

I suspect that the change introduced by https://github.com/discourse/prometheus_exporter/pull/250/files#diff-cb9a335ee1d17a77e55d1bc374c4c76ebbf05e6b84de4ce41f1fbc560bda41b7R68 applies the patch to the original class. It causes NoMethodError and breaks the bahavior when the original class doesn't have super class.

I think this will fix https://github.com/discourse/prometheus_exporter/issues/252 .

SamSaffron commented 2 years ago

Oh nice catch! thanks for adding the test as well.

SamSaffron commented 2 years ago

getting a stack level too deep in some of the tests, can you have a look?

oakcask commented 2 years ago

@SamSaffron

Thank you for keeping in touch.

In commit c8976ce, put out invocation of PrometheusExporter::Instrumentation::MethodProfiler.patch from setup because the MethodProfiler.patch cannot be reverted cleanly.

SamSaffron commented 2 years ago

cool looks good, will do a version release shortly