From bc48089a5e8554af35fc80bc240a0c725dfff722 Mon Sep 17 00:00:00 2001 From: Jonatan Ivanov Date: Mon, 9 Dec 2024 17:17:46 -0800 Subject: [PATCH] Work around a concurrent modification issue of KeyValues --- .../core/ObservationKeyValuesBenchmark.java | 73 +++++++++++++++++++ .../ObservationContextConcurrencyTest.java | 71 ++++++++++++++++++ .../java/io/micrometer/common/KeyValues.java | 7 +- 3 files changed, 150 insertions(+), 1 deletion(-) create mode 100644 benchmarks/benchmarks-core/src/jmh/java/io/micrometer/benchmark/core/ObservationKeyValuesBenchmark.java create mode 100644 concurrency-tests/src/jcstress/java/io/micrometer/concurrencytests/ObservationContextConcurrencyTest.java diff --git a/benchmarks/benchmarks-core/src/jmh/java/io/micrometer/benchmark/core/ObservationKeyValuesBenchmark.java b/benchmarks/benchmarks-core/src/jmh/java/io/micrometer/benchmark/core/ObservationKeyValuesBenchmark.java new file mode 100644 index 0000000000..d92f9d48aa --- /dev/null +++ b/benchmarks/benchmarks-core/src/jmh/java/io/micrometer/benchmark/core/ObservationKeyValuesBenchmark.java @@ -0,0 +1,73 @@ +/* + * Copyright 2024 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.benchmark.core; + +import io.micrometer.common.KeyValue; +import io.micrometer.common.KeyValues; +import io.micrometer.observation.Observation; +import org.openjdk.jmh.annotations.*; +import org.openjdk.jmh.runner.Runner; +import org.openjdk.jmh.runner.RunnerException; +import org.openjdk.jmh.runner.options.Options; +import org.openjdk.jmh.runner.options.OptionsBuilder; + +import java.util.concurrent.TimeUnit; + +@State(Scope.Benchmark) +@OutputTimeUnit(TimeUnit.MICROSECONDS) +public class ObservationKeyValuesBenchmark { + + private static final KeyValues KEY_VALUES = KeyValues.of("key1", "value1", "key2", "value2", "key3", "value3", + "key4", "value4", "key5", "value5"); + + private static final KeyValue KEY_VALUE = KeyValue.of("testKey", "testValue"); + + @Benchmark + public void noopBaseline() { + } + + @Benchmark + public Observation.Context contextBaseline() { + return new TestContext().addLowCardinalityKeyValues(KEY_VALUES); + } + + @Benchmark + public Observation.Context putKeyValue() { + return new TestContext().addLowCardinalityKeyValues(KEY_VALUES).addLowCardinalityKeyValue(KEY_VALUE); + } + + @Benchmark + public KeyValues readKeyValues() { + return new TestContext().addLowCardinalityKeyValues(KEY_VALUES).getLowCardinalityKeyValues(); + } + + public static void main(String[] args) throws RunnerException { + Options options = new OptionsBuilder().include(ObservationKeyValuesBenchmark.class.getSimpleName()) + .warmupIterations(5) + .measurementIterations(10) + .mode(Mode.SampleTime) + .forks(1) + .build(); + + new Runner(options).run(); + } + + static class TestContext extends Observation.Context { + + } + +} diff --git a/concurrency-tests/src/jcstress/java/io/micrometer/concurrencytests/ObservationContextConcurrencyTest.java b/concurrency-tests/src/jcstress/java/io/micrometer/concurrencytests/ObservationContextConcurrencyTest.java new file mode 100644 index 0000000000..37f38dcb52 --- /dev/null +++ b/concurrency-tests/src/jcstress/java/io/micrometer/concurrencytests/ObservationContextConcurrencyTest.java @@ -0,0 +1,71 @@ +/* + * Copyright 2024 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.concurrencytests; + +import io.micrometer.common.KeyValue; +import io.micrometer.observation.Observation; +import org.openjdk.jcstress.annotations.Actor; +import org.openjdk.jcstress.annotations.JCStressTest; +import org.openjdk.jcstress.annotations.Outcome; +import org.openjdk.jcstress.annotations.State; +import org.openjdk.jcstress.infra.results.Z_Result; + +import java.util.UUID; + +import static org.openjdk.jcstress.annotations.Expect.ACCEPTABLE; +import static org.openjdk.jcstress.annotations.Expect.FORBIDDEN; + +public class ObservationContextConcurrencyTest { + + @JCStressTest + @State + @Outcome(id = "true", expect = ACCEPTABLE) + @Outcome(expect = FORBIDDEN) + public static class ConsistentKeyValues { + + private final Observation.Context context = new TestContext(); + + @Actor + public void read(Z_Result r) { + try { + context.getHighCardinalityKeyValues(); + r.r1 = true; + } + catch (Exception e) { + r.r1 = false; + } + } + + @Actor + public void write(Z_Result r) { + try { + String uuid = UUID.randomUUID().toString(); + context.addHighCardinalityKeyValue(KeyValue.of(uuid, uuid)); + r.r1 = true; + } + catch (Exception e) { + r.r1 = false; + } + } + + } + + static class TestContext extends Observation.Context { + + } + +} diff --git a/micrometer-commons/src/main/java/io/micrometer/common/KeyValues.java b/micrometer-commons/src/main/java/io/micrometer/common/KeyValues.java index 6c32e8f878..e48eaf5c67 100644 --- a/micrometer-commons/src/main/java/io/micrometer/common/KeyValues.java +++ b/micrometer-commons/src/main/java/io/micrometer/common/KeyValues.java @@ -371,7 +371,12 @@ else if (keyValues instanceof KeyValues) { } else if (keyValues instanceof Collection) { Collection keyValuesCollection = (Collection) keyValues; - return toKeyValues(keyValuesCollection.toArray(EMPTY_KEY_VALUE_ARRAY)); + try { + return toKeyValues(keyValuesCollection.toArray(EMPTY_KEY_VALUE_ARRAY)); + } + catch (ArrayIndexOutOfBoundsException exception) { + return of(keyValues); + } } else { return toKeyValues(StreamSupport.stream(keyValues.spliterator(), false).toArray(KeyValue[]::new));