Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

add custom data to the correct payload key #473

Merged
merged 2 commits into from
Feb 10, 2025

Conversation

waltjones
Copy link
Contributor

@waltjones waltjones commented Feb 10, 2025

Description of the change

The PR writes extra_data to the custom key of the payload as specified in the payload schema: https://docs.rollbar.com/reference/create-item

Pyrollbar exception payloads already do this correctly, and the change only affects message payloads. The impact of the current bug is that the custom data is discarded in the pipeline, doesn't get stored or indexed with the occurrence, and therefore cannot be searched.

This PR treats the change as non-breaking, since the custom data can currently only be found in the raw json of the occurrence, and is discarded everywhere else. One example where this is used is the "Params" section of the item detail page in the Rollbar app. After this change, the custom data would still be visible there, but with key names consistent with other items.

If we do consider this a breaking change and want to avoid that, the PR can be updated to write the data in both places, with the tradeoff of a larger payload.

Update:
Updated to preserve the previous location, to be on the side of caution re. the breaking behavior.

Type of change

  • Bug fix (non-breaking change that fixes an issue)

Development

  • Lint rules pass locally
  • The code changed/added as part of this pull request has been covered with tests
  • All tests related to the changed code pass in development

Code review

  • This pull request has a descriptive title and information useful to a reviewer. There may be a screenshot or screencast attached

@brianr
Copy link
Member

brianr commented Feb 10, 2025

👍 Agree with this change. Given the behavior change I think it warrants a minor version bump, i.e. 1.2.0.

Copy link
Collaborator

@danielmorell danielmorell left a comment

Choose a reason for hiding this comment

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

Looks good!

@waltjones waltjones merged commit 49f1b30 into master Feb 10, 2025
89 checks passed
@waltjones waltjones deleted the waltjones/message-payload-custom-data branch February 10, 2025 22:15
@danielmorell danielmorell added this to the 1.2.0 milestone Feb 10, 2025
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants