From 3eb70851aa1e1f830c83b38259d332024c4f598e Mon Sep 17 00:00:00 2001 From: cleverchuk Date: Mon, 4 May 2026 09:16:31 -0400 Subject: [PATCH] remove workaround, bug fixed in sdk v1.61.0. --- .../extensions/PropagatedContext.java | 41 ------- .../SolarwindsContextPropagator.java | 1 - .../extensions/SolarwindsSampler.java | 103 ++++++++---------- .../SolarwindsContextPropagatorTest.java | 16 --- .../extensions/SolarwindsSamplerTest.java | 38 ------- 5 files changed, 47 insertions(+), 152 deletions(-) delete mode 100644 libs/shared/src/main/java/com/solarwinds/opentelemetry/extensions/PropagatedContext.java diff --git a/libs/shared/src/main/java/com/solarwinds/opentelemetry/extensions/PropagatedContext.java b/libs/shared/src/main/java/com/solarwinds/opentelemetry/extensions/PropagatedContext.java deleted file mode 100644 index 023f4908..00000000 --- a/libs/shared/src/main/java/com/solarwinds/opentelemetry/extensions/PropagatedContext.java +++ /dev/null @@ -1,41 +0,0 @@ -/* - * © SolarWinds Worldwide, LLC. All rights reserved. - * - * 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 com.solarwinds.opentelemetry.extensions; - -import com.solarwinds.joboe.sampling.XtraceOptions; - -/** - * Thread-local workaround for OTel SDK replacing the parent context with a primordial context for - * root spans (see open-telemetry/opentelemetry-java#8012). Remove once the upstream fix is shipped. - */ -final class PropagatedContext { - private static final ThreadLocal XTRACE_OPTIONS = new ThreadLocal<>(); - - private PropagatedContext() {} - - static void set(XtraceOptions options) { - XTRACE_OPTIONS.set(options); - } - - static XtraceOptions getXtraceOptions() { - return XTRACE_OPTIONS.get(); - } - - static void clear() { - XTRACE_OPTIONS.remove(); - } -} diff --git a/libs/shared/src/main/java/com/solarwinds/opentelemetry/extensions/SolarwindsContextPropagator.java b/libs/shared/src/main/java/com/solarwinds/opentelemetry/extensions/SolarwindsContextPropagator.java index 814ded61..2439f7a6 100644 --- a/libs/shared/src/main/java/com/solarwinds/opentelemetry/extensions/SolarwindsContextPropagator.java +++ b/libs/shared/src/main/java/com/solarwinds/opentelemetry/extensions/SolarwindsContextPropagator.java @@ -171,7 +171,6 @@ public Context extract(Context context, C carrier, TextMapGetter getter) context = context.with(TriggerTraceContextKey.XTRACE_OPTIONS_SIGNATURE, traceOptionsSignature); } - PropagatedContext.set(xTraceOptions); } final String traceState = getter.get(carrier, TRACE_STATE); diff --git a/libs/shared/src/main/java/com/solarwinds/opentelemetry/extensions/SolarwindsSampler.java b/libs/shared/src/main/java/com/solarwinds/opentelemetry/extensions/SolarwindsSampler.java index fbc1b5ef..6fca103d 100644 --- a/libs/shared/src/main/java/com/solarwinds/opentelemetry/extensions/SolarwindsSampler.java +++ b/libs/shared/src/main/java/com/solarwinds/opentelemetry/extensions/SolarwindsSampler.java @@ -104,77 +104,68 @@ public SamplingResult shouldSample( final SamplingResult samplingResult; final AttributesBuilder additionalAttributesBuilder = Attributes.builder(); - XtraceOptions xTraceOptions = parentContext.get(TriggerTraceContextKey.KEY); - - if (xTraceOptions == null) { - xTraceOptions = PropagatedContext.getXtraceOptions(); - } + final XtraceOptions xTraceOptions = parentContext.get(TriggerTraceContextKey.KEY); String xtraceOptionsResponseStr = null; List signals = Arrays.asList( constructUrl(attributes), String.format(LAYER_NAME_PLACEHOLDER, spanKind, name.trim())); - try { - if (!parentSpanContext.isValid()) { // no valid traceparent, it is a new trace - TraceDecision traceDecision = shouldTraceRequest(name, null, xTraceOptions, signals); - samplingResult = toOtSamplingResult(traceDecision, xTraceOptions, true); - XTraceOptionsResponse xtraceOptionsResponse = - XTraceOptionsResponse.computeResponse(xTraceOptions, traceDecision, true); + if (!parentSpanContext.isValid()) { // no valid traceparent, it is a new trace + TraceDecision traceDecision = shouldTraceRequest(name, null, xTraceOptions, signals); + samplingResult = toOtSamplingResult(traceDecision, xTraceOptions, true); + XTraceOptionsResponse xtraceOptionsResponse = + XTraceOptionsResponse.computeResponse(xTraceOptions, traceDecision, true); - if (xtraceOptionsResponse != null) { - xtraceOptionsResponseStr = xtraceOptionsResponse.toString(); - } + if (xtraceOptionsResponse != null) { + xtraceOptionsResponseStr = xtraceOptionsResponse.toString(); + } - } else if (parentSpanContext.isRemote()) { - final String swTraceState = traceState.get(SW_TRACESTATE_KEY); - - if (SamplingUtil.isValidSwTraceState(swTraceState)) { // pass through for request counting - additionalAttributesBuilder.put(Constants.SW_PARENT_ID, swTraceState.split("-")[0]); - final String xTraceId = Util.w3cContextToHexString(parentSpanContext); - final TraceDecision traceDecision = - shouldTraceRequest(name, xTraceId, xTraceOptions, signals); - - samplingResult = toOtSamplingResult(traceDecision, xTraceOptions, false); - final XTraceOptionsResponse xTraceOptionsResponse = - XTraceOptionsResponse.computeResponse(xTraceOptions, traceDecision, false); - - if (xTraceOptionsResponse != null) { - xtraceOptionsResponseStr = xTraceOptionsResponse.toString(); - } - - } else { // no swTraceState, treat it as a new trace - final TraceDecision traceDecision = - shouldTraceRequest(name, null, xTraceOptions, signals); - samplingResult = toOtSamplingResult(traceDecision, xTraceOptions, true); - - final XTraceOptionsResponse xTraceOptionsResponse = - XTraceOptionsResponse.computeResponse(xTraceOptions, traceDecision, true); - if (xTraceOptionsResponse != null) { - xtraceOptionsResponseStr = xTraceOptionsResponse.toString(); - } - } + } else if (parentSpanContext.isRemote()) { + final String swTraceState = traceState.get(SW_TRACESTATE_KEY); + + if (SamplingUtil.isValidSwTraceState(swTraceState)) { // pass through for request counting + additionalAttributesBuilder.put(Constants.SW_PARENT_ID, swTraceState.split("-")[0]); + final String xTraceId = Util.w3cContextToHexString(parentSpanContext); + final TraceDecision traceDecision = + shouldTraceRequest(name, xTraceId, xTraceOptions, signals); + + samplingResult = toOtSamplingResult(traceDecision, xTraceOptions, false); + final XTraceOptionsResponse xTraceOptionsResponse = + XTraceOptionsResponse.computeResponse(xTraceOptions, traceDecision, false); - final String traceStateValue = parentContext.get(TraceStateKey.KEY); - if (traceStateValue != null) { - additionalAttributesBuilder.put(Constants.SW_UPSTREAM_TRACESTATE, traceStateValue); + if (xTraceOptionsResponse != null) { + xtraceOptionsResponseStr = xTraceOptionsResponse.toString(); } - } else { // local span, continue with parent based sampling - samplingResult = - Sampler.parentBased(Sampler.alwaysOff()) - .shouldSample(parentContext, traceId, name, spanKind, attributes, parentLinks); + } else { // no swTraceState, treat it as a new trace + final TraceDecision traceDecision = shouldTraceRequest(name, null, xTraceOptions, signals); + samplingResult = toOtSamplingResult(traceDecision, xTraceOptions, true); + + final XTraceOptionsResponse xTraceOptionsResponse = + XTraceOptionsResponse.computeResponse(xTraceOptions, traceDecision, true); + if (xTraceOptionsResponse != null) { + xtraceOptionsResponseStr = xTraceOptionsResponse.toString(); + } } - SamplingResult result = - TraceStateSamplingResult.wrap( - samplingResult, additionalAttributesBuilder.build(), xtraceOptionsResponseStr); + final String traceStateValue = parentContext.get(TraceStateKey.KEY); + if (traceStateValue != null) { + additionalAttributesBuilder.put(Constants.SW_UPSTREAM_TRACESTATE, traceStateValue); + } - logger.trace(String.format("Sampling decision: %s", result.getDecision())); - return result; - } finally { - PropagatedContext.clear(); + } else { // local span, continue with parent based sampling + samplingResult = + Sampler.parentBased(Sampler.alwaysOff()) + .shouldSample(parentContext, traceId, name, spanKind, attributes, parentLinks); } + + SamplingResult result = + TraceStateSamplingResult.wrap( + samplingResult, additionalAttributesBuilder.build(), xtraceOptionsResponseStr); + + logger.trace(String.format("Sampling decision: %s", result.getDecision())); + return result; } String constructUrl(Attributes attributes) { diff --git a/libs/shared/src/test/java/com/solarwinds/opentelemetry/extensions/SolarwindsContextPropagatorTest.java b/libs/shared/src/test/java/com/solarwinds/opentelemetry/extensions/SolarwindsContextPropagatorTest.java index acc90c5c..c601bca6 100644 --- a/libs/shared/src/test/java/com/solarwinds/opentelemetry/extensions/SolarwindsContextPropagatorTest.java +++ b/libs/shared/src/test/java/com/solarwinds/opentelemetry/extensions/SolarwindsContextPropagatorTest.java @@ -203,22 +203,6 @@ void verifyThatXtraceOptionsSignatureIsExtractedAndPutIntoContext() { assertEquals("test-sig", newContext.get(XTRACE_OPTIONS_SIGNATURE)); } - @Test - void verifyThatExtractPopulatesPropagatedContext() { - PropagatedContext.clear(); - final Map carrier = - new HashMap() { - { - put("X-Trace-Options", "trigger-trace;custom-senderhost=chubi;"); - } - }; - - solarwindsContextPropagator.extract(Context.current(), carrier, textMapGetterStub); - - assertNotNull(PropagatedContext.getXtraceOptions()); - PropagatedContext.clear(); - } - @Test void verifyThatTracestateIsExtractedAndPutIntoContext() { final Map carrier = diff --git a/libs/shared/src/test/java/com/solarwinds/opentelemetry/extensions/SolarwindsSamplerTest.java b/libs/shared/src/test/java/com/solarwinds/opentelemetry/extensions/SolarwindsSamplerTest.java index d6f08ea5..95542141 100644 --- a/libs/shared/src/test/java/com/solarwinds/opentelemetry/extensions/SolarwindsSamplerTest.java +++ b/libs/shared/src/test/java/com/solarwinds/opentelemetry/extensions/SolarwindsSamplerTest.java @@ -269,44 +269,6 @@ void returnDropDecisionWhenLocalSpanAndParentIsNotSampled() { } } - @Test - void verifyThatSamplerFallsBackToPropagatedContextForRootSpan() { - try (MockedStatic spanMockedStatic = mockStatic(Span.class); - MockedStatic traceDecisionUtilMockedStatic = - mockStatic(TraceDecisionUtil.class)) { - spanMockedStatic.when(() -> Span.fromContext(any())).thenReturn(spanMock); - - when(spanContextMock.isValid()).thenReturn(false); - when(spanMock.getSpanContext()).thenReturn(spanContextMock); - - when(traceDecisionMock.isSampled()).thenReturn(false); - when(traceDecisionMock.isReportMetrics()).thenReturn(false); - - XtraceOptions realOptions = - XtraceOptions.getXTraceOptions("trigger-trace;custom-senderhost=test;", null); - - ArgumentCaptor xtraceCaptor = ArgumentCaptor.forClass(XtraceOptions.class); - traceDecisionUtilMockedStatic - .when(() -> TraceDecisionUtil.shouldTraceRequest(any(), any(), any(), any())) - .thenReturn(traceDecisionMock); - - PropagatedContext.set(realOptions); - - tested.shouldSample( - Context.root(), - idGenerator.generateTraceId(), - "name", - SpanKind.SERVER, - Attributes.empty(), - Collections.emptyList()); - - traceDecisionUtilMockedStatic.verify( - () -> TraceDecisionUtil.shouldTraceRequest(any(), any(), xtraceCaptor.capture(), any())); - assertEquals(realOptions, xtraceCaptor.getValue()); - assertNull(PropagatedContext.getXtraceOptions()); - } - } - @ParameterizedTest @MethodSource("constructParams") void testConstructUrl(String expected, Attributes attributes) {