efigence / yabeda-grape

Extensible metrics for monitoring Grape API endpoints
4 stars 1 forks source link

Different Tags on Metric Declarations and Implementation #9

Closed vit-panchuk closed 4 years ago

vit-panchuk commented 4 years ago

Hi, thanks for your work on Grape metrics implementation of Yabeda!

Yesterday I've tried to use it together with yabeda-prometheus, but I could not initialize the metrics with Yabeda::Grape.bind_metrics because of this exception:

Yabeda::Prometheus::Adapter::UndeclaredMetricTags
(Prometheus requires all used tags to be declared at metric registration time.
Please add `tags` option to the declaration of metric `grape_requests_total`.
Error: labels must have the same signature (keys given: [:method, :path, :status] vs. keys expected: [:action, :controller, :format, :method, :status])

Seems like your declared tags differ from tags that actually implemented:

counter   :requests_total, comment: "A counter of the total number of HTTP requests rails processed.",
                                     tags: %i[controller action status format method]
histogram :request_duration, unit: :seconds, buckets: LONG_RUNNING_REQUEST_BUCKETS,
                                     tags: %i[controller action status format method],
                                     comment: "A histogram of the response latency."

https://github.com/efigence/yabeda-grape/blob/master/lib/yabeda/grape.rb#L18-L22

labels = {
  method: event.payload[:endpoint].options[:method].first.downcase,
  path: event.payload[:endpoint].request.path,
  status: event.payload[:endpoint].status
}

https://github.com/efigence/yabeda-grape/blob/master/lib/yabeda/grape.rb#L27-L31

So I've extracted the code of Yabeda::Grapes.bind_metrics to an inializer and changed the tags in metrics declaration to tags: %i[method path status] and everything worked out:

Yabeda.configure do
  group :grape

  counter   :requests_total, comment: 'A counter of the total number of HTTP requests rails processed.',
                             tags: %i[method path status]
  histogram :request_duration, unit: :seconds, buckets: Yabeda::Grape::LONG_RUNNING_REQUEST_BUCKETS,
                               tags: %i[method path status],
                               comment: 'A histogram of the response latency.'

  ActiveSupport::Notifications.subscribe 'endpoint_run.grape' do |*args|
    event = ActiveSupport::Notifications::Event.new(*args)

    labels = {
      method: event.payload[:endpoint].options[:method].first.downcase,
      path: event.payload[:endpoint].request.path,
      status: event.payload[:endpoint].status
    }

    grape_requests_total.increment(labels)
    grape_request_duration.measure(labels, Yabeda::Grape.ms2s(event.duration))
  end
end
Prorok64b commented 4 years ago

@vit-panchuk thanks for your feedback, and I appreciate if you make a pull request with your changes.

vit-panchuk commented 4 years ago

@Prorok64b here we go, I've created a fork and fixed the bug as well as bumped the version to 0.1.3 and created a PR to this repo. Hope, I've done everything correctly.

Prorok64b commented 4 years ago

@vit-panchuk thanks, new version is published on rubygems.