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

Fix entry data bleed when using WithContext and WithTime #1072

Merged
merged 2 commits into from
Feb 25, 2020

Conversation

taywrobel
Copy link
Contributor

@taywrobel taywrobel commented Nov 28, 2019

Creates a copy of the Data map when using WithContext to create a
child Entry. Without this, the data map of the parent Entry,
which is exposed in the Entry struct, is shared between a parent
and all children instances.

This can create bugs of shared or overwritten data when a parent
Entry is used to make children in differing contexts, and behaves
differently than WithField and its derivatives which does make
a copy of the Data map.

Additionally implements the same logic for WithTime, for API
consistency in behavior.

Creates a copy of the data map when using WithContext to create a
child entity.  Without this, the data map of the parent entitiy,
which is exposed in the entity struct, is shared between a parent
and all children instances.

This can create bugs of shared or overwritten data when a parent
entity is used to make children in differing contexts, and behaves
differently than `WithField` and its diritivites which does make
a copy of the data.

Additionally implements the same logic for WithTime, for API
consistency in behavior.
@taywrobel
Copy link
Contributor Author

@dgsb Looks like you're one of the more active maintainers here. If this LGTY (or another maintainer) I can add an entry to the changelog if you'd like me to, or you're welcome to.

This change maintains API compatibility but technically breaks backwards compatibility if a user is reliant on the bug to share data amongst entities. However, I don't think that's a significant enough risk to justify a major version release as strict semver would dictate, so I was thinking a patch release to 1.4.3 would be best?

Clarifies the data used in the EntryWithContextCopiesData test and
adds an equivalent test to verify the behavior of WithTime.
@taywrobel taywrobel changed the title Fix entity data bleed when using WithContext and WithTime Fix entry data bleed when using WithContext and WithTime Dec 4, 2019
@taywrobel
Copy link
Contributor Author

@freeformzSFDC Any chance you can review this and get it published? We're needing to do a pretty questionable workaround for this right now, and having an upstream fix available would be 🍬

@markphelps markphelps merged commit 2f069dd into sirupsen:master Feb 25, 2020
cgxxv pushed a commit to cgxxv/logrus that referenced this pull request Mar 25, 2022
…sEntities

Fix entry data bleed when using WithContext and WithTime
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants