From 9b20ca516dff96951613b5a4590a575969ad57ae Mon Sep 17 00:00:00 2001 From: Tadaya Tsuyukubo Date: Mon, 19 Sep 2022 10:58:54 -0700 Subject: [PATCH 1/2] Make Observation#getContext() return Context --- .../java/io/micrometer/observation/NoopObservation.java | 4 ++-- .../main/java/io/micrometer/observation/Observation.java | 6 ++++-- .../java/io/micrometer/observation/SimpleObservation.java | 2 +- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/micrometer-observation/src/main/java/io/micrometer/observation/NoopObservation.java b/micrometer-observation/src/main/java/io/micrometer/observation/NoopObservation.java index 8312a52768..b11abdd188 100644 --- a/micrometer-observation/src/main/java/io/micrometer/observation/NoopObservation.java +++ b/micrometer-observation/src/main/java/io/micrometer/observation/NoopObservation.java @@ -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() { } @@ -89,7 +89,7 @@ public Observation start() { } @Override - public ContextView getContext() { + public Context getContext() { return CONTEXT; } diff --git a/micrometer-observation/src/main/java/io/micrometer/observation/Observation.java b/micrometer-observation/src/main/java/io/micrometer/observation/Observation.java index 90b35c1b41..4f1c2c3343 100644 --- a/micrometer-observation/src/main/java/io/micrometer/observation/Observation.java +++ b/micrometer-observation/src/main/java/io/micrometer/observation/Observation.java @@ -356,10 +356,12 @@ default boolean isNoop() { Observation start(); /** - * Returns the context attached to this observation. + * Returns the context attached to this observation. This returns a mutable + * {@link Context} for this observation. However, it is strongly discouraged to modify + * the {@link Context} in the parent observation. * @return corresponding context */ - ContextView getContext(); + Context getContext(); /** * Stop the observation. Remember to call this method, otherwise timing calculations diff --git a/micrometer-observation/src/main/java/io/micrometer/observation/SimpleObservation.java b/micrometer-observation/src/main/java/io/micrometer/observation/SimpleObservation.java index 5f1ea61fc3..943999ead8 100644 --- a/micrometer-observation/src/main/java/io/micrometer/observation/SimpleObservation.java +++ b/micrometer-observation/src/main/java/io/micrometer/observation/SimpleObservation.java @@ -145,7 +145,7 @@ public Observation start() { } @Override - public ContextView getContext() { + public Context getContext() { return this.context; } From c6c2fac18b8b905965eb8437c3c4b78f204406bd Mon Sep 17 00:00:00 2001 From: Tadaya Tsuyukubo Date: Mon, 19 Sep 2022 11:56:57 -0700 Subject: [PATCH 2/2] 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. --- .../tck/ObservationContextAssert.java | 19 ++++++----- .../micrometer/observation/Observation.java | 28 +++++++++------- .../observation/ObservationView.java | 33 +++++++++++++++++++ .../observation/SimpleObservation.java | 6 ++-- .../observation/ObservationTests.java | 7 ++-- 5 files changed, 68 insertions(+), 25 deletions(-) create mode 100644 micrometer-observation/src/main/java/io/micrometer/observation/ObservationView.java diff --git a/micrometer-observation-test/src/main/java/io/micrometer/observation/tck/ObservationContextAssert.java b/micrometer-observation-test/src/main/java/io/micrometer/observation/tck/ObservationContextAssert.java index 4bffdc918f..0462ab7bed 100644 --- a/micrometer-observation-test/src/main/java/io/micrometer/observation/tck/ObservationContextAssert.java +++ b/micrometer-observation-test/src/main/java/io/micrometer/observation/tck/ObservationContextAssert.java @@ -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; @@ -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"); } @@ -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); } @@ -403,9 +404,9 @@ public SELF doesNotHaveParentObservation() { */ public SELF hasParentObservationContextSatisfying( ThrowingConsumer 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()); @@ -423,8 +424,8 @@ public SELF hasParentObservationContextSatisfying( */ public SELF hasParentObservationContextMatching( Predicate 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; @@ -438,8 +439,8 @@ public SELF hasParentObservationContextMatching( */ public SELF hasParentObservationContextMatching( Predicate 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); } diff --git a/micrometer-observation/src/main/java/io/micrometer/observation/Observation.java b/micrometer-observation/src/main/java/io/micrometer/observation/Observation.java index 4f1c2c3343..af26f4cc7c 100644 --- a/micrometer-observation/src/main/java/io/micrometer/observation/Observation.java +++ b/micrometer-observation/src/main/java/io/micrometer/observation/Observation.java @@ -44,7 +44,7 @@ * @author Marcin Grzejszczak * @since 1.10.0 */ -public interface Observation { +public interface Observation extends ObservationView { /** * No-op observation. @@ -356,13 +356,20 @@ default boolean isNoop() { Observation start(); /** - * Returns the context attached to this observation. This returns a mutable - * {@link Context} for this observation. However, it is strongly discouraged to modify - * the {@link Context} in the parent observation. + * Returns the context attached to this observation. * @return corresponding context */ 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 * won't be finished. @@ -676,7 +683,7 @@ class Context implements ContextView { private Throwable error; @Nullable - private Observation parentObservation; + private ObservationView parentObservation; private final Set lowCardinalityKeyValues = new LinkedHashSet<>(); @@ -718,12 +725,11 @@ 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; } @@ -731,7 +737,7 @@ public Observation getParentObservation() { * 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; } @@ -1013,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}. diff --git a/micrometer-observation/src/main/java/io/micrometer/observation/ObservationView.java b/micrometer-observation/src/main/java/io/micrometer/observation/ObservationView.java new file mode 100644 index 0000000000..a3693861f5 --- /dev/null +++ b/micrometer-observation/src/main/java/io/micrometer/observation/ObservationView.java @@ -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(); + +} diff --git a/micrometer-observation/src/main/java/io/micrometer/observation/SimpleObservation.java b/micrometer-observation/src/main/java/io/micrometer/observation/SimpleObservation.java index 943999ead8..a664d12c77 100644 --- a/micrometer-observation/src/main/java/io/micrometer/observation/SimpleObservation.java +++ b/micrometer-observation/src/main/java/io/micrometer/observation/SimpleObservation.java @@ -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()); } @@ -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); } } diff --git a/micrometer-observation/src/test/java/io/micrometer/observation/ObservationTests.java b/micrometer-observation/src/test/java/io/micrometer/observation/ObservationTests.java index a70d1121b9..82be5dcb99 100644 --- a/micrometer-observation/src/test/java/io/micrometer/observation/ObservationTests.java +++ b/micrometer-observation/src/test/java/io/micrometer/observation/ObservationTests.java @@ -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 @@ -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(); });