Skip to content

Conversation

xuan-cao-swi
Copy link
Contributor

Description

Based on issue #1431: when a Grape API has multiple versions (for example:

class V5 < Grape::API
  version 'v5', 'v6', 'v7', using: :path

  get '/other' do
    # ...
  end
end

—although using multiple versions in this way is not best practice and may be rare), the route string can become /api/[\"v5\", \"v6\", \"v7\"]//other, which, as the user points out, is not well formatted (see attributes).

attributes=
  {"http.request.method"=>"GET",
   "server.address"=>"localhost:3000",
   "url.scheme"=>"http",
   "url.path"=>"/api/v5/other",
   "user_agent.original"=>"curl/7.74.0",
   "sw.is_entry_span"=>true,
   "code.namespace"=>"V5",
   "http.route"=>"/api/[\"v5\", \"v6\", \"v7\"]//other",
   "http.response.status_code"=>200}

This PR adds a configuration option that allows users to format the version string as they prefer. For example, to produce /api/{v5|v6|v7}/other, a user can provide a lambda like:

OpenTelemetry::SDK.configure do |c|
  c.use 'OpenTelemetry::Instrumentation::Grape',
    {
      version_format: ->(version) {
        version.is_a?(Array) ? "{#{version.join('|')}}" : version.to_s
      }
    }
end

If the user does not provide a config option, the instrumentation keeps the original formatting (for example: "/api/[\"v5\", \"v6\", \"v7\"]//other").

@ericmustin
Copy link
Contributor

@muripic Would you have any time to take a look at this? I believe you contributed the Graph instrumentation (albeit, many years ago), your input would be super helpful.

At a high level I think we'd prefer to solve this within the instrumentation code itself rather than need to support another config option like a callable.

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

Successfully merging this pull request may close these issues.

2 participants