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 #1178

Merged
merged 2 commits into from
Jul 10, 2024

Conversation

tombruijn
Copy link
Member

PR to fix merge for #1170

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
[skip review]

Move the ShoryukenMiddleware to the integrations directory, matching how
we organize other integrations: a separate hook and integrations file.
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 self-assigned this Jul 10, 2024
@tombruijn tombruijn merged commit 5b62733 into main Jul 10, 2024
117 checks passed
@tombruijn tombruijn deleted the update-shoryuken-instrumentation branch July 10, 2024 18:44
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.

None yet

1 participant