From ed6a69bd2230e2770d38f295c7a9fc7113baced6 Mon Sep 17 00:00:00 2001 From: Philipp Page Date: Fri, 14 Nov 2025 14:51:11 +0100 Subject: [PATCH 1/4] Add proxy to manage thread local metrics provider instances to achieve zero-lock thread-safety. --- .../powertools/metrics/MetricsFactory.java | 18 +- .../internal/ThreadLocalMetricsProxy.java | 149 ++++++++++ .../metrics/ConfigurationPrecedenceTest.java | 2 +- .../metrics/MetricsBuilderTest.java | 14 +- .../metrics/MetricsFactoryTest.java | 86 +++++- .../metrics/RequestHandlerTest.java | 2 +- .../internal/EmfMetricsLoggerTest.java | 12 +- .../internal/LambdaMetricsAspectTest.java | 2 +- .../internal/ThreadLocalMetricsProxyTest.java | 267 ++++++++++++++++++ 9 files changed, 525 insertions(+), 27 deletions(-) create mode 100644 powertools-metrics/src/main/java/software/amazon/lambda/powertools/metrics/internal/ThreadLocalMetricsProxy.java create mode 100644 powertools-metrics/src/test/java/software/amazon/lambda/powertools/metrics/internal/ThreadLocalMetricsProxyTest.java diff --git a/powertools-metrics/src/main/java/software/amazon/lambda/powertools/metrics/MetricsFactory.java b/powertools-metrics/src/main/java/software/amazon/lambda/powertools/metrics/MetricsFactory.java index 67ab17b7b..aab10e770 100644 --- a/powertools-metrics/src/main/java/software/amazon/lambda/powertools/metrics/MetricsFactory.java +++ b/powertools-metrics/src/main/java/software/amazon/lambda/powertools/metrics/MetricsFactory.java @@ -16,9 +16,11 @@ import org.crac.Core; import org.crac.Resource; + import software.amazon.lambda.powertools.common.internal.ClassPreLoader; import software.amazon.lambda.powertools.common.internal.LambdaConstants; import software.amazon.lambda.powertools.common.internal.LambdaHandlerProcessor; +import software.amazon.lambda.powertools.metrics.internal.ThreadLocalMetricsProxy; import software.amazon.lambda.powertools.metrics.model.DimensionSet; import software.amazon.lambda.powertools.metrics.provider.EmfMetricsProvider; import software.amazon.lambda.powertools.metrics.provider.MetricsProvider; @@ -28,7 +30,7 @@ */ public final class MetricsFactory implements Resource { private static MetricsProvider provider = new EmfMetricsProvider(); - private static Metrics metrics; + private static ThreadLocalMetricsProxy metricsProxy; // Dummy instance to register MetricsFactory with CRaC private static final MetricsFactory INSTANCE = new MetricsFactory(); @@ -44,23 +46,23 @@ public final class MetricsFactory implements Resource { * @return the singleton Metrics instance */ public static synchronized Metrics getMetricsInstance() { - if (metrics == null) { - metrics = provider.getMetricsInstance(); + if (metricsProxy == null) { + metricsProxy = new ThreadLocalMetricsProxy(provider); // Apply default configuration from environment variables String envNamespace = System.getenv("POWERTOOLS_METRICS_NAMESPACE"); if (envNamespace != null) { - metrics.setNamespace(envNamespace); + metricsProxy.setNamespace(envNamespace); } // Only set Service dimension if it's not the default undefined value String serviceName = LambdaHandlerProcessor.serviceName(); if (!LambdaConstants.SERVICE_UNDEFINED.equals(serviceName)) { - metrics.setDefaultDimensions(DimensionSet.of("Service", serviceName)); + metricsProxy.setDefaultDimensions(DimensionSet.of("Service", serviceName)); } } - return metrics; + return metricsProxy; } /** @@ -73,8 +75,8 @@ public static synchronized void setMetricsProvider(MetricsProvider metricsProvid throw new IllegalArgumentException("Metrics provider cannot be null"); } provider = metricsProvider; - // Reset the metrics instance so it will be recreated with the new provider - metrics = null; + // Reset the metrics proxy so it will be recreated with the new provider + metricsProxy = null; } @Override diff --git a/powertools-metrics/src/main/java/software/amazon/lambda/powertools/metrics/internal/ThreadLocalMetricsProxy.java b/powertools-metrics/src/main/java/software/amazon/lambda/powertools/metrics/internal/ThreadLocalMetricsProxy.java new file mode 100644 index 000000000..fe3e92dcb --- /dev/null +++ b/powertools-metrics/src/main/java/software/amazon/lambda/powertools/metrics/internal/ThreadLocalMetricsProxy.java @@ -0,0 +1,149 @@ +/* + * Copyright 2023 Amazon.com, Inc. or its affiliates. + * 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 + * http://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 software.amazon.lambda.powertools.metrics.internal; + +import java.time.Instant; +import java.util.HashMap; +import java.util.Optional; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicReference; +import java.util.function.Consumer; + +import com.amazonaws.services.lambda.runtime.Context; + +import software.amazon.lambda.powertools.metrics.Metrics; +import software.amazon.lambda.powertools.metrics.model.DimensionSet; +import software.amazon.lambda.powertools.metrics.model.MetricResolution; +import software.amazon.lambda.powertools.metrics.model.MetricUnit; +import software.amazon.lambda.powertools.metrics.provider.MetricsProvider; + +public class ThreadLocalMetricsProxy implements Metrics { + private final InheritableThreadLocal threadLocalMetrics = new InheritableThreadLocal<>(); + private final MetricsProvider provider; + private final AtomicReference initialNamespace = new AtomicReference<>(); + private final AtomicReference initialDefaultDimensions = new AtomicReference<>(); + private final AtomicBoolean initialRaiseOnEmptyMetrics = new AtomicBoolean(false); + + public ThreadLocalMetricsProxy(MetricsProvider provider) { + this.provider = provider; + } + + private Metrics getOrCreateThreadLocalMetrics() { + Metrics metrics = threadLocalMetrics.get(); + if (metrics == null) { + metrics = provider.getMetricsInstance(); + String namespace = initialNamespace.get(); + if (namespace != null) { + metrics.setNamespace(namespace); + } + DimensionSet dimensions = initialDefaultDimensions.get(); + if (dimensions != null) { + metrics.setDefaultDimensions(dimensions); + } + metrics.setRaiseOnEmptyMetrics(initialRaiseOnEmptyMetrics.get()); + threadLocalMetrics.set(metrics); + } + return metrics; + } + + // Configuration methods - called by MetricsFactory and MetricsBuilder + // These methods DO NOT eagerly create thread-local instances because they are typically called + // outside the Lambda handler (e.g., during class initialization) potentially on a different thread. + // We delay instance creation until the first operation that needs the metrics backend (e.g., addMetric). + // See {@link software.amazon.lambda.powertools.metrics.MetricsFactory#getMetricsInstance()} + // and {@link software.amazon.lambda.powertools.metrics.MetricsBuilder#build()} + + @Override + public void setNamespace(String namespace) { + this.initialNamespace.set(namespace); + Optional.ofNullable(threadLocalMetrics.get()).ifPresent(m -> m.setNamespace(namespace)); + } + + @Override + public void setDefaultDimensions(DimensionSet dimensionSet) { + if (dimensionSet == null) { + throw new IllegalArgumentException("DimensionSet cannot be null"); + } + this.initialDefaultDimensions.set(dimensionSet); + Optional.ofNullable(threadLocalMetrics.get()).ifPresent(m -> m.setDefaultDimensions(dimensionSet)); + } + + @Override + public void setRaiseOnEmptyMetrics(boolean raiseOnEmptyMetrics) { + this.initialRaiseOnEmptyMetrics.set(raiseOnEmptyMetrics); + Optional.ofNullable(threadLocalMetrics.get()).ifPresent(m -> m.setRaiseOnEmptyMetrics(raiseOnEmptyMetrics)); + } + + @Override + public DimensionSet getDefaultDimensions() { + Metrics metrics = threadLocalMetrics.get(); + if (metrics != null) { + return metrics.getDefaultDimensions(); + } + DimensionSet dimensions = initialDefaultDimensions.get(); + return dimensions != null ? dimensions : DimensionSet.of(new HashMap<>()); + } + + // Metrics operations - these eagerly create thread-local instances + + @Override + public void addMetric(String key, double value, MetricUnit unit, MetricResolution resolution) { + getOrCreateThreadLocalMetrics().addMetric(key, value, unit, resolution); + } + + @Override + public void addDimension(DimensionSet dimensionSet) { + getOrCreateThreadLocalMetrics().addDimension(dimensionSet); + } + + @Override + public void setTimestamp(Instant timestamp) { + getOrCreateThreadLocalMetrics().setTimestamp(timestamp); + } + + @Override + public void addMetadata(String key, Object value) { + getOrCreateThreadLocalMetrics().addMetadata(key, value); + } + + @Override + public void clearDefaultDimensions() { + getOrCreateThreadLocalMetrics().clearDefaultDimensions(); + } + + @Override + public void flush() { + // Always create instance to ensure validation and warnings are triggered. E.g. when raiseOnEmptyMetrics + // is enabled. + Metrics metrics = getOrCreateThreadLocalMetrics(); + metrics.flush(); + threadLocalMetrics.remove(); + } + + @Override + public void captureColdStartMetric(Context context, DimensionSet dimensions) { + getOrCreateThreadLocalMetrics().captureColdStartMetric(context, dimensions); + } + + @Override + public void captureColdStartMetric(DimensionSet dimensions) { + getOrCreateThreadLocalMetrics().captureColdStartMetric(dimensions); + } + + @Override + public void flushMetrics(Consumer metricsConsumer) { + getOrCreateThreadLocalMetrics().flushMetrics(metricsConsumer); + } +} diff --git a/powertools-metrics/src/test/java/software/amazon/lambda/powertools/metrics/ConfigurationPrecedenceTest.java b/powertools-metrics/src/test/java/software/amazon/lambda/powertools/metrics/ConfigurationPrecedenceTest.java index c9c772313..84164c091 100644 --- a/powertools-metrics/src/test/java/software/amazon/lambda/powertools/metrics/ConfigurationPrecedenceTest.java +++ b/powertools-metrics/src/test/java/software/amazon/lambda/powertools/metrics/ConfigurationPrecedenceTest.java @@ -68,7 +68,7 @@ void tearDown() throws Exception { System.setOut(standardOut); // Reset the singleton state between tests - java.lang.reflect.Field field = MetricsFactory.class.getDeclaredField("metrics"); + java.lang.reflect.Field field = MetricsFactory.class.getDeclaredField("metricsProxy"); field.setAccessible(true); field.set(null, null); diff --git a/powertools-metrics/src/test/java/software/amazon/lambda/powertools/metrics/MetricsBuilderTest.java b/powertools-metrics/src/test/java/software/amazon/lambda/powertools/metrics/MetricsBuilderTest.java index bd300fb6b..87e264c5e 100644 --- a/powertools-metrics/src/test/java/software/amazon/lambda/powertools/metrics/MetricsBuilderTest.java +++ b/powertools-metrics/src/test/java/software/amazon/lambda/powertools/metrics/MetricsBuilderTest.java @@ -27,10 +27,10 @@ import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; +import software.amazon.lambda.powertools.metrics.internal.ThreadLocalMetricsProxy; import software.amazon.lambda.powertools.metrics.model.DimensionSet; import software.amazon.lambda.powertools.metrics.model.MetricUnit; import software.amazon.lambda.powertools.metrics.provider.MetricsProvider; -import software.amazon.lambda.powertools.metrics.testutils.TestMetrics; import software.amazon.lambda.powertools.metrics.testutils.TestMetricsProvider; class MetricsBuilderTest { @@ -49,7 +49,7 @@ void tearDown() throws Exception { System.setOut(standardOut); // Reset the singleton state between tests - java.lang.reflect.Field field = MetricsFactory.class.getDeclaredField("metrics"); + java.lang.reflect.Field field = MetricsFactory.class.getDeclaredField("metricsProxy"); field.setAccessible(true); field.set(null, null); @@ -151,7 +151,7 @@ void shouldBuildWithMultipleDefaultDimensions() throws Exception { } @Test - void shouldBuildWithCustomMetricsProvider() { + void shouldBuildWithCustomMetricsProvider() throws Exception { // Given MetricsProvider testProvider = new TestMetricsProvider(); @@ -161,7 +161,13 @@ void shouldBuildWithCustomMetricsProvider() { .build(); // Then - assertThat(metrics).isInstanceOf(TestMetrics.class); + assertThat(metrics) + .isInstanceOf(ThreadLocalMetricsProxy.class); + + java.lang.reflect.Field providerField = metrics.getClass().getDeclaredField("provider"); + providerField.setAccessible(true); + MetricsProvider actualProvider = (MetricsProvider) providerField.get(metrics); + assertThat(actualProvider).isSameAs(testProvider); } @Test diff --git a/powertools-metrics/src/test/java/software/amazon/lambda/powertools/metrics/MetricsFactoryTest.java b/powertools-metrics/src/test/java/software/amazon/lambda/powertools/metrics/MetricsFactoryTest.java index 4fc98d2a5..105871301 100644 --- a/powertools-metrics/src/test/java/software/amazon/lambda/powertools/metrics/MetricsFactoryTest.java +++ b/powertools-metrics/src/test/java/software/amazon/lambda/powertools/metrics/MetricsFactoryTest.java @@ -20,6 +20,7 @@ import java.io.ByteArrayOutputStream; import java.io.PrintStream; import java.lang.reflect.Method; +import java.util.concurrent.CountDownLatch; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; @@ -30,9 +31,9 @@ import com.fasterxml.jackson.databind.ObjectMapper; import software.amazon.lambda.powertools.common.internal.LambdaHandlerProcessor; +import software.amazon.lambda.powertools.metrics.internal.ThreadLocalMetricsProxy; import software.amazon.lambda.powertools.metrics.model.MetricUnit; import software.amazon.lambda.powertools.metrics.provider.MetricsProvider; -import software.amazon.lambda.powertools.metrics.testutils.TestMetrics; import software.amazon.lambda.powertools.metrics.testutils.TestMetricsProvider; class MetricsFactoryTest { @@ -64,7 +65,7 @@ void tearDown() throws Exception { System.setOut(standardOut); // Reset the singleton state between tests - java.lang.reflect.Field field = MetricsFactory.class.getDeclaredField("metrics"); + java.lang.reflect.Field field = MetricsFactory.class.getDeclaredField("metricsProxy"); field.setAccessible(true); field.set(null, null); @@ -126,7 +127,7 @@ void shouldUseServiceNameFromEnvironmentVariable() throws Exception { } @Test - void shouldSetCustomMetricsProvider() { + void shouldSetCustomMetricsProvider() throws Exception { // Given MetricsProvider testProvider = new TestMetricsProvider(); @@ -135,7 +136,13 @@ void shouldSetCustomMetricsProvider() { Metrics metrics = MetricsFactory.getMetricsInstance(); // Then - assertThat(metrics).isInstanceOf(TestMetrics.class); + assertThat(metrics) + .isInstanceOf(ThreadLocalMetricsProxy.class); + + java.lang.reflect.Field providerField = metrics.getClass().getDeclaredField("provider"); + providerField.setAccessible(true); + MetricsProvider actualProvider = (MetricsProvider) providerField.get(metrics); + assertThat(actualProvider).isSameAs(testProvider); } @Test @@ -163,4 +170,75 @@ void shouldNotSetServiceDimensionWhenServiceUndefined() throws Exception { // Service dimension should not be present assertThat(rootNode.has("Service")).isFalse(); } + + @Test + void concurrentInvocations_shouldIsolateDimensions() throws Exception { + // GIVEN - Simulate real Lambda scenario: Metrics instance created outside handler + Metrics metrics = MetricsFactory.getMetricsInstance(); + + CountDownLatch latch = new CountDownLatch(2); + Exception[] exceptions = new Exception[2]; + + Thread thread1 = new Thread(() -> { + try { + latch.countDown(); + latch.await(); + + // Simulate handleRequest execution + metrics.setNamespace("TestNamespace"); + metrics.addDimension("userId", "user123"); + metrics.addMetric("ProcessedOrder", 1, MetricUnit.COUNT); + metrics.flush(); + } catch (Exception e) { + exceptions[0] = e; + } + }); + + Thread thread2 = new Thread(() -> { + try { + latch.countDown(); + latch.await(); + + // Simulate handleRequest execution + metrics.setNamespace("TestNamespace"); + metrics.addDimension("userId", "user456"); + metrics.addMetric("ProcessedOrder", 1, MetricUnit.COUNT); + metrics.flush(); + } catch (Exception e) { + exceptions[1] = e; + } + }); + + // WHEN + thread1.start(); + thread2.start(); + thread1.join(); + thread2.join(); + + // THEN + assertThat(exceptions[0]).isNull(); + assertThat(exceptions[1]).isNull(); + + String emfOutput = outputStreamCaptor.toString().trim(); + String[] jsonLines = emfOutput.split("\n"); + assertThat(jsonLines).hasSize(2); + + JsonNode output1 = objectMapper.readTree(jsonLines[0]); + JsonNode output2 = objectMapper.readTree(jsonLines[1]); + + // Check if dimensions are leaking across threads + boolean hasLeakage = false; + + if (output1.has("userId") && output2.has("userId")) { + String userId1 = output1.get("userId").asText(); + String userId2 = output2.get("userId").asText(); + // Both should have different userIds + hasLeakage = userId1.equals(userId2); + } + + // Each thread should have isolated dimensions + assertThat(hasLeakage) + .as("Dimensions should NOT leak across threads - each thread should have its own userId") + .isFalse(); + } } diff --git a/powertools-metrics/src/test/java/software/amazon/lambda/powertools/metrics/RequestHandlerTest.java b/powertools-metrics/src/test/java/software/amazon/lambda/powertools/metrics/RequestHandlerTest.java index d3ed64fe3..fcd677c02 100644 --- a/powertools-metrics/src/test/java/software/amazon/lambda/powertools/metrics/RequestHandlerTest.java +++ b/powertools-metrics/src/test/java/software/amazon/lambda/powertools/metrics/RequestHandlerTest.java @@ -56,7 +56,7 @@ void tearDown() throws Exception { System.setOut(STDOUT); // Reset the singleton state between tests - java.lang.reflect.Field field = MetricsFactory.class.getDeclaredField("metrics"); + java.lang.reflect.Field field = MetricsFactory.class.getDeclaredField("metricsProxy"); field.setAccessible(true); field.set(null, null); diff --git a/powertools-metrics/src/test/java/software/amazon/lambda/powertools/metrics/internal/EmfMetricsLoggerTest.java b/powertools-metrics/src/test/java/software/amazon/lambda/powertools/metrics/internal/EmfMetricsLoggerTest.java index bab039640..8c14eeea2 100644 --- a/powertools-metrics/src/test/java/software/amazon/lambda/powertools/metrics/internal/EmfMetricsLoggerTest.java +++ b/powertools-metrics/src/test/java/software/amazon/lambda/powertools/metrics/internal/EmfMetricsLoggerTest.java @@ -38,14 +38,15 @@ import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; +import software.amazon.cloudwatchlogs.emf.environment.EnvironmentProvider; +import software.amazon.cloudwatchlogs.emf.model.MetricsContext; import software.amazon.cloudwatchlogs.emf.model.Unit; import software.amazon.lambda.powertools.common.internal.LambdaHandlerProcessor; +import software.amazon.lambda.powertools.common.stubs.TestLambdaContext; import software.amazon.lambda.powertools.metrics.Metrics; -import software.amazon.lambda.powertools.metrics.MetricsFactory; import software.amazon.lambda.powertools.metrics.model.DimensionSet; import software.amazon.lambda.powertools.metrics.model.MetricResolution; import software.amazon.lambda.powertools.metrics.model.MetricUnit; -import software.amazon.lambda.powertools.common.stubs.TestLambdaContext; class EmfMetricsLoggerTest { @@ -66,7 +67,7 @@ void setUp() throws Exception { coldStartField.setAccessible(true); coldStartField.set(null, null); - metrics = MetricsFactory.getMetricsInstance(); + metrics = new EmfMetricsLogger(new EnvironmentProvider(), new MetricsContext()); metrics.setNamespace("TestNamespace"); System.setOut(new PrintStream(outputStreamCaptor)); } @@ -74,11 +75,6 @@ void setUp() throws Exception { @AfterEach void tearDown() throws Exception { System.setOut(standardOut); - - // Reset the singleton state between tests - java.lang.reflect.Field field = MetricsFactory.class.getDeclaredField("metrics"); - field.setAccessible(true); - field.set(null, null); } @ParameterizedTest diff --git a/powertools-metrics/src/test/java/software/amazon/lambda/powertools/metrics/internal/LambdaMetricsAspectTest.java b/powertools-metrics/src/test/java/software/amazon/lambda/powertools/metrics/internal/LambdaMetricsAspectTest.java index 031fe4553..30dc144d1 100644 --- a/powertools-metrics/src/test/java/software/amazon/lambda/powertools/metrics/internal/LambdaMetricsAspectTest.java +++ b/powertools-metrics/src/test/java/software/amazon/lambda/powertools/metrics/internal/LambdaMetricsAspectTest.java @@ -65,7 +65,7 @@ void tearDown() throws Exception { System.setOut(standardOut); // Reset the singleton state between tests - java.lang.reflect.Field field = MetricsFactory.class.getDeclaredField("metrics"); + java.lang.reflect.Field field = MetricsFactory.class.getDeclaredField("metricsProxy"); field.setAccessible(true); field.set(null, null); } diff --git a/powertools-metrics/src/test/java/software/amazon/lambda/powertools/metrics/internal/ThreadLocalMetricsProxyTest.java b/powertools-metrics/src/test/java/software/amazon/lambda/powertools/metrics/internal/ThreadLocalMetricsProxyTest.java new file mode 100644 index 000000000..ef4db69bc --- /dev/null +++ b/powertools-metrics/src/test/java/software/amazon/lambda/powertools/metrics/internal/ThreadLocalMetricsProxyTest.java @@ -0,0 +1,267 @@ +package software.amazon.lambda.powertools.metrics.internal; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import java.time.Instant; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.Mock.Strictness; +import org.mockito.junit.jupiter.MockitoExtension; + +import software.amazon.lambda.powertools.metrics.Metrics; +import software.amazon.lambda.powertools.metrics.model.DimensionSet; +import software.amazon.lambda.powertools.metrics.model.MetricResolution; +import software.amazon.lambda.powertools.metrics.model.MetricUnit; +import software.amazon.lambda.powertools.metrics.provider.MetricsProvider; + +/** + * Tests for ThreadLocalMetricsProxy focusing on lazy vs eager initialization behavior. + * + * CRITICAL: These tests ensure configuration methods (setNamespace, setDefaultDimensions, + * setRaiseOnEmptyMetrics) do NOT eagerly create instances, while metrics operations + * (addMetric, addDimension, flush) DO eagerly create instances. + */ +@ExtendWith(MockitoExtension.class) +class ThreadLocalMetricsProxyTest { + + @Mock(strictness = Strictness.LENIENT) + private MetricsProvider mockProvider; + + @Mock(strictness = Strictness.LENIENT) + private Metrics mockMetrics; + + private ThreadLocalMetricsProxy proxy; + + @BeforeEach + void setUp() { + when(mockProvider.getMetricsInstance()).thenReturn(mockMetrics); + proxy = new ThreadLocalMetricsProxy(mockProvider); + } + + // ========== LAZY INITIALIZATION TESTS (Configuration Methods) ========== + + @Test + void setNamespace_shouldNotEagerlyCreateInstance() { + // WHEN + proxy.setNamespace("TestNamespace"); + + // THEN - Provider should NOT be called (lazy initialization) + verify(mockProvider, never()).getMetricsInstance(); + } + + @Test + void setDefaultDimensions_shouldNotEagerlyCreateInstance() { + // WHEN + proxy.setDefaultDimensions(DimensionSet.of("key", "value")); + + // THEN - Provider should NOT be called (lazy initialization) + verify(mockProvider, never()).getMetricsInstance(); + } + + @Test + void setDefaultDimensions_shouldThrowExceptionWhenNull() { + // When/Then + assertThatThrownBy(() -> proxy.setDefaultDimensions(null)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("DimensionSet cannot be null"); + } + + @Test + void setRaiseOnEmptyMetrics_shouldNotEagerlyCreateInstance() { + // WHEN + proxy.setRaiseOnEmptyMetrics(true); + + // THEN - Provider should NOT be called (lazy initialization) + verify(mockProvider, never()).getMetricsInstance(); + } + + // ========== EAGER INITIALIZATION TESTS (Metrics Operations) ========== + + @Test + void addMetric_shouldEagerlyCreateInstance() { + // WHEN + proxy.addMetric("test", 1, MetricUnit.COUNT, MetricResolution.HIGH); + + // THEN - Provider SHOULD be called (eager initialization) + verify(mockProvider, times(1)).getMetricsInstance(); + verify(mockMetrics).addMetric("test", 1, MetricUnit.COUNT, MetricResolution.HIGH); + } + + @Test + void addDimension_shouldEagerlyCreateInstance() { + // WHEN + proxy.addDimension(DimensionSet.of("key", "value")); + + // THEN - Provider SHOULD be called (eager initialization) + verify(mockProvider, times(1)).getMetricsInstance(); + verify(mockMetrics).addDimension(any(DimensionSet.class)); + } + + @Test + void addMetadata_shouldEagerlyCreateInstance() { + // WHEN + proxy.addMetadata("key", "value"); + + // THEN - Provider SHOULD be called (eager initialization) + verify(mockProvider, times(1)).getMetricsInstance(); + verify(mockMetrics).addMetadata("key", "value"); + } + + @Test + void flush_shouldAlwaysCreateInstance() { + // WHEN + proxy.flush(); + + // THEN - Provider SHOULD be called even if no metrics added + verify(mockProvider, times(1)).getMetricsInstance(); + verify(mockMetrics).flush(); + } + + // ========== CONFIGURATION APPLIED ON FIRST METRICS OPERATION ========== + + @Test + void firstMetricsOperation_shouldApplyStoredConfiguration() { + // GIVEN - Set configuration without creating instance + proxy.setNamespace("TestNamespace"); + proxy.setDefaultDimensions(DimensionSet.of("Service", "TestService")); + proxy.setRaiseOnEmptyMetrics(true); + verify(mockProvider, never()).getMetricsInstance(); + + // WHEN - First metrics operation + proxy.addMetric("test", 1, MetricUnit.COUNT); + + // THEN - Instance created and configuration applied + verify(mockProvider, times(1)).getMetricsInstance(); + verify(mockMetrics).setNamespace("TestNamespace"); + verify(mockMetrics).setDefaultDimensions(any(DimensionSet.class)); + verify(mockMetrics).setRaiseOnEmptyMetrics(true); + verify(mockMetrics).addMetric("test", 1, MetricUnit.COUNT, MetricResolution.STANDARD); + } + + // ========== THREAD ISOLATION TESTS ========== + + @Test + void differentThreads_shouldInheritParentInstance() throws Exception { + // WHEN - Parent thread adds metric + proxy.addMetric("metric1", 1, MetricUnit.COUNT); + verify(mockProvider, times(1)).getMetricsInstance(); + + // WHEN - Child thread adds metric (inherits parent's instance) + Thread thread2 = new Thread(() -> { + proxy.addMetric("metric2", 2, MetricUnit.COUNT); + }); + thread2.start(); + thread2.join(); + + // THEN - Only one instance created (child inherits via InheritableThreadLocal) + verify(mockProvider, times(1)).getMetricsInstance(); + } + + @Test + void flush_shouldRemoveThreadLocalInstance() { + // GIVEN - Create instance + proxy.addMetric("test", 1, MetricUnit.COUNT); + verify(mockProvider, times(1)).getMetricsInstance(); + + // WHEN - Flush + proxy.flush(); + + // WHEN - Add another metric (should create new instance) + proxy.addMetric("test2", 2, MetricUnit.COUNT); + + // THEN - Provider called twice (instance was removed after flush) + verify(mockProvider, times(2)).getMetricsInstance(); + } + + // ========== EDGE CASES ========== + + @Test + void multipleConfigurationCalls_shouldUpdateAtomicReferences() { + // WHEN + proxy.setNamespace("Namespace1"); + proxy.setNamespace("Namespace2"); + proxy.setNamespace("Namespace3"); + + // THEN - No instance created + verify(mockProvider, never()).getMetricsInstance(); + + // WHEN - First metrics operation + proxy.addMetric("test", 1, MetricUnit.COUNT); + + // THEN - Only last namespace applied + verify(mockMetrics).setNamespace("Namespace3"); + verify(mockMetrics, never()).setNamespace("Namespace1"); + verify(mockMetrics, never()).setNamespace("Namespace2"); + } + + @Test + void configurationAfterInstanceCreation_shouldApplyImmediately() { + // GIVEN - Instance already created + proxy.addMetric("test", 1, MetricUnit.COUNT); + + // WHEN - Set configuration + proxy.setNamespace("NewNamespace"); + + // THEN - Applied immediately to existing instance + verify(mockMetrics).setNamespace("NewNamespace"); + } + + @Test + void setTimestamp_shouldEagerlyCreateInstance() { + // When + proxy.setTimestamp(Instant.now()); + + // Then + verify(mockProvider, times(1)).getMetricsInstance(); + verify(mockMetrics).setTimestamp(any()); + } + + @Test + void getDefaultDimensions_shouldNotEagerlyCreateInstance() { + // WHEN + DimensionSet result = proxy.getDefaultDimensions(); + + // THEN - Provider should NOT be called (returns stored config or empty) + verify(mockProvider, never()).getMetricsInstance(); + assertThat(result).isNotNull(); + } + + @Test + void captureColdStartMetric_shouldEagerlyCreateInstance() { + // WHEN + proxy.captureColdStartMetric(DimensionSet.of("key", "value")); + + // THEN - Provider SHOULD be called (eager initialization) + verify(mockProvider, times(1)).getMetricsInstance(); + verify(mockMetrics).captureColdStartMetric(any(DimensionSet.class)); + } + + @Test + void flushMetrics_shouldEagerlyCreateInstance() { + // WHEN + proxy.flushMetrics(m -> m.addMetric("test", 1, MetricUnit.COUNT)); + + // THEN - Provider SHOULD be called (eager initialization) + verify(mockProvider, times(1)).getMetricsInstance(); + verify(mockMetrics).flushMetrics(any()); + } + + @Test + void clearDefaultDimensions_shouldEagerlyCreateInstance() { + // WHEN + proxy.clearDefaultDimensions(); + + // THEN - Provider SHOULD be called (eager initialization) + verify(mockProvider, times(1)).getMetricsInstance(); + verify(mockMetrics).clearDefaultDimensions(); + } +} From 8857936b39ba0cd0f5d81459add3890b9b821c89 Mon Sep 17 00:00:00 2001 From: Philipp Page Date: Fri, 14 Nov 2025 16:12:50 +0100 Subject: [PATCH 2/4] Refactor from inheritable thread local to tracking by trace id. Similar to log buffer. --- .../powertools/metrics/MetricsFactory.java | 6 +- ...xy.java => RequestScopedMetricsProxy.java} | 58 ++++++----- .../metrics/MetricsBuilderTest.java | 4 +- .../metrics/MetricsFactoryTest.java | 95 ++++++++----------- ...ava => RequestScopedMetricsProxyTest.java} | 23 +++-- 5 files changed, 90 insertions(+), 96 deletions(-) rename powertools-metrics/src/main/java/software/amazon/lambda/powertools/metrics/internal/{ThreadLocalMetricsProxy.java => RequestScopedMetricsProxy.java} (69%) rename powertools-metrics/src/test/java/software/amazon/lambda/powertools/metrics/internal/{ThreadLocalMetricsProxyTest.java => RequestScopedMetricsProxyTest.java} (92%) diff --git a/powertools-metrics/src/main/java/software/amazon/lambda/powertools/metrics/MetricsFactory.java b/powertools-metrics/src/main/java/software/amazon/lambda/powertools/metrics/MetricsFactory.java index aab10e770..0644329c9 100644 --- a/powertools-metrics/src/main/java/software/amazon/lambda/powertools/metrics/MetricsFactory.java +++ b/powertools-metrics/src/main/java/software/amazon/lambda/powertools/metrics/MetricsFactory.java @@ -20,7 +20,7 @@ import software.amazon.lambda.powertools.common.internal.ClassPreLoader; import software.amazon.lambda.powertools.common.internal.LambdaConstants; import software.amazon.lambda.powertools.common.internal.LambdaHandlerProcessor; -import software.amazon.lambda.powertools.metrics.internal.ThreadLocalMetricsProxy; +import software.amazon.lambda.powertools.metrics.internal.RequestScopedMetricsProxy; import software.amazon.lambda.powertools.metrics.model.DimensionSet; import software.amazon.lambda.powertools.metrics.provider.EmfMetricsProvider; import software.amazon.lambda.powertools.metrics.provider.MetricsProvider; @@ -30,7 +30,7 @@ */ public final class MetricsFactory implements Resource { private static MetricsProvider provider = new EmfMetricsProvider(); - private static ThreadLocalMetricsProxy metricsProxy; + private static RequestScopedMetricsProxy metricsProxy; // Dummy instance to register MetricsFactory with CRaC private static final MetricsFactory INSTANCE = new MetricsFactory(); @@ -47,7 +47,7 @@ public final class MetricsFactory implements Resource { */ public static synchronized Metrics getMetricsInstance() { if (metricsProxy == null) { - metricsProxy = new ThreadLocalMetricsProxy(provider); + metricsProxy = new RequestScopedMetricsProxy(provider); // Apply default configuration from environment variables String envNamespace = System.getenv("POWERTOOLS_METRICS_NAMESPACE"); diff --git a/powertools-metrics/src/main/java/software/amazon/lambda/powertools/metrics/internal/ThreadLocalMetricsProxy.java b/powertools-metrics/src/main/java/software/amazon/lambda/powertools/metrics/internal/RequestScopedMetricsProxy.java similarity index 69% rename from powertools-metrics/src/main/java/software/amazon/lambda/powertools/metrics/internal/ThreadLocalMetricsProxy.java rename to powertools-metrics/src/main/java/software/amazon/lambda/powertools/metrics/internal/RequestScopedMetricsProxy.java index fe3e92dcb..762c087ac 100644 --- a/powertools-metrics/src/main/java/software/amazon/lambda/powertools/metrics/internal/ThreadLocalMetricsProxy.java +++ b/powertools-metrics/src/main/java/software/amazon/lambda/powertools/metrics/internal/RequestScopedMetricsProxy.java @@ -17,33 +17,40 @@ import java.time.Instant; import java.util.HashMap; import java.util.Optional; +import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicReference; import java.util.function.Consumer; import com.amazonaws.services.lambda.runtime.Context; +import software.amazon.lambda.powertools.common.internal.LambdaHandlerProcessor; import software.amazon.lambda.powertools.metrics.Metrics; import software.amazon.lambda.powertools.metrics.model.DimensionSet; import software.amazon.lambda.powertools.metrics.model.MetricResolution; import software.amazon.lambda.powertools.metrics.model.MetricUnit; import software.amazon.lambda.powertools.metrics.provider.MetricsProvider; -public class ThreadLocalMetricsProxy implements Metrics { - private final InheritableThreadLocal threadLocalMetrics = new InheritableThreadLocal<>(); +public class RequestScopedMetricsProxy implements Metrics { + private static final String DEFAULT_TRACE_ID = "DEFAULT"; + private final ConcurrentHashMap metricsMap = new ConcurrentHashMap<>(); private final MetricsProvider provider; private final AtomicReference initialNamespace = new AtomicReference<>(); private final AtomicReference initialDefaultDimensions = new AtomicReference<>(); private final AtomicBoolean initialRaiseOnEmptyMetrics = new AtomicBoolean(false); - public ThreadLocalMetricsProxy(MetricsProvider provider) { + public RequestScopedMetricsProxy(MetricsProvider provider) { this.provider = provider; } - private Metrics getOrCreateThreadLocalMetrics() { - Metrics metrics = threadLocalMetrics.get(); - if (metrics == null) { - metrics = provider.getMetricsInstance(); + private String getTraceId() { + return LambdaHandlerProcessor.getXrayTraceId().orElse(DEFAULT_TRACE_ID); + } + + private Metrics getOrCreateMetrics() { + String traceId = getTraceId(); + return metricsMap.computeIfAbsent(traceId, key -> { + Metrics metrics = provider.getMetricsInstance(); String namespace = initialNamespace.get(); if (namespace != null) { metrics.setNamespace(namespace); @@ -53,13 +60,12 @@ private Metrics getOrCreateThreadLocalMetrics() { metrics.setDefaultDimensions(dimensions); } metrics.setRaiseOnEmptyMetrics(initialRaiseOnEmptyMetrics.get()); - threadLocalMetrics.set(metrics); - } - return metrics; + return metrics; + }); } // Configuration methods - called by MetricsFactory and MetricsBuilder - // These methods DO NOT eagerly create thread-local instances because they are typically called + // These methods DO NOT eagerly create instances because they are typically called // outside the Lambda handler (e.g., during class initialization) potentially on a different thread. // We delay instance creation until the first operation that needs the metrics backend (e.g., addMetric). // See {@link software.amazon.lambda.powertools.metrics.MetricsFactory#getMetricsInstance()} @@ -68,7 +74,7 @@ private Metrics getOrCreateThreadLocalMetrics() { @Override public void setNamespace(String namespace) { this.initialNamespace.set(namespace); - Optional.ofNullable(threadLocalMetrics.get()).ifPresent(m -> m.setNamespace(namespace)); + Optional.ofNullable(metricsMap.get(getTraceId())).ifPresent(m -> m.setNamespace(namespace)); } @Override @@ -77,18 +83,18 @@ public void setDefaultDimensions(DimensionSet dimensionSet) { throw new IllegalArgumentException("DimensionSet cannot be null"); } this.initialDefaultDimensions.set(dimensionSet); - Optional.ofNullable(threadLocalMetrics.get()).ifPresent(m -> m.setDefaultDimensions(dimensionSet)); + Optional.ofNullable(metricsMap.get(getTraceId())).ifPresent(m -> m.setDefaultDimensions(dimensionSet)); } @Override public void setRaiseOnEmptyMetrics(boolean raiseOnEmptyMetrics) { this.initialRaiseOnEmptyMetrics.set(raiseOnEmptyMetrics); - Optional.ofNullable(threadLocalMetrics.get()).ifPresent(m -> m.setRaiseOnEmptyMetrics(raiseOnEmptyMetrics)); + Optional.ofNullable(metricsMap.get(getTraceId())).ifPresent(m -> m.setRaiseOnEmptyMetrics(raiseOnEmptyMetrics)); } @Override public DimensionSet getDefaultDimensions() { - Metrics metrics = threadLocalMetrics.get(); + Metrics metrics = metricsMap.get(getTraceId()); if (metrics != null) { return metrics.getDefaultDimensions(); } @@ -96,54 +102,54 @@ public DimensionSet getDefaultDimensions() { return dimensions != null ? dimensions : DimensionSet.of(new HashMap<>()); } - // Metrics operations - these eagerly create thread-local instances + // Metrics operations - these eagerly create instances @Override public void addMetric(String key, double value, MetricUnit unit, MetricResolution resolution) { - getOrCreateThreadLocalMetrics().addMetric(key, value, unit, resolution); + getOrCreateMetrics().addMetric(key, value, unit, resolution); } @Override public void addDimension(DimensionSet dimensionSet) { - getOrCreateThreadLocalMetrics().addDimension(dimensionSet); + getOrCreateMetrics().addDimension(dimensionSet); } @Override public void setTimestamp(Instant timestamp) { - getOrCreateThreadLocalMetrics().setTimestamp(timestamp); + getOrCreateMetrics().setTimestamp(timestamp); } @Override public void addMetadata(String key, Object value) { - getOrCreateThreadLocalMetrics().addMetadata(key, value); + getOrCreateMetrics().addMetadata(key, value); } @Override public void clearDefaultDimensions() { - getOrCreateThreadLocalMetrics().clearDefaultDimensions(); + getOrCreateMetrics().clearDefaultDimensions(); } @Override public void flush() { // Always create instance to ensure validation and warnings are triggered. E.g. when raiseOnEmptyMetrics // is enabled. - Metrics metrics = getOrCreateThreadLocalMetrics(); + Metrics metrics = getOrCreateMetrics(); metrics.flush(); - threadLocalMetrics.remove(); + metricsMap.remove(getTraceId()); } @Override public void captureColdStartMetric(Context context, DimensionSet dimensions) { - getOrCreateThreadLocalMetrics().captureColdStartMetric(context, dimensions); + getOrCreateMetrics().captureColdStartMetric(context, dimensions); } @Override public void captureColdStartMetric(DimensionSet dimensions) { - getOrCreateThreadLocalMetrics().captureColdStartMetric(dimensions); + getOrCreateMetrics().captureColdStartMetric(dimensions); } @Override public void flushMetrics(Consumer metricsConsumer) { - getOrCreateThreadLocalMetrics().flushMetrics(metricsConsumer); + getOrCreateMetrics().flushMetrics(metricsConsumer); } } diff --git a/powertools-metrics/src/test/java/software/amazon/lambda/powertools/metrics/MetricsBuilderTest.java b/powertools-metrics/src/test/java/software/amazon/lambda/powertools/metrics/MetricsBuilderTest.java index 87e264c5e..42f453be5 100644 --- a/powertools-metrics/src/test/java/software/amazon/lambda/powertools/metrics/MetricsBuilderTest.java +++ b/powertools-metrics/src/test/java/software/amazon/lambda/powertools/metrics/MetricsBuilderTest.java @@ -27,7 +27,7 @@ import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; -import software.amazon.lambda.powertools.metrics.internal.ThreadLocalMetricsProxy; +import software.amazon.lambda.powertools.metrics.internal.RequestScopedMetricsProxy; import software.amazon.lambda.powertools.metrics.model.DimensionSet; import software.amazon.lambda.powertools.metrics.model.MetricUnit; import software.amazon.lambda.powertools.metrics.provider.MetricsProvider; @@ -162,7 +162,7 @@ void shouldBuildWithCustomMetricsProvider() throws Exception { // Then assertThat(metrics) - .isInstanceOf(ThreadLocalMetricsProxy.class); + .isInstanceOf(RequestScopedMetricsProxy.class); java.lang.reflect.Field providerField = metrics.getClass().getDeclaredField("provider"); providerField.setAccessible(true); diff --git a/powertools-metrics/src/test/java/software/amazon/lambda/powertools/metrics/MetricsFactoryTest.java b/powertools-metrics/src/test/java/software/amazon/lambda/powertools/metrics/MetricsFactoryTest.java index 105871301..2b8febc67 100644 --- a/powertools-metrics/src/test/java/software/amazon/lambda/powertools/metrics/MetricsFactoryTest.java +++ b/powertools-metrics/src/test/java/software/amazon/lambda/powertools/metrics/MetricsFactoryTest.java @@ -20,7 +20,6 @@ import java.io.ByteArrayOutputStream; import java.io.PrintStream; import java.lang.reflect.Method; -import java.util.concurrent.CountDownLatch; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; @@ -31,7 +30,7 @@ import com.fasterxml.jackson.databind.ObjectMapper; import software.amazon.lambda.powertools.common.internal.LambdaHandlerProcessor; -import software.amazon.lambda.powertools.metrics.internal.ThreadLocalMetricsProxy; +import software.amazon.lambda.powertools.metrics.internal.RequestScopedMetricsProxy; import software.amazon.lambda.powertools.metrics.model.MetricUnit; import software.amazon.lambda.powertools.metrics.provider.MetricsProvider; import software.amazon.lambda.powertools.metrics.testutils.TestMetricsProvider; @@ -63,6 +62,7 @@ void setUp() throws Exception { @AfterEach void tearDown() throws Exception { System.setOut(standardOut); + System.clearProperty("com.amazonaws.xray.traceHeader"); // Reset the singleton state between tests java.lang.reflect.Field field = MetricsFactory.class.getDeclaredField("metricsProxy"); @@ -137,7 +137,7 @@ void shouldSetCustomMetricsProvider() throws Exception { // Then assertThat(metrics) - .isInstanceOf(ThreadLocalMetricsProxy.class); + .isInstanceOf(RequestScopedMetricsProxy.class); java.lang.reflect.Field providerField = metrics.getClass().getDeclaredField("provider"); providerField.setAccessible(true); @@ -172,53 +172,25 @@ void shouldNotSetServiceDimensionWhenServiceUndefined() throws Exception { } @Test - void concurrentInvocations_shouldIsolateDimensions() throws Exception { - // GIVEN - Simulate real Lambda scenario: Metrics instance created outside handler + void shouldIsolateMetricsByTraceId() throws Exception { + // GIVEN Metrics metrics = MetricsFactory.getMetricsInstance(); - CountDownLatch latch = new CountDownLatch(2); - Exception[] exceptions = new Exception[2]; - - Thread thread1 = new Thread(() -> { - try { - latch.countDown(); - latch.await(); - - // Simulate handleRequest execution - metrics.setNamespace("TestNamespace"); - metrics.addDimension("userId", "user123"); - metrics.addMetric("ProcessedOrder", 1, MetricUnit.COUNT); - metrics.flush(); - } catch (Exception e) { - exceptions[0] = e; - } - }); - - Thread thread2 = new Thread(() -> { - try { - latch.countDown(); - latch.await(); - - // Simulate handleRequest execution - metrics.setNamespace("TestNamespace"); - metrics.addDimension("userId", "user456"); - metrics.addMetric("ProcessedOrder", 1, MetricUnit.COUNT); - metrics.flush(); - } catch (Exception e) { - exceptions[1] = e; - } - }); - - // WHEN - thread1.start(); - thread2.start(); - thread1.join(); - thread2.join(); + // WHEN - Simulate Lambda invocation 1 with trace ID 1 + System.setProperty("com.amazonaws.xray.traceHeader", "Root=1-trace-id-1"); + metrics.setNamespace("TestNamespace"); + metrics.addDimension("userId", "user123"); + metrics.addMetric("ProcessedOrder", 1, MetricUnit.COUNT); + metrics.flush(); - // THEN - assertThat(exceptions[0]).isNull(); - assertThat(exceptions[1]).isNull(); + // WHEN - Simulate Lambda invocation 2 with trace ID 2 + System.setProperty("com.amazonaws.xray.traceHeader", "Root=1-trace-id-2"); + metrics.setNamespace("TestNamespace"); + metrics.addDimension("userId", "user456"); + metrics.addMetric("ProcessedOrder", 1, MetricUnit.COUNT); + metrics.flush(); + // THEN - Verify each invocation has isolated metrics String emfOutput = outputStreamCaptor.toString().trim(); String[] jsonLines = emfOutput.split("\n"); assertThat(jsonLines).hasSize(2); @@ -226,19 +198,26 @@ void concurrentInvocations_shouldIsolateDimensions() throws Exception { JsonNode output1 = objectMapper.readTree(jsonLines[0]); JsonNode output2 = objectMapper.readTree(jsonLines[1]); - // Check if dimensions are leaking across threads - boolean hasLeakage = false; + assertThat(output1.get("userId").asText()).isEqualTo("user123"); + assertThat(output2.get("userId").asText()).isEqualTo("user456"); + } + + @Test + void shouldUseDefaultKeyWhenNoTraceId() throws Exception { + // GIVEN - No trace ID set + System.clearProperty("com.amazonaws.xray.traceHeader"); + + // WHEN + Metrics metrics = MetricsFactory.getMetricsInstance(); + metrics.setNamespace("TestNamespace"); + metrics.addMetric("TestMetric", 1, MetricUnit.COUNT); + metrics.flush(); - if (output1.has("userId") && output2.has("userId")) { - String userId1 = output1.get("userId").asText(); - String userId2 = output2.get("userId").asText(); - // Both should have different userIds - hasLeakage = userId1.equals(userId2); - } + // THEN - Should work without X-Ray trace ID + String emfOutput = outputStreamCaptor.toString().trim(); + assertThat(emfOutput).isNotEmpty(); - // Each thread should have isolated dimensions - assertThat(hasLeakage) - .as("Dimensions should NOT leak across threads - each thread should have its own userId") - .isFalse(); + JsonNode rootNode = objectMapper.readTree(emfOutput); + assertThat(rootNode.get("TestMetric").asDouble()).isEqualTo(1.0); } } diff --git a/powertools-metrics/src/test/java/software/amazon/lambda/powertools/metrics/internal/ThreadLocalMetricsProxyTest.java b/powertools-metrics/src/test/java/software/amazon/lambda/powertools/metrics/internal/RequestScopedMetricsProxyTest.java similarity index 92% rename from powertools-metrics/src/test/java/software/amazon/lambda/powertools/metrics/internal/ThreadLocalMetricsProxyTest.java rename to powertools-metrics/src/test/java/software/amazon/lambda/powertools/metrics/internal/RequestScopedMetricsProxyTest.java index ef4db69bc..0202e1eda 100644 --- a/powertools-metrics/src/test/java/software/amazon/lambda/powertools/metrics/internal/ThreadLocalMetricsProxyTest.java +++ b/powertools-metrics/src/test/java/software/amazon/lambda/powertools/metrics/internal/RequestScopedMetricsProxyTest.java @@ -10,6 +10,7 @@ import java.time.Instant; +import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -24,14 +25,14 @@ import software.amazon.lambda.powertools.metrics.provider.MetricsProvider; /** - * Tests for ThreadLocalMetricsProxy focusing on lazy vs eager initialization behavior. + * Tests for RequestScopedMetricsProxy focusing on lazy vs eager initialization behavior. * * CRITICAL: These tests ensure configuration methods (setNamespace, setDefaultDimensions, * setRaiseOnEmptyMetrics) do NOT eagerly create instances, while metrics operations * (addMetric, addDimension, flush) DO eagerly create instances. */ @ExtendWith(MockitoExtension.class) -class ThreadLocalMetricsProxyTest { +class RequestScopedMetricsProxyTest { @Mock(strictness = Strictness.LENIENT) private MetricsProvider mockProvider; @@ -39,12 +40,17 @@ class ThreadLocalMetricsProxyTest { @Mock(strictness = Strictness.LENIENT) private Metrics mockMetrics; - private ThreadLocalMetricsProxy proxy; + private RequestScopedMetricsProxy proxy; @BeforeEach void setUp() { when(mockProvider.getMetricsInstance()).thenReturn(mockMetrics); - proxy = new ThreadLocalMetricsProxy(mockProvider); + proxy = new RequestScopedMetricsProxy(mockProvider); + } + + @AfterEach + void tearDown() { + System.clearProperty("com.amazonaws.xray.traceHeader"); } // ========== LAZY INITIALIZATION TESTS (Configuration Methods) ========== @@ -150,19 +156,22 @@ void firstMetricsOperation_shouldApplyStoredConfiguration() { // ========== THREAD ISOLATION TESTS ========== @Test - void differentThreads_shouldInheritParentInstance() throws Exception { + void shouldShareInstanceAcrossThreadsWithSameTraceId() throws Exception { + // GIVEN - Set trace ID + System.setProperty("com.amazonaws.xray.traceHeader", "Root=1-test-trace-id"); + // WHEN - Parent thread adds metric proxy.addMetric("metric1", 1, MetricUnit.COUNT); verify(mockProvider, times(1)).getMetricsInstance(); - // WHEN - Child thread adds metric (inherits parent's instance) + // WHEN - Child thread adds metric (same trace ID) Thread thread2 = new Thread(() -> { proxy.addMetric("metric2", 2, MetricUnit.COUNT); }); thread2.start(); thread2.join(); - // THEN - Only one instance created (child inherits via InheritableThreadLocal) + // THEN - Only one instance created (same trace ID = shared instance) verify(mockProvider, times(1)).getMetricsInstance(); } From 784af1e3eb72062a9c597db8829e36b1b4547f87 Mon Sep 17 00:00:00 2001 From: Philipp Page Date: Fri, 14 Nov 2025 16:18:48 +0100 Subject: [PATCH 3/4] Do not hard code JVM system property for trace id. --- .../lambda/powertools/metrics/MetricsFactoryTest.java | 9 +++++---- .../metrics/internal/EmfMetricsLoggerTest.java | 2 +- .../metrics/internal/RequestScopedMetricsProxyTest.java | 5 +++-- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/powertools-metrics/src/test/java/software/amazon/lambda/powertools/metrics/MetricsFactoryTest.java b/powertools-metrics/src/test/java/software/amazon/lambda/powertools/metrics/MetricsFactoryTest.java index 2b8febc67..4b08d8151 100644 --- a/powertools-metrics/src/test/java/software/amazon/lambda/powertools/metrics/MetricsFactoryTest.java +++ b/powertools-metrics/src/test/java/software/amazon/lambda/powertools/metrics/MetricsFactoryTest.java @@ -29,6 +29,7 @@ import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; +import software.amazon.lambda.powertools.common.internal.LambdaConstants; import software.amazon.lambda.powertools.common.internal.LambdaHandlerProcessor; import software.amazon.lambda.powertools.metrics.internal.RequestScopedMetricsProxy; import software.amazon.lambda.powertools.metrics.model.MetricUnit; @@ -62,7 +63,7 @@ void setUp() throws Exception { @AfterEach void tearDown() throws Exception { System.setOut(standardOut); - System.clearProperty("com.amazonaws.xray.traceHeader"); + System.clearProperty(LambdaConstants.XRAY_TRACE_HEADER); // Reset the singleton state between tests java.lang.reflect.Field field = MetricsFactory.class.getDeclaredField("metricsProxy"); @@ -177,14 +178,14 @@ void shouldIsolateMetricsByTraceId() throws Exception { Metrics metrics = MetricsFactory.getMetricsInstance(); // WHEN - Simulate Lambda invocation 1 with trace ID 1 - System.setProperty("com.amazonaws.xray.traceHeader", "Root=1-trace-id-1"); + System.setProperty(LambdaConstants.XRAY_TRACE_HEADER, "Root=1-trace-id-1"); metrics.setNamespace("TestNamespace"); metrics.addDimension("userId", "user123"); metrics.addMetric("ProcessedOrder", 1, MetricUnit.COUNT); metrics.flush(); // WHEN - Simulate Lambda invocation 2 with trace ID 2 - System.setProperty("com.amazonaws.xray.traceHeader", "Root=1-trace-id-2"); + System.setProperty(LambdaConstants.XRAY_TRACE_HEADER, "Root=1-trace-id-2"); metrics.setNamespace("TestNamespace"); metrics.addDimension("userId", "user456"); metrics.addMetric("ProcessedOrder", 1, MetricUnit.COUNT); @@ -205,7 +206,7 @@ void shouldIsolateMetricsByTraceId() throws Exception { @Test void shouldUseDefaultKeyWhenNoTraceId() throws Exception { // GIVEN - No trace ID set - System.clearProperty("com.amazonaws.xray.traceHeader"); + System.clearProperty(LambdaConstants.XRAY_TRACE_HEADER); // WHEN Metrics metrics = MetricsFactory.getMetricsInstance(); diff --git a/powertools-metrics/src/test/java/software/amazon/lambda/powertools/metrics/internal/EmfMetricsLoggerTest.java b/powertools-metrics/src/test/java/software/amazon/lambda/powertools/metrics/internal/EmfMetricsLoggerTest.java index 8c14eeea2..c2238711a 100644 --- a/powertools-metrics/src/test/java/software/amazon/lambda/powertools/metrics/internal/EmfMetricsLoggerTest.java +++ b/powertools-metrics/src/test/java/software/amazon/lambda/powertools/metrics/internal/EmfMetricsLoggerTest.java @@ -73,7 +73,7 @@ void setUp() throws Exception { } @AfterEach - void tearDown() throws Exception { + void tearDown() { System.setOut(standardOut); } diff --git a/powertools-metrics/src/test/java/software/amazon/lambda/powertools/metrics/internal/RequestScopedMetricsProxyTest.java b/powertools-metrics/src/test/java/software/amazon/lambda/powertools/metrics/internal/RequestScopedMetricsProxyTest.java index 0202e1eda..848222bae 100644 --- a/powertools-metrics/src/test/java/software/amazon/lambda/powertools/metrics/internal/RequestScopedMetricsProxyTest.java +++ b/powertools-metrics/src/test/java/software/amazon/lambda/powertools/metrics/internal/RequestScopedMetricsProxyTest.java @@ -18,6 +18,7 @@ import org.mockito.Mock.Strictness; import org.mockito.junit.jupiter.MockitoExtension; +import software.amazon.lambda.powertools.common.internal.LambdaConstants; import software.amazon.lambda.powertools.metrics.Metrics; import software.amazon.lambda.powertools.metrics.model.DimensionSet; import software.amazon.lambda.powertools.metrics.model.MetricResolution; @@ -50,7 +51,7 @@ void setUp() { @AfterEach void tearDown() { - System.clearProperty("com.amazonaws.xray.traceHeader"); + System.clearProperty(LambdaConstants.XRAY_TRACE_HEADER); } // ========== LAZY INITIALIZATION TESTS (Configuration Methods) ========== @@ -158,7 +159,7 @@ void firstMetricsOperation_shouldApplyStoredConfiguration() { @Test void shouldShareInstanceAcrossThreadsWithSameTraceId() throws Exception { // GIVEN - Set trace ID - System.setProperty("com.amazonaws.xray.traceHeader", "Root=1-test-trace-id"); + System.setProperty(LambdaConstants.XRAY_TRACE_HEADER, "Root=1-test-trace-id"); // WHEN - Parent thread adds metric proxy.addMetric("metric1", 1, MetricUnit.COUNT); From 308faa139aaa35a5864e93f38fe138a242a2911c Mon Sep 17 00:00:00 2001 From: Philipp Page Date: Fri, 14 Nov 2025 16:29:46 +0100 Subject: [PATCH 4/4] Fix PMD findings. --- .../metrics/internal/RequestScopedMetricsProxy.java | 3 ++- .../metrics/ConfigurationPrecedenceTest.java | 11 ++++++----- .../lambda/powertools/metrics/MetricsBuilderTest.java | 4 ++-- .../lambda/powertools/metrics/MetricsFactoryTest.java | 4 ++-- .../metrics/internal/LambdaMetricsAspectTest.java | 6 +++--- 5 files changed, 15 insertions(+), 13 deletions(-) diff --git a/powertools-metrics/src/main/java/software/amazon/lambda/powertools/metrics/internal/RequestScopedMetricsProxy.java b/powertools-metrics/src/main/java/software/amazon/lambda/powertools/metrics/internal/RequestScopedMetricsProxy.java index 762c087ac..61c83be17 100644 --- a/powertools-metrics/src/main/java/software/amazon/lambda/powertools/metrics/internal/RequestScopedMetricsProxy.java +++ b/powertools-metrics/src/main/java/software/amazon/lambda/powertools/metrics/internal/RequestScopedMetricsProxy.java @@ -18,6 +18,7 @@ import java.util.HashMap; import java.util.Optional; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentMap; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicReference; import java.util.function.Consumer; @@ -33,7 +34,7 @@ public class RequestScopedMetricsProxy implements Metrics { private static final String DEFAULT_TRACE_ID = "DEFAULT"; - private final ConcurrentHashMap metricsMap = new ConcurrentHashMap<>(); + private final ConcurrentMap metricsMap = new ConcurrentHashMap<>(); private final MetricsProvider provider; private final AtomicReference initialNamespace = new AtomicReference<>(); private final AtomicReference initialDefaultDimensions = new AtomicReference<>(); diff --git a/powertools-metrics/src/test/java/software/amazon/lambda/powertools/metrics/ConfigurationPrecedenceTest.java b/powertools-metrics/src/test/java/software/amazon/lambda/powertools/metrics/ConfigurationPrecedenceTest.java index 84164c091..492ecfc1e 100644 --- a/powertools-metrics/src/test/java/software/amazon/lambda/powertools/metrics/ConfigurationPrecedenceTest.java +++ b/powertools-metrics/src/test/java/software/amazon/lambda/powertools/metrics/ConfigurationPrecedenceTest.java @@ -33,8 +33,8 @@ import com.fasterxml.jackson.databind.ObjectMapper; import software.amazon.lambda.powertools.common.internal.LambdaHandlerProcessor; -import software.amazon.lambda.powertools.metrics.model.MetricUnit; import software.amazon.lambda.powertools.common.stubs.TestLambdaContext; +import software.amazon.lambda.powertools.metrics.model.MetricUnit; /** * Tests to verify the hierarchy of precedence for configuration: @@ -44,7 +44,7 @@ */ class ConfigurationPrecedenceTest { - private final PrintStream standardOut = System.out; + private static final PrintStream STANDARD_OUT = System.out; private final ByteArrayOutputStream outputStreamCaptor = new ByteArrayOutputStream(); private final ObjectMapper objectMapper = new ObjectMapper(); @@ -65,7 +65,7 @@ void setUp() throws Exception { @AfterEach void tearDown() throws Exception { - System.setOut(standardOut); + System.setOut(STANDARD_OUT); // Reset the singleton state between tests java.lang.reflect.Field field = MetricsFactory.class.getDeclaredField("metricsProxy"); @@ -183,7 +183,7 @@ void shouldUseDefaultsWhenNoConfiguration() throws Exception { assertThat(rootNode.has("Service")).isFalse(); } - private static class HandlerWithMetricsAnnotation implements RequestHandler, String> { + private static final class HandlerWithMetricsAnnotation implements RequestHandler, String> { @Override @FlushMetrics(namespace = "AnnotationNamespace", service = "AnnotationService") public String handleRequest(Map input, Context context) { @@ -193,7 +193,8 @@ public String handleRequest(Map input, Context context) { } } - private static class HandlerWithDefaultMetricsAnnotation implements RequestHandler, String> { + private static final class HandlerWithDefaultMetricsAnnotation + implements RequestHandler, String> { @Override @FlushMetrics public String handleRequest(Map input, Context context) { diff --git a/powertools-metrics/src/test/java/software/amazon/lambda/powertools/metrics/MetricsBuilderTest.java b/powertools-metrics/src/test/java/software/amazon/lambda/powertools/metrics/MetricsBuilderTest.java index 42f453be5..12ac46e43 100644 --- a/powertools-metrics/src/test/java/software/amazon/lambda/powertools/metrics/MetricsBuilderTest.java +++ b/powertools-metrics/src/test/java/software/amazon/lambda/powertools/metrics/MetricsBuilderTest.java @@ -35,7 +35,7 @@ class MetricsBuilderTest { - private final PrintStream standardOut = System.out; + private static final PrintStream STANDARD_OUT = System.out; private final ByteArrayOutputStream outputStreamCaptor = new ByteArrayOutputStream(); private final ObjectMapper objectMapper = new ObjectMapper(); @@ -46,7 +46,7 @@ void setUp() { @AfterEach void tearDown() throws Exception { - System.setOut(standardOut); + System.setOut(STANDARD_OUT); // Reset the singleton state between tests java.lang.reflect.Field field = MetricsFactory.class.getDeclaredField("metricsProxy"); diff --git a/powertools-metrics/src/test/java/software/amazon/lambda/powertools/metrics/MetricsFactoryTest.java b/powertools-metrics/src/test/java/software/amazon/lambda/powertools/metrics/MetricsFactoryTest.java index 4b08d8151..c9b183a1a 100644 --- a/powertools-metrics/src/test/java/software/amazon/lambda/powertools/metrics/MetricsFactoryTest.java +++ b/powertools-metrics/src/test/java/software/amazon/lambda/powertools/metrics/MetricsFactoryTest.java @@ -41,7 +41,7 @@ class MetricsFactoryTest { private static final String TEST_NAMESPACE = "TestNamespace"; private static final String TEST_SERVICE = "TestService"; - private final PrintStream standardOut = System.out; + private static final PrintStream STANDARD_OUT = System.out; private final ByteArrayOutputStream outputStreamCaptor = new ByteArrayOutputStream(); private final ObjectMapper objectMapper = new ObjectMapper(); @@ -62,7 +62,7 @@ void setUp() throws Exception { @AfterEach void tearDown() throws Exception { - System.setOut(standardOut); + System.setOut(STANDARD_OUT); System.clearProperty(LambdaConstants.XRAY_TRACE_HEADER); // Reset the singleton state between tests diff --git a/powertools-metrics/src/test/java/software/amazon/lambda/powertools/metrics/internal/LambdaMetricsAspectTest.java b/powertools-metrics/src/test/java/software/amazon/lambda/powertools/metrics/internal/LambdaMetricsAspectTest.java index 30dc144d1..119e094a9 100644 --- a/powertools-metrics/src/test/java/software/amazon/lambda/powertools/metrics/internal/LambdaMetricsAspectTest.java +++ b/powertools-metrics/src/test/java/software/amazon/lambda/powertools/metrics/internal/LambdaMetricsAspectTest.java @@ -41,7 +41,7 @@ class LambdaMetricsAspectTest { - private final PrintStream standardOut = System.out; + private static final PrintStream STANDARD_OUT = System.out; private final ByteArrayOutputStream outputStreamCaptor = new ByteArrayOutputStream(); private final ObjectMapper objectMapper = new ObjectMapper(); @@ -62,7 +62,7 @@ void setUp() throws Exception { @AfterEach void tearDown() throws Exception { - System.setOut(standardOut); + System.setOut(STANDARD_OUT); // Reset the singleton state between tests java.lang.reflect.Field field = MetricsFactory.class.getDeclaredField("metricsProxy"); @@ -216,7 +216,7 @@ void shouldUseCustomFunctionNameWhenProvidedForColdStartMetric() throws Exceptio JsonNode dimensions = coldStartNode.get("_aws").get("CloudWatchMetrics").get(0).get("Dimensions").get(0); boolean hasFunctionName = false; for (JsonNode dimension : dimensions) { - if (dimension.asText().equals("FunctionName")) { + if ("FunctionName".equals(dimension.asText())) { hasFunctionName = true; break; }