Skip to content

Add logic to Sidekiq for ActiveJob log_arguments#1504

Merged
tombruijn merged 1 commit intorespect-activejob-log-arguments-tomfrom
respect-activejob-log-arguments-per-library
Apr 7, 2026
Merged

Add logic to Sidekiq for ActiveJob log_arguments#1504
tombruijn merged 1 commit intorespect-activejob-log-arguments-tomfrom
respect-activejob-log-arguments-per-library

Conversation

@tombruijn
Copy link
Copy Markdown
Member

@tombruijn tombruijn commented Mar 27, 2026

When a customer Configures ActiveJob log_arguments to be false, it would not adhere to this in the other integrations like Sidekiq, DelayedJob, etc.

Implemented this logic in Sidekiq so that it doesn't set the parameters if they're unset on ActiveJob.

Based on #1499 and #1507

@tombruijn tombruijn self-assigned this Mar 27, 2026
@tombruijn tombruijn added the enhancement An improvement to an existing feature. label Mar 27, 2026
When a customer Configures ActiveJob `log_arguments` to be false, it
would not adhere to this in the other integrations like Sidekiq,
DelayedJob, etc.

Implemented this logic in Sidekiq so that it doesn't set the parameters
if they're unset on ActiveJob.
@tombruijn tombruijn changed the base branch from respect-activejob-log-arguments to respect-activejob-log-arguments-tom March 31, 2026 09:36
@tombruijn tombruijn force-pushed the respect-activejob-log-arguments-per-library branch from dcb0af0 to ce1ddd3 Compare March 31, 2026 09:36
if transaction
transaction.add_params_if_nil { parse_arguments(item) }
# If ActiveJob log_arguments is set to false, don't set params
store_arguments = transaction.store("activejob").fetch("log_arguments", true)
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't necessarily love this system but it already exists in the Ruby gem so I tried making it work with this first.

@tombruijn tombruijn requested review from lipskis and unflxw March 31, 2026 09:51
@tombruijn
Copy link
Copy Markdown
Member Author

@unflxw @lipskis What do you think of this as a solution to the problem I pointed out in the other PR? We have to implement this logic for every library that is compatible with ActiveJob.

@tombruijn tombruijn marked this pull request as ready for review March 31, 2026 09:52
@backlog-helper

This comment has been minimized.

2 similar comments
@backlog-helper

This comment has been minimized.

@backlog-helper

This comment has been minimized.

Copy link
Copy Markdown
Contributor

@lipskis lipskis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@tombruijn tombruijn merged commit 51151a9 into respect-activejob-log-arguments-tom Apr 7, 2026
195 checks passed
@tombruijn tombruijn deleted the respect-activejob-log-arguments-per-library branch April 7, 2026 09:09
@backlog-helper
Copy link
Copy Markdown

backlog-helper Bot commented Apr 8, 2026

  • This Pull Request has been closed or merged, but is still in a column that is considered to be 'in progress'. Please move the Pull Request to the 'Done' column when no more work will be done on this. - (More info)

This is a message from the daily scheduled checks.

New issue guide | Backlog management | Rules | Feedback

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement An improvement to an existing feature.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants