Added option to set a parent Observation on an existing Observation #3291
Conversation
without this change in order to e.g. create spans we always need to put an observation in scope so that spans are put in thread local and a child span is being set. That doesn't make a lot of sense when we exactly know what the parent observation is. with this change we're allowing to set a parent observation direcly on an existing observation. That way we're able to maintain a parent child relationship of observations and of e.g. spans
without this change in order to e.g. create spans we always need to put an observation in scope so that spans are put in thread local and a child span is being set. That doesn't make a lot of sense when we exactly know what the parent observation is. with this change we're allowing to set a parent observation direcly on an existing observation. That way we're able to maintain a parent child relationship of observations and of e.g. spans related to micrometer-metrics/micrometer#3291
![sonatype-lift[bot]](https://avatars.githubusercontent.com/in/9843?s=60&v=4){kind=small}
|
|
||
| @Override | ||
| @Nullable | ||
| public <T> T get(Object key) { |
There was a problem hiding this comment.
TypeParameterUnusedInFormals: Declaring a type parameter that is only used in the return type is a misuse of generics: operations on the type parameter are unchecked, it hides unsafe casts at invocations of the method, and it interacts badly with method overload resolution.
Reply with "@sonatype-lift help" for info about LiftBot commands.
Reply with "@sonatype-lift ignore" to tell LiftBot to leave out the above finding from this PR.
Reply with "@sonatype-lift ignoreall" to tell LiftBot to leave out all the findings from this PR and from the status bar in Github.
When talking to LiftBot, you need to refresh the page to see its response. Click here to get to know more about LiftBot commands.
Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]
|
|
||
| @Override | ||
| @NonNull | ||
| public <T> T getRequired(Object key) { |
There was a problem hiding this comment.
TypeParameterUnusedInFormals: Declaring a type parameter that is only used in the return type is a misuse of generics: operations on the type parameter are unchecked, it hides unsafe casts at invocations of the method, and it interacts badly with method overload resolution.
Reply with "@sonatype-lift help" for info about LiftBot commands.
Reply with "@sonatype-lift ignore" to tell LiftBot to leave out the above finding from this PR.
Reply with "@sonatype-lift ignoreall" to tell LiftBot to leave out all the findings from this PR and from the status bar in Github.
When talking to LiftBot, you need to refresh the page to see its response. Click here to get to know more about LiftBot commands.
Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]
| * @return entry ot exception if not present | ||
| */ | ||
| @NonNull | ||
| <T> T getRequired(Object key); |
There was a problem hiding this comment.
TypeParameterUnusedInFormals: Declaring a type parameter that is only used in the return type is a misuse of generics: operations on the type parameter are unchecked, it hides unsafe casts at invocations of the method, and it interacts badly with method overload resolution.
Reply with "@sonatype-lift help" for info about LiftBot commands.
Reply with "@sonatype-lift ignore" to tell LiftBot to leave out the above finding from this PR.
Reply with "@sonatype-lift ignoreall" to tell LiftBot to leave out all the findings from this PR and from the status bar in Github.
When talking to LiftBot, you need to refresh the page to see its response. Click here to get to know more about LiftBot commands.
Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]
| * @return entry or {@code null} if not present | ||
| */ | ||
| @Nullable | ||
| <T> T get(Object key); |
There was a problem hiding this comment.
TypeParameterUnusedInFormals: Declaring a type parameter that is only used in the return type is a misuse of generics: operations on the type parameter are unchecked, it hides unsafe casts at invocations of the method, and it interacts badly with method overload resolution.
Reply with "@sonatype-lift help" for info about LiftBot commands.
Reply with "@sonatype-lift ignore" to tell LiftBot to leave out the above finding from this PR.
Reply with "@sonatype-lift ignoreall" to tell LiftBot to leave out all the findings from this PR and from the status bar in Github.
When talking to LiftBot, you need to refresh the page to see its response. Click here to get to know more about LiftBot commands.
Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]
| @@ -228,6 +228,13 @@ static Observation createNotStarted(ObservationConvention<?> observationConventi | |||
| */ | |||
| Observation contextualName(String contextualName); | |||
|
|
|||
| /** | |||
| * Sets the parent {@link Observation}. | |||
There was a problem hiding this comment.
It would be good for us to explain here why a user would want to do this (avoid needing to scope) and that it isn't needed if you are creating a scope.
|
|
||
| } | ||
|
|
||
| static class ImmutableContext extends Context { |
) * Added option to set a parent Observation on an existing Observation without this change in order to e.g. create spans we always need to put an observation in scope so that spans are put in thread local and a child span is being set. That doesn't make a lot of sense when we exactly know what the parent observation is. with this change we're allowing to set a parent observation direcly on an existing observation. That way we're able to maintain a parent child relationship of observations and of e.g. spans related to micrometer-metrics/micrometer#3291 * Fixed the http observation handlers
without this change in order to e.g. create spans we always need to put an observation in scope so that spans are put in thread local and a child span is being set. That doesn't make a lot of sense when we exactly know what the parent observation is.
with this change we're allowing to set a parent observation direcly on an existing observation. That way we're able to maintain a parent child relationship of observations and of e.g. spans