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.

Sign up for GitHub

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

Jump to bottom

LettuceObservationContext should be constructed with parent Observation if exists #2591

Closed
quaff opened this issue May 31, 2023 · 1 comment
Closed

LettuceObservationContext should be constructed with parent Observation if exists #2591

quaff opened this issue May 31, 2023 · 1 comment
Labels
type: bug A general bug
Milestone
3.1.1 (2023.0.1)

Comments

@quaff
Copy link
Contributor

quaff commented May 31, 2023 •
edited
Loading

Currently LettuceObservationContext.getParentObservation() rely on micrometer default implementation, which get current observation from ThreadLocal, but lettuce is executed in its own thread other than parent observation's thread, which lead to LettuceObservationContext.getParentObservation() be null even it called within a parent observation scope, LettuceObservationContext.setParentObservation() should be called before observation created, then we could use LettuceObservationContext.getParentObservation() in ObservationPredicate, see also micrometer-metrics/micrometer#3867.

https://github.com/micrometer-metrics/micrometer/blob/main/micrometer-observation/src/main/java/io/micrometer/observation/SimpleObservationRegistry.java#L30-L47
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label May 31, 2023
quaff added a commit to quaff/spring-data-redis that referenced this issue May 31, 2023
@quaff
Construct LettuceObservationContext with parent observation
c989438 
After this commit, LettuceObservationContext.setParentObservation() is called right after Context rather than Observation created, then Context.getParentObservation() could be used in ObservationPredicate to determine whether Observation should be created.

Fix spring-projectsGH-2591
@quaff quaff mentioned this issue May 31, 2023
Closed
4 tasks
quaff added a commit to quaff/spring-data-redis that referenced this issue May 31, 2023
@quaff
Construct LettuceObservationContext with parent observation
7c7e4c2 
After this commit, LettuceObservationContext.setParentObservation() is called right after Context rather than Observation created, then Context.getParentObservation() could be used in ObservationPredicate to determine whether Observation should be created.

Fix spring-projectsGH-2591
@quaff
Copy link
Contributor Author

quaff commented May 31, 2023

Superseded by #2592

@quaff quaff closed this as completed May 31, 2023
@mp911de mp911de added type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged labels Jun 8, 2023
@mp911de mp911de added this to the 3.1.1 (2023.0.1) milestone Jun 8, 2023
@mp911de mp911de changed the title LettuceObservationContext should be constructed with parent Observation if exists LettuceObservationContext should be constructed with parent Observation if exists Jun 8, 2023
mp911de pushed a commit that referenced this issue Jun 8, 2023
@quaff @mp911de
Construct LettuceObservationContext with parent observation
a9e439c 
After this commit, LettuceObservationContext.setParentObservation() is called right after Context rather than Observation created, then Context.getParentObservation() could be used in ObservationPredicate to determine whether Observation should be created.

Fixes #2591
Original pull request: #2592
mp911de added a commit that referenced this issue Jun 8, 2023
@mp911de
Polishing.
ab72d44 
Reformat code. Fix typo.

See #2591
Original pull request: #2592
mp911de pushed a commit that referenced this issue Jun 8, 2023
@quaff @mp911de
Construct LettuceObservationContext with parent observation
1069ede 
After this commit, LettuceObservationContext.setParentObservation() is called right after Context rather than Observation created, then Context.getParentObservation() could be used in ObservationPredicate to determine whether Observation should be created.

Fixes #2591
Original pull request: #2592
mp911de added a commit that referenced this issue Jun 8, 2023
@mp911de
Polishing.
5949d25 
Reformat code. Fix typo.

See #2591
Original pull request: #2592
jxblum pushed a commit to jxblum/spring-data-redis that referenced this issue Jun 8, 2023
@quaff @jxblum
Construct LettuceObservationContext with parent observation
734a359 
After this commit, LettuceObservationContext.setParentObservation() is called right after Context rather than Observation created, then Context.getParentObservation() could be used in ObservationPredicate to determine whether Observation should be created.

Fixes spring-projects#2591
Original pull request: spring-projects#2592
jxblum pushed a commit to jxblum/spring-data-redis that referenced this issue Jun 8, 2023
@mp911de @jxblum
Polishing.
14571a5 
Reformat code. Fix typo.

See spring-projects#2591
Original pull request: spring-projects#2592
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
type: bug A general bug
Projects
None yet
Milestone
3.1.1 (2023.0.1)
Development

Successfully merging a pull request may close this issue.

Construct LettuceObservationContext with parent observation quaff/spring-data-redis
3 participants
@quaff @mp911de @spring-projects-issues