[HOLD] Unifying ContextSnapshot vocabulary and fixing setAllThreadLocalsFrom #73
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![sonatype-lift[bot]](https://avatars.githubusercontent.com/in/9843?s=60&v=4){kind=small}
context-propagation/src/main/java/io/micrometer/context/ContextSnapshot.java
Outdated
Show resolved
Hide resolved
context-propagation/src/main/java/io/micrometer/context/ContextSnapshot.java
Outdated
Show resolved
Hide resolved
🛠 Lift Auto-fixSome of the Lift findings in this PR can be automatically fixed. You can download and apply these changes in your local project directory of your branch to review the suggestions before committing.1 # Download the patch
curl https://lift.sonatype.com/api/patch/github.com/micrometer-metrics/context-propagation/73.diff -o lift-autofixes.diff
# Apply the patch with git
git apply lift-autofixes.diff
# Review the changes
git diffWant it all in a single command? Open a terminal in your project's directory and copy and paste the following command: curl https://lift.sonatype.com/api/patch/github.com/micrometer-metrics/context-propagation/73.diff | git applyOnce you're satisfied, commit and push your changes in your project. Footnotes |
chemicL
pushed a commit
to reactor/reactor-core
that referenced
this pull request
Feb 10, 2023
…ibrary. Dependent upon merge of micrometer-metrics/context-propagation#73
Recent change to `ContextSnapshot#setThreadLocals()` method allows to clear `ThreadLocal` values not present in the snapshot. The associated change to `ContextSnapshot.setAllThreadLocalsFrom(sourceContext)` should be on par with `ContextSnapshot.captureAll(...)` followed by `ContextSnapshot#setThreadLocals()` but avoiding the temporary snapshot container. Unfortunately, the method clears existing `ThreadLocal` values that are not present in the sourceContext object and creates a mismatch in the vocabulary. The previous behaviour is restored for that static method. Instead, two new static methods are introduced: - `ContextSnapshot.setThreadLocals(Object)` - `ContextSnapshot.setThreadLocals(Object, ContextRegistry)` that allow to skip the temporary container and clear `ThreadLocal` values not present in the sourceContext provided and set only the present ones.
chemicL
force-pushed
the
unify-vocabulary-fix-setallthreadlocalsfrom
branch
from
18e0306 to
aacbb3c
Compare
marcingrzejszczak
approved these changes
Feb 10, 2023
chemicL
changed the title
|
Putting on hold. #74 reverts the previous functionality. This PR should serve as basis for a unification of the API and brining the choice to clear ThreadLocal values that are not present in a source context object vs only setting present values and ignoring others. Both scenarios have their place in application usage patterns and the API should allow an informed interaction. |
|
Closing this one in favour of #105. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Recent change to
ContextSnapshot#setThreadLocals()method allows to clearThreadLocalvalues not present in the snapshot.The associated change to
ContextSnapshot.setAllThreadLocalsFrom(sourceContext)should be on par withContextSnapshot.captureAll(...)followed byContextSnapshot#setThreadLocals()but avoiding the temporary snapshot container.Unfortunately, the method clears existing
ThreadLocalvalues that are not present in the sourceContext object and creates a mismatch in the vocabulary.The previous behaviour is restored for that static method.
Instead, two new static methods are introduced:
ContextSnapshot.setThreadLocals(Object)ContextSnapshot.setThreadLocals(Object, ContextRegistry)that allow to skip the temporary container and clear
ThreadLocalvalues not present in the sourceContext provided and set only the present ones.