Skip to content

Commit

Permalink
Make observation return its context and immutable access to parent (#…
Browse files Browse the repository at this point in the history
…3423)

* Make Observation#getContext() return Context

* Introduce ObservationView to access parent context immutably

To prevent modifying the parent observation, introduces the
`ObservationView`, a read only view of the observation.  Then, makes
`Observation.Context#getParentObservation` to return the
`ObservationView` which gives immutable way of referencing the context.
  • Loading branch information
ttddyy authored Sep 20, 2022
1 parent 484ad98 commit d054e63
Show file tree
Hide file tree
Showing 6 changed files with 71 additions and 26 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import io.micrometer.common.KeyValue;
import io.micrometer.common.KeyValues;
import io.micrometer.observation.Observation;
import io.micrometer.observation.ObservationView;
import org.assertj.core.api.AbstractAssert;
import org.assertj.core.api.AbstractThrowableAssert;
import org.assertj.core.api.ThrowingConsumer;
Expand Down Expand Up @@ -354,9 +355,9 @@ public SELF hasParentObservation() {
return (SELF) this;
}

private Observation checkedParentObservation() {
private ObservationView checkedParentObservation() {
isNotNull();
Observation p = this.actual.getParentObservation();
ObservationView p = this.actual.getParentObservation();
if (p == null) {
failWithMessage("Observation should have a parent");
}
Expand All @@ -371,7 +372,7 @@ private Observation checkedParentObservation() {
*/
public SELF hasParentObservationEqualTo(Observation expectedParent) {
isNotNull();
Observation realParent = this.actual.getParentObservation();
ObservationView realParent = this.actual.getParentObservation();
if (realParent == null) {
failWithMessage("Observation should have parent <%s> but has none", expectedParent);
}
Expand Down Expand Up @@ -403,9 +404,9 @@ public SELF doesNotHaveParentObservation() {
*/
public SELF hasParentObservationContextSatisfying(
ThrowingConsumer<Observation.ContextView> parentContextViewAssertion) {
Observation p = checkedParentObservation();
ObservationView p = checkedParentObservation();
try {
parentContextViewAssertion.accept(p.getContext());
parentContextViewAssertion.accept(p.getContextView());
}
catch (Throwable e) {
failWithMessage("Parent observation does not satisfy given assertion: " + e.getMessage());
Expand All @@ -423,8 +424,8 @@ public SELF hasParentObservationContextSatisfying(
*/
public SELF hasParentObservationContextMatching(
Predicate<? super Observation.ContextView> parentContextViewPredicate) {
Observation p = checkedParentObservation();
if (!parentContextViewPredicate.test(p.getContext())) {
ObservationView p = checkedParentObservation();
if (!parentContextViewPredicate.test(p.getContextView())) {
failWithMessage("Observation should have parent that matches given predicate but <%s> didn't", p);
}
return (SELF) this;
Expand All @@ -438,8 +439,8 @@ public SELF hasParentObservationContextMatching(
*/
public SELF hasParentObservationContextMatching(
Predicate<? super Observation.ContextView> parentContextViewPredicate, String description) {
Observation p = checkedParentObservation();
if (!parentContextViewPredicate.test(p.getContext())) {
ObservationView p = checkedParentObservation();
if (!parentContextViewPredicate.test(p.getContextView())) {
failWithMessage("Observation should have parent that matches '%s' predicate but <%s> didn't", description,
p);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ final class NoopObservation implements Observation {
*/
static final NoopObservation INSTANCE = new NoopObservation();

private static final ContextView CONTEXT = new Context();
private static final Context CONTEXT = new Context();

private NoopObservation() {
}
Expand Down Expand Up @@ -89,7 +89,7 @@ public Observation start() {
}

@Override
public ContextView getContext() {
public Context getContext() {
return CONTEXT;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@
* @author Marcin Grzejszczak
* @since 1.10.0
*/
public interface Observation {
public interface Observation extends ObservationView {

/**
* No-op observation.
Expand Down Expand Up @@ -359,7 +359,16 @@ default boolean isNoop() {
* Returns the context attached to this observation.
* @return corresponding context
*/
ContextView getContext();
Context getContext();

/**
* Returns the context attached to this observation as a read only view.
* @return corresponding context
*/
@Override
default ContextView getContextView() {
return this.getContext();
}

/**
* Stop the observation. Remember to call this method, otherwise timing calculations
Expand Down Expand Up @@ -674,7 +683,7 @@ class Context implements ContextView {
private Throwable error;

@Nullable
private Observation parentObservation;
private ObservationView parentObservation;

private final Set<KeyValue> lowCardinalityKeyValues = new LinkedHashSet<>();

Expand Down Expand Up @@ -716,20 +725,19 @@ public void setContextualName(@Nullable String contextualName) {
}

/**
* Returns the parent {@link Observation}.
* Returns the parent {@link ObservationView}.
* @return parent observation or {@code null} if there was no parent
*/
@Override
@Nullable
public Observation getParentObservation() {
public ObservationView getParentObservation() {
return parentObservation;
}

/**
* Sets the parent {@link Observation}.
* @param parentObservation parent observation to set
*/
public void setParentObservation(@Nullable Observation parentObservation) {
public void setParentObservation(@Nullable ObservationView parentObservation) {
this.parentObservation = parentObservation;
}

Expand Down Expand Up @@ -1011,11 +1019,11 @@ interface ContextView {
String getContextualName();

/**
* Returns the parent {@link Observation}.
* Returns the parent {@link ObservationView}.
* @return parent observation or {@code null} if there was no parent
*/
@Nullable
Observation getParentObservation();
ObservationView getParentObservation();

/**
* Optional error that occurred while processing the {@link Observation}.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
/*
* Copyright 2022 VMware, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package io.micrometer.observation;

import io.micrometer.observation.Observation.ContextView;

/**
* Read only view on the {@link Observation}.
*
* @since 1.10.0
*/
public interface ObservationView {

/**
* Returns the {@link ContextView} attached to this observation.
* @return corresponding context
*/
ContextView getContextView();

}
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ public Observation start() {
}

@Override
public ContextView getContext() {
public Context getContext() {
return this.context;
}

Expand Down Expand Up @@ -232,13 +232,13 @@ static class SimpleScope implements Scope {
private final Observation.Scope previousObservationScope;

@Nullable
private final Observation previousParentObservation;
private final ObservationView previousParentObservationView;

SimpleScope(ObservationRegistry registry, SimpleObservation current) {
this.registry = registry;
this.currentObservation = current;
this.previousObservationScope = registry.getCurrentObservationScope();
this.previousParentObservation = current.context.getParentObservation();
this.previousParentObservationView = current.context.getParentObservation();
if (this.previousObservationScope != null) {
current.context.setParentObservation(this.previousObservationScope.getCurrentObservation());
}
Expand All @@ -254,7 +254,7 @@ public Observation getCurrentObservation() {
public void close() {
this.registry.setCurrentObservationScope(previousObservationScope);
this.currentObservation.notifyOnScopeClosed();
this.currentObservation.context.setParentObservation(this.previousParentObservation);
this.currentObservation.context.setParentObservation(this.previousParentObservationView);
}

}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,10 @@ void settingParentObservationMakesAReferenceOnParentContext() {
parent.stop();
child.stop();

assertThat(childContext.getParentObservation().getContext()).isSameAs(parentContext);
assertThat(child.getContextView()).isSameAs(childContext);
assertThat(parent.getContextView()).isSameAs(parentContext);

assertThat(childContext.getParentObservation().getContextView()).isSameAs(parentContext);
}

@Test
Expand All @@ -105,7 +108,7 @@ void settingScopeMakesAReferenceOnParentContext() {
parent.scoped(() -> {
assertThat(childContext.getParentObservation()).isNull();
Observation.createNotStarted("child", childContext, registry).observe(() -> {
assertThat(childContext.getParentObservation().getContext()).isSameAs(parentContext);
assertThat(childContext.getParentObservation().getContextView()).isSameAs(parentContext);
});
assertThat(childContext.getParentObservation()).isNull();
});
Expand Down

0 comments on commit d054e63

Please sign in to comment.