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.

Sign up for GitHub

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

Jump to bottom

Extend instrumentation hook to permit distributed tracing #900

Closed
rossatclutter opened this issue Jan 30, 2020 · 3 comments · Fixed by #902
Closed

Extend instrumentation hook to permit distributed tracing #900

rossatclutter opened this issue Jan 30, 2020 · 3 comments · Fixed by #902

Comments

@rossatclutter
Copy link

rossatclutter commented Jan 30, 2020 •
edited
Loading

#870 added a callback hook for adding metrics after an API request is completed. I'd like to instrument the Stripe client with Honeycomb spans, but this requires having a hook before the HTTP request starts and then a hook when the HTTP request ends, with some way of passing a variable between the two hooks for a single request. This would be necessary for nearly any distributed tracing integration in Ruby. Any chance the instrumentation hook could be extended for surfacing the start request event? Thanks!
@brandur-stripe
Copy link
Contributor

Spoke to a few people internally and I think this would be fine to add.

Our biggest problem is just naming — @remi-stripe brought up that we actually have this in place already in stripe-node, but unfortunately there we called the two callbacks request and response. In stripe-ruby, our request callback is really more like Node's response — it might be possible to remap stripe-ruby to be the same with some difficulty, but it's probably not worth it.

Instead, we should probably do with something like request_begin and request_end.

brandur-stripe pushed a commit that referenced this issue Feb 10, 2020
@brandur
Add request_begin instrumentation callback
b3fc7f0 
Adds a new instrumentation callback called `request_begin` which, as the
name suggests, is invoked before an HTTP request is dispatched. As
outlined originally in #900, the idea is that this will enable a set of
hooks that can be used for distributed tracing.

The PR also renames the existing `request` callback to `request_end`,
although the old name is still invoked for the time being for backwards
compatibility.

A special `user_data` property is passed to `request_begin` which allows
subscribers to set custom data that will be passed through to
`request_end` for any given request. This allows, for example, a user
assigned ID to be set for the request and recognized on both ends.

I chose the naming `_begin` and `_end` (as opposed to start/finish or
any other combination) based on the naming conventions of Ruby itself.

Fixes #900.
@brandur brandur mentioned this issue Feb 10, 2020
Merged
brandur-stripe pushed a commit that referenced this issue Feb 10, 2020
@brandur
Add request_begin instrumentation callback
ba30f14 
Adds a new instrumentation callback called `request_begin` which, as the
name suggests, is invoked before an HTTP request is dispatched. As
outlined originally in #900, the idea is that this will enable a set of
hooks that can be used for distributed tracing.

The PR also renames the existing `request` callback to `request_end`,
although the old name is still invoked for the time being for backwards
compatibility.

A special `user_data` property is passed to `request_begin` which allows
subscribers to set custom data that will be passed through to
`request_end` for any given request. This allows, for example, a user
assigned ID to be set for the request and recognized on both ends.

I chose the naming `_begin` and `_end` (as opposed to start/finish or
any other combination) based on the naming conventions of Ruby itself.

Fixes #900.
@brandur-stripe
Copy link
Contributor

Posted a patch over in #902.

brandur-stripe pushed a commit that referenced this issue Feb 10, 2020
@brandur
Add request_begin instrumentation callback (#902)
81407b6 
Adds a new instrumentation callback called `request_begin` which, as the
name suggests, is invoked before an HTTP request is dispatched. As
outlined originally in #900, the idea is that this will enable a set of
hooks that can be used for distributed tracing.

The PR also renames the existing `request` callback to `request_end`,
although the old name is still invoked for the time being for backwards
compatibility.

A special `user_data` property is passed to `request_begin` which allows
subscribers to set custom data that will be passed through to
`request_end` for any given request. This allows, for example, a user
assigned ID to be set for the request and recognized on both ends.

I chose the naming `_begin` and `_end` (as opposed to start/finish or
any other combination) based on the naming conventions of Ruby itself.

Fixes #900.
@brandur-stripe
Copy link
Contributor

Released as 5.15.0. See README for usage details.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add request_begin instrumentation callback stripe/stripe-ruby
2 participants
@brandur-stripe @rossatclutter