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

Refactor Shoryuken to not use monitor_transaction #1170

Merged
merged 2 commits into from
Jul 10, 2024

Conversation

tombruijn
Copy link
Member

@tombruijn tombruijn commented Jul 9, 2024

Based on #1169

Move Shoryuken middleware to integrations

Move the ShoryukenMiddleware to the integrations directory, matching how we organize other integrations: a separate hook and integrations file.

Refactor Shoryuken to not use monitor_transaction

Refactor the Shoryuken integration not to use the monitor_transaction helper. A helper we want to deprecate.

The implementation is mostly the same, except how we set the attributes. I have changed it to store them as tags. Previously we set the attributes as "sample data metadata". This is another way to set metadata/tags in the transaction API. All three data types end up in the "overview" tag box, so it doesn't really matter how we set the values.

The only thing is that the sample metadata seems to support more value types than tags. I changed this in PR #1169 by accepting boolean values in tags.

For reference, here's a single and matched message's attributes Shoryuken can receive.

# Single
{"queue"=>"default",
 "SenderId"=>"AIDAIT2UOQQY3AUEKVGXU",
 "SentTimestamp"=>"1720542601233",
 "ApproximateReceiveCount"=>"1",
 "ApproximateFirstReceiveTimestamp"=>"1720542601319",
 "message_id"=>"9b40dbd8-5c26-40ea-8ded-6d68428072fb"}

# Batched
{"queue"=>"batched",
 "SenderId"=>"AIDAIT2UOQQY3AUEKVGXU",
 "SentTimestamp"=>"1720542649820",
 "ApproximateReceiveCount"=>"1",
 "ApproximateFirstReceiveTimestamp"=>"1720542649833",
 "batch"=>true}

[skip changeset] because it's an internal refactor

Move the ShoryukenMiddleware to the integrations directory, matching how
we organize other integrations: a separate hook and integrations file.
@tombruijn tombruijn added the chore label Jul 9, 2024
@tombruijn tombruijn self-assigned this Jul 9, 2024
@tombruijn tombruijn mentioned this pull request Jul 9, 2024
82 tasks
Refactor the Shoryuken integration not to use the `monitor_transaction`
helper. A helper we want to deprecate.

The implementation is mostly the same, except how we set the attributes.
I have changed it to store them as tags. Previously we set the
attributes as "sample data metadata". This is another way to set
metadata/tags in the transaction API. All three data types end up in the
"overview" tag box, so it doesn't really matter how we set the values.

The only thing is that the sample metadata seems to support more value
types than tags. I changed this in PR #1169 by accepting boolean values
in tags.

For reference, here's a single and matched message's attributes
Shoryuken can receive.

```ruby
# Single
{"queue"=>"default",
 "SenderId"=>"AIDAIT2UOQQY3AUEKVGXU",
 "SentTimestamp"=>"1720542601233",
 "ApproximateReceiveCount"=>"1",
 "ApproximateFirstReceiveTimestamp"=>"1720542601319",
 "message_id"=>"9b40dbd8-5c26-40ea-8ded-6d68428072fb"}

# Batched
{"queue"=>"batched",
 "SenderId"=>"AIDAIT2UOQQY3AUEKVGXU",
 "SentTimestamp"=>"1720542649820",
 "ApproximateReceiveCount"=>"1",
 "ApproximateFirstReceiveTimestamp"=>"1720542649833",
 "batch"=>true}
```
@tombruijn tombruijn force-pushed the update-shoryuken-instrumentation branch from 7ff8640 to 658a70d Compare July 9, 2024 16:50
@tombruijn tombruijn merged commit e3a755a into tag-boolean Jul 10, 2024
117 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant