From 00558e319acfb949ec952c86610fae05105ea606 Mon Sep 17 00:00:00 2001 From: Gagan Juneja Date: Sat, 12 Aug 2023 01:10:58 +0530 Subject: [PATCH] Adds attributes to startSpan (#9199) * Adds attributes to startSpan Signed-off-by: Gagan Juneja * Update Changelog Signed-off-by: Gagan Juneja * Adds attributes to startSpan Signed-off-by: Gagan Juneja * Refactor code Signed-off-by: Gagan Juneja * Add java doc Signed-off-by: Gagan Juneja * Refactor code Signed-off-by: Gagan Juneja * Refactor code Signed-off-by: Gagan Juneja * Refactor code Signed-off-by: Gagan Juneja * Removes dependency Signed-off-by: Gagan Juneja --------- Signed-off-by: Gagan Juneja Co-authored-by: Gagan Juneja --- CHANGELOG.md | 3 +- .../telemetry/tracing/DefaultTracer.java | 18 ++-- .../opensearch/telemetry/tracing/Tracer.java | 15 ++- .../telemetry/tracing/TracingTelemetry.java | 4 +- .../tracing/attributes/Attributes.java | 94 +++++++++++++++++++ .../tracing/attributes/package-info.java | 12 +++ .../telemetry/tracing/noop/NoopTracer.java | 12 ++- .../tracing/runnable/TraceableRunnable.java | 8 +- .../telemetry/tracing/DefaultTracerTests.java | 47 +++++++++- .../tracing/TraceableRunnableTests.java | 12 ++- .../tracing/OTelAttributesConverter.java | 51 ++++++++++ .../tracing/OTelTracingTelemetry.java | 19 ++-- .../tracing/OTelAttributesConverterTests.java | 48 ++++++++++ .../tracing/OTelTracingTelemetryTests.java | 62 +++++++++++- .../telemetry/tracing/WrappedTracer.java | 12 ++- .../telemetry/tracing/WrappedTracerTests.java | 18 +++- .../test/telemetry/tracing/MockSpan.java | 37 +++++++- .../tracing/MockTracingContextPropagator.java | 3 +- .../tracing/MockTracingTelemetry.java | 5 +- 19 files changed, 440 insertions(+), 40 deletions(-) create mode 100644 libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/attributes/Attributes.java create mode 100644 libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/attributes/package-info.java create mode 100644 plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelAttributesConverter.java create mode 100644 plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/OTelAttributesConverterTests.java diff --git a/CHANGELOG.md b/CHANGELOG.md index 6b677e9d33849..59de7ea2fc2d4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -127,6 +127,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - [Remove] Deprecated Fractional ByteSizeValue support #9005 ([#9005](https://github.com/opensearch-project/OpenSearch/pull/9005)) - Make MultiBucketConsumerService thread safe to use across slices during search ([#9047](https://github.com/opensearch-project/OpenSearch/pull/9047)) - Change shard_size and shard_min_doc_count evaluation to happen in shard level reduce phase ([#9085](https://github.com/opensearch-project/OpenSearch/pull/9085)) +- Add attributes to startSpan methods ([#9199](https://github.com/opensearch-project/OpenSearch/pull/9199)) ### Deprecated @@ -138,4 +139,4 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), ### Security [Unreleased 3.0]: https://github.com/opensearch-project/OpenSearch/compare/2.x...HEAD -[Unreleased 2.x]: https://github.com/opensearch-project/OpenSearch/compare/2.10...2.x \ No newline at end of file +[Unreleased 2.x]: https://github.com/opensearch-project/OpenSearch/compare/2.10...2.x diff --git a/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/DefaultTracer.java b/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/DefaultTracer.java index f46c3e57b6c4b..98d7863bc6bfe 100644 --- a/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/DefaultTracer.java +++ b/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/DefaultTracer.java @@ -10,6 +10,7 @@ import java.io.Closeable; import java.io.IOException; +import org.opensearch.telemetry.tracing.attributes.Attributes; /** * @@ -37,16 +38,21 @@ public DefaultTracer(TracingTelemetry tracingTelemetry, TracerContextStorage attributesMap; + /** + * Empty value. + */ + public final static Attributes EMPTY = new Attributes(Collections.emptyMap()); + + /** + * Factory method. + * @return attributes. + */ + public static Attributes create() { + return new Attributes(new HashMap<>()); + } + + /** + * Constructor. + */ + private Attributes(Map attributesMap) { + this.attributesMap = attributesMap; + } + + /** + * Add String attribute. + * @param key key + * @param value value + * @return Same instance. + */ + public Attributes addAttribute(String key, String value) { + Objects.requireNonNull(value, "value cannot be null"); + attributesMap.put(key, value); + return this; + } + + /** + * Add long attribute. + * @param key key + * @param value value + * @return Same instance. + */ + public Attributes addAttribute(String key, long value) { + attributesMap.put(key, value); + return this; + }; + + /** + * Add double attribute. + * @param key key + * @param value value + * @return Same instance. + */ + public Attributes addAttribute(String key, double value) { + attributesMap.put(key, value); + return this; + }; + + /** + * Add boolean attribute. + * @param key key + * @param value value + * @return Same instance. + */ + public Attributes addAttribute(String key, boolean value) { + attributesMap.put(key, value); + return this; + }; + + /** + * Returns the attribute map. + * @return attributes map + */ + public Map getAttributesMap() { + return Collections.unmodifiableMap(attributesMap); + } + +} diff --git a/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/attributes/package-info.java b/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/attributes/package-info.java new file mode 100644 index 0000000000000..91d1c3291a4a5 --- /dev/null +++ b/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/attributes/package-info.java @@ -0,0 +1,12 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +/** + * Contains No-op implementations + */ +package org.opensearch.telemetry.tracing.attributes; diff --git a/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/noop/NoopTracer.java b/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/noop/NoopTracer.java index 74480844f5a3c..8ac0deebbcfca 100644 --- a/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/noop/NoopTracer.java +++ b/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/noop/NoopTracer.java @@ -11,6 +11,7 @@ import org.opensearch.telemetry.tracing.SpanScope; import org.opensearch.telemetry.tracing.Tracer; import org.opensearch.telemetry.tracing.SpanContext; +import org.opensearch.telemetry.tracing.attributes.Attributes; /** * No-op implementation of Tracer @@ -32,15 +33,20 @@ public SpanScope startSpan(String spanName) { } @Override - public SpanContext getCurrentSpan() { - return null; + public SpanScope startSpan(String spanName, Attributes attributes) { + return SpanScope.NO_OP; } @Override - public SpanScope startSpan(String spanName, SpanContext parentSpan) { + public SpanScope startSpan(String spanName, SpanContext parentSpan, Attributes attributes) { return SpanScope.NO_OP; } + @Override + public SpanContext getCurrentSpan() { + return null; + } + @Override public void close() { diff --git a/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/runnable/TraceableRunnable.java b/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/runnable/TraceableRunnable.java index 5f179467d5426..54a5a7f1678e6 100644 --- a/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/runnable/TraceableRunnable.java +++ b/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/runnable/TraceableRunnable.java @@ -11,6 +11,7 @@ import org.opensearch.telemetry.tracing.SpanContext; import org.opensearch.telemetry.tracing.SpanScope; import org.opensearch.telemetry.tracing.Tracer; +import org.opensearch.telemetry.tracing.attributes.Attributes; /** * Wraps the runnable and add instrumentation to trace the {@link Runnable} @@ -20,24 +21,27 @@ public class TraceableRunnable implements Runnable { private final SpanContext parent; private final Tracer tracer; private final String spanName; + private final Attributes attributes; /** * Constructor. * @param tracer tracer * @param spanName spanName * @param parent parent Span. + * @param attributes attributes. * @param runnable runnable. */ - public TraceableRunnable(Tracer tracer, String spanName, SpanContext parent, Runnable runnable) { + public TraceableRunnable(Tracer tracer, String spanName, SpanContext parent, Attributes attributes, Runnable runnable) { this.tracer = tracer; this.spanName = spanName; this.parent = parent; + this.attributes = attributes; this.runnable = runnable; } @Override public void run() { - try (SpanScope spanScope = tracer.startSpan(spanName, parent)) { + try (SpanScope spanScope = tracer.startSpan(spanName, parent, attributes)) { runnable.run(); } } diff --git a/libs/telemetry/src/test/java/org/opensearch/telemetry/tracing/DefaultTracerTests.java b/libs/telemetry/src/test/java/org/opensearch/telemetry/tracing/DefaultTracerTests.java index 6c39b81783f65..e0d9889855414 100644 --- a/libs/telemetry/src/test/java/org/opensearch/telemetry/tracing/DefaultTracerTests.java +++ b/libs/telemetry/src/test/java/org/opensearch/telemetry/tracing/DefaultTracerTests.java @@ -11,12 +11,17 @@ import org.junit.Assert; import org.opensearch.common.settings.Settings; import org.opensearch.common.util.concurrent.ThreadContext; +import org.opensearch.telemetry.tracing.attributes.Attributes; import org.opensearch.test.OpenSearchTestCase; import java.io.IOException; +import org.opensearch.test.telemetry.tracing.MockSpan; import org.opensearch.test.telemetry.tracing.MockTracingTelemetry; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; import static org.mockito.Mockito.when; import static org.mockito.Mockito.verify; @@ -46,6 +51,42 @@ public void testCreateSpan() { Assert.assertEquals("span_name", defaultTracer.getCurrentSpan().getSpan().getSpanName()); } + public void testCreateSpanWithAttributesWithMock() { + DefaultTracer defaultTracer = new DefaultTracer(mockTracingTelemetry, mockTracerContextStorage); + Attributes attributes = Attributes.create().addAttribute("name", "value"); + when(mockTracingTelemetry.createSpan("span_name", mockParentSpan, attributes)).thenReturn(mockSpan); + defaultTracer.startSpan("span_name", attributes); + verify(mockTracingTelemetry).createSpan("span_name", mockParentSpan, attributes); + } + + public void testCreateSpanWithAttributesWithParentMock() { + DefaultTracer defaultTracer = new DefaultTracer(mockTracingTelemetry, mockTracerContextStorage); + Attributes attributes = Attributes.create().addAttribute("name", "value"); + when(mockTracingTelemetry.createSpan("span_name", mockParentSpan, attributes)).thenReturn(mockSpan); + defaultTracer.startSpan("span_name", new SpanContext(mockParentSpan), attributes); + verify(mockTracingTelemetry).createSpan("span_name", mockParentSpan, attributes); + verify(mockTracerContextStorage, never()).get(TracerContextStorage.CURRENT_SPAN); + } + + public void testCreateSpanWithAttributes() { + TracingTelemetry tracingTelemetry = new MockTracingTelemetry(); + DefaultTracer defaultTracer = new DefaultTracer( + tracingTelemetry, + new ThreadContextBasedTracerContextStorage(new ThreadContext(Settings.EMPTY), tracingTelemetry) + ); + + defaultTracer.startSpan( + "span_name", + Attributes.create().addAttribute("key1", 1.0).addAttribute("key2", 2l).addAttribute("key3", true).addAttribute("key4", "key4") + ); + + Assert.assertEquals("span_name", defaultTracer.getCurrentSpan().getSpan().getSpanName()); + Assert.assertEquals(1.0, ((MockSpan) defaultTracer.getCurrentSpan().getSpan()).getAttribute("key1")); + Assert.assertEquals(2l, ((MockSpan) defaultTracer.getCurrentSpan().getSpan()).getAttribute("key2")); + Assert.assertEquals(true, ((MockSpan) defaultTracer.getCurrentSpan().getSpan()).getAttribute("key3")); + Assert.assertEquals("key4", ((MockSpan) defaultTracer.getCurrentSpan().getSpan()).getAttribute("key4")); + } + public void testCreateSpanWithParent() { TracingTelemetry tracingTelemetry = new MockTracingTelemetry(); DefaultTracer defaultTracer = new DefaultTracer( @@ -57,7 +98,7 @@ public void testCreateSpanWithParent() { SpanContext parentSpan = defaultTracer.getCurrentSpan(); - defaultTracer.startSpan("span_name_1", parentSpan); + defaultTracer.startSpan("span_name_1", parentSpan, Attributes.EMPTY); Assert.assertEquals("span_name_1", defaultTracer.getCurrentSpan().getSpan().getSpanName()); Assert.assertEquals(parentSpan.getSpan(), defaultTracer.getCurrentSpan().getSpan().getParentSpan()); @@ -70,7 +111,7 @@ public void testCreateSpanWithNullParent() { new ThreadContextBasedTracerContextStorage(new ThreadContext(Settings.EMPTY), tracingTelemetry) ); - defaultTracer.startSpan("span_name", null); + defaultTracer.startSpan("span_name", null, Attributes.EMPTY); Assert.assertEquals("span_name", defaultTracer.getCurrentSpan().getSpan().getSpanName()); Assert.assertEquals(null, defaultTracer.getCurrentSpan().getSpan().getParentSpan()); @@ -105,6 +146,6 @@ private void setupMocks() { when(mockParentSpan.getSpanId()).thenReturn("parent_span_id"); when(mockParentSpan.getTraceId()).thenReturn("trace_id"); when(mockTracerContextStorage.get(TracerContextStorage.CURRENT_SPAN)).thenReturn(mockParentSpan, mockSpan); - when(mockTracingTelemetry.createSpan("span_name", mockParentSpan)).thenReturn(mockSpan); + when(mockTracingTelemetry.createSpan(eq("span_name"), eq(mockParentSpan), any(Attributes.class))).thenReturn(mockSpan); } } diff --git a/libs/telemetry/src/test/java/org/opensearch/telemetry/tracing/TraceableRunnableTests.java b/libs/telemetry/src/test/java/org/opensearch/telemetry/tracing/TraceableRunnableTests.java index 28cd036d44e5e..32ae48fd479a4 100644 --- a/libs/telemetry/src/test/java/org/opensearch/telemetry/tracing/TraceableRunnableTests.java +++ b/libs/telemetry/src/test/java/org/opensearch/telemetry/tracing/TraceableRunnableTests.java @@ -12,8 +12,10 @@ import java.util.concurrent.atomic.AtomicReference; import org.opensearch.common.settings.Settings; import org.opensearch.common.util.concurrent.ThreadContext; +import org.opensearch.telemetry.tracing.attributes.Attributes; import org.opensearch.telemetry.tracing.runnable.TraceableRunnable; import org.opensearch.test.OpenSearchTestCase; +import org.opensearch.test.telemetry.tracing.MockSpan; import org.opensearch.test.telemetry.tracing.MockTracingTelemetry; public class TraceableRunnableTests extends OpenSearchTestCase { @@ -27,16 +29,22 @@ public void testRunnableWithNullParent() throws Exception { String spanName = "testRunnable"; DefaultTracer defaultTracer = new DefaultTracer(new MockTracingTelemetry(), contextStorage); final AtomicBoolean isRunnableCompleted = new AtomicBoolean(false); + TraceableRunnable traceableRunnable = new TraceableRunnable( defaultTracer, spanName, null, - () -> { isRunnableCompleted.set(true); } + Attributes.create().addAttribute("name", "value"), + () -> { + isRunnableCompleted.set(true); + } ); traceableRunnable.run(); assertTrue(isRunnableCompleted.get()); assertEquals(spanName, defaultTracer.getCurrentSpan().getSpan().getSpanName()); assertEquals(null, defaultTracer.getCurrentSpan().getSpan().getParentSpan()); + assertEquals("value", ((MockSpan) defaultTracer.getCurrentSpan().getSpan()).getAttribute("name")); + } public void testRunnableWithParent() throws Exception { @@ -47,7 +55,7 @@ public void testRunnableWithParent() throws Exception { SpanContext parentSpan = defaultTracer.getCurrentSpan(); AtomicReference currrntSpan = new AtomicReference<>(new SpanContext(null)); final AtomicBoolean isRunnableCompleted = new AtomicBoolean(false); - TraceableRunnable traceableRunnable = new TraceableRunnable(defaultTracer, spanName, parentSpan, () -> { + TraceableRunnable traceableRunnable = new TraceableRunnable(defaultTracer, spanName, parentSpan, Attributes.EMPTY, () -> { isRunnableCompleted.set(true); currrntSpan.set(defaultTracer.getCurrentSpan()); }); diff --git a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelAttributesConverter.java b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelAttributesConverter.java new file mode 100644 index 0000000000000..ecb2296f959a2 --- /dev/null +++ b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelAttributesConverter.java @@ -0,0 +1,51 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.telemetry.tracing; + +import io.opentelemetry.api.common.Attributes; +import io.opentelemetry.api.common.AttributesBuilder; +import java.util.Locale; + +/** + * Converts {@link org.opensearch.telemetry.tracing.attributes.Attributes} to OTel {@link Attributes} + */ +final class OTelAttributesConverter { + + /** + * Constructor. + */ + private OTelAttributesConverter() {} + + /** + * Attribute converter. + * @param attributes attributes + * @return otel attributes. + */ + static Attributes convert(org.opensearch.telemetry.tracing.attributes.Attributes attributes) { + AttributesBuilder attributesBuilder = Attributes.builder(); + if (attributes != null) { + attributes.getAttributesMap().forEach((x, y) -> addSpanAttribute(x, y, attributesBuilder)); + } + return attributesBuilder.build(); + } + + private static void addSpanAttribute(String key, Object value, AttributesBuilder attributesBuilder) { + if (value instanceof Boolean) { + attributesBuilder.put(key, (Boolean) value); + } else if (value instanceof Long) { + attributesBuilder.put(key, (Long) value); + } else if (value instanceof Double) { + attributesBuilder.put(key, (Double) value); + } else if (value instanceof String) { + attributesBuilder.put(key, (String) value); + } else { + throw new IllegalArgumentException(String.format(Locale.ROOT, "Span attribute value %s type not supported", value)); + } + } +} diff --git a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelTracingTelemetry.java b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelTracingTelemetry.java index 8a0034e098461..15914d7531afa 100644 --- a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelTracingTelemetry.java +++ b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelTracingTelemetry.java @@ -15,6 +15,7 @@ import java.io.Closeable; import java.io.IOException; +import org.opensearch.telemetry.tracing.attributes.Attributes; /** * OTel based Telemetry provider @@ -22,7 +23,6 @@ public class OTelTracingTelemetry implements TracingTelemetry { private static final Logger logger = LogManager.getLogger(OTelTracingTelemetry.class); - private final OpenTelemetry openTelemetry; private final io.opentelemetry.api.trace.Tracer otelTracer; @@ -46,8 +46,8 @@ public void close() { } @Override - public Span createSpan(String spanName, Span parentSpan) { - return createOtelSpan(spanName, parentSpan); + public Span createSpan(String spanName, Span parentSpan, Attributes attributes) { + return createOtelSpan(spanName, parentSpan, attributes); } @Override @@ -55,14 +55,17 @@ public TracingContextPropagator getContextPropagator() { return new OTelTracingContextPropagator(openTelemetry); } - private Span createOtelSpan(String spanName, Span parentSpan) { - io.opentelemetry.api.trace.Span otelSpan = otelSpan(spanName, parentSpan); + private Span createOtelSpan(String spanName, Span parentSpan, Attributes attributes) { + io.opentelemetry.api.trace.Span otelSpan = otelSpan(spanName, parentSpan, OTelAttributesConverter.convert(attributes)); return new OTelSpan(spanName, otelSpan, parentSpan); } - io.opentelemetry.api.trace.Span otelSpan(String spanName, Span parentOTelSpan) { + io.opentelemetry.api.trace.Span otelSpan(String spanName, Span parentOTelSpan, io.opentelemetry.api.common.Attributes attributes) { return parentOTelSpan == null || !(parentOTelSpan instanceof OTelSpan) - ? otelTracer.spanBuilder(spanName).startSpan() - : otelTracer.spanBuilder(spanName).setParent(Context.current().with(((OTelSpan) parentOTelSpan).getDelegateSpan())).startSpan(); + ? otelTracer.spanBuilder(spanName).setAllAttributes(attributes).startSpan() + : otelTracer.spanBuilder(spanName) + .setParent(Context.current().with(((OTelSpan) parentOTelSpan).getDelegateSpan())) + .setAllAttributes(attributes) + .startSpan(); } } diff --git a/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/OTelAttributesConverterTests.java b/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/OTelAttributesConverterTests.java new file mode 100644 index 0000000000000..ae304286fc0fd --- /dev/null +++ b/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/OTelAttributesConverterTests.java @@ -0,0 +1,48 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.telemetry.tracing; + +import io.opentelemetry.api.common.AttributeType; +import io.opentelemetry.api.internal.InternalAttributeKeyImpl; +import java.util.Map; +import org.opensearch.telemetry.tracing.attributes.Attributes; +import org.opensearch.test.OpenSearchTestCase; + +public class OTelAttributesConverterTests extends OpenSearchTestCase { + + public void testConverterNullAttributes() { + io.opentelemetry.api.common.Attributes otelAttributes = OTelAttributesConverter.convert(null); + assertEquals(0, otelAttributes.size()); + } + + public void testConverterEmptyAttributes() { + Attributes attributes = Attributes.EMPTY; + io.opentelemetry.api.common.Attributes otelAttributes = OTelAttributesConverter.convert(null); + assertEquals(0, otelAttributes.size()); + } + + public void testConverterSingleAttributes() { + Attributes attributes = Attributes.create().addAttribute("key1", "value"); + io.opentelemetry.api.common.Attributes otelAttributes = OTelAttributesConverter.convert(attributes); + assertEquals(1, otelAttributes.size()); + assertEquals("value", otelAttributes.get(InternalAttributeKeyImpl.create("key1", AttributeType.STRING))); + } + + public void testConverterMultipleAttributes() { + Attributes attributes = Attributes.create() + .addAttribute("key1", 1l) + .addAttribute("key2", 1.0) + .addAttribute("key3", true) + .addAttribute("key4", "value4"); + Map attributeMap = attributes.getAttributesMap(); + io.opentelemetry.api.common.Attributes otelAttributes = OTelAttributesConverter.convert(attributes); + assertEquals(4, otelAttributes.size()); + otelAttributes.asMap().forEach((x, y) -> assertEquals(attributeMap.get(x.getKey()), y)); + } +} diff --git a/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/OTelTracingTelemetryTests.java b/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/OTelTracingTelemetryTests.java index 7dec7824b9790..7ff99787b240b 100644 --- a/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/OTelTracingTelemetryTests.java +++ b/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/OTelTracingTelemetryTests.java @@ -9,8 +9,12 @@ package org.opensearch.telemetry.tracing; import io.opentelemetry.api.OpenTelemetry; +import io.opentelemetry.api.common.AttributesBuilder; import io.opentelemetry.api.trace.SpanBuilder; import io.opentelemetry.api.trace.Tracer; +import java.util.Collections; +import java.util.Map; +import org.opensearch.telemetry.tracing.attributes.Attributes; import org.opensearch.test.OpenSearchTestCase; import static org.mockito.ArgumentMatchers.any; @@ -27,12 +31,15 @@ public void testCreateSpanWithoutParent() { when(mockOpenTelemetry.getTracer("os-tracer")).thenReturn(mockTracer); SpanBuilder mockSpanBuilder = mock(SpanBuilder.class); when(mockTracer.spanBuilder("span_name")).thenReturn(mockSpanBuilder); + when(mockSpanBuilder.setAllAttributes(any(io.opentelemetry.api.common.Attributes.class))).thenReturn(mockSpanBuilder); when(mockSpanBuilder.startSpan()).thenReturn(mock(io.opentelemetry.api.trace.Span.class)); - + Map attributeMap = Collections.singletonMap("name", "value"); + Attributes attributes = Attributes.create().addAttribute("name", "value"); TracingTelemetry tracingTelemetry = new OTelTracingTelemetry(mockOpenTelemetry); - Span span = tracingTelemetry.createSpan("span_name", null); + Span span = tracingTelemetry.createSpan("span_name", null, attributes); verify(mockSpanBuilder, never()).setParent(any()); + verify(mockSpanBuilder).setAllAttributes(createAttribute(attributes)); assertNull(span.getParentSpan()); } @@ -43,18 +50,67 @@ public void testCreateSpanWithParent() { SpanBuilder mockSpanBuilder = mock(SpanBuilder.class); when(mockTracer.spanBuilder("span_name")).thenReturn(mockSpanBuilder); when(mockSpanBuilder.setParent(any())).thenReturn(mockSpanBuilder); + when(mockSpanBuilder.setAllAttributes(any(io.opentelemetry.api.common.Attributes.class))).thenReturn(mockSpanBuilder); + when(mockSpanBuilder.startSpan()).thenReturn(mock(io.opentelemetry.api.trace.Span.class)); + + Span parentSpan = new OTelSpan("parent_span", mock(io.opentelemetry.api.trace.Span.class), null); + + TracingTelemetry tracingTelemetry = new OTelTracingTelemetry(mockOpenTelemetry); + Attributes attributes = Attributes.create().addAttribute("name", 1l); + Span span = tracingTelemetry.createSpan("span_name", parentSpan, attributes); + + verify(mockSpanBuilder).setParent(any()); + verify(mockSpanBuilder).setAllAttributes(createAttributeLong(attributes)); + assertNotNull(span.getParentSpan()); + + assertEquals("parent_span", span.getParentSpan().getSpanName()); + } + + public void testCreateSpanWithParentWithMultipleAttributes() { + OpenTelemetry mockOpenTelemetry = mock(OpenTelemetry.class); + Tracer mockTracer = mock(Tracer.class); + when(mockOpenTelemetry.getTracer("os-tracer")).thenReturn(mockTracer); + SpanBuilder mockSpanBuilder = mock(SpanBuilder.class); + when(mockTracer.spanBuilder("span_name")).thenReturn(mockSpanBuilder); + when(mockSpanBuilder.setParent(any())).thenReturn(mockSpanBuilder); + when(mockSpanBuilder.setAllAttributes(any(io.opentelemetry.api.common.Attributes.class))).thenReturn(mockSpanBuilder); when(mockSpanBuilder.startSpan()).thenReturn(mock(io.opentelemetry.api.trace.Span.class)); Span parentSpan = new OTelSpan("parent_span", mock(io.opentelemetry.api.trace.Span.class), null); TracingTelemetry tracingTelemetry = new OTelTracingTelemetry(mockOpenTelemetry); - Span span = tracingTelemetry.createSpan("span_name", parentSpan); + Attributes attributes = Attributes.create() + .addAttribute("key1", 1l) + .addAttribute("key2", 2.0) + .addAttribute("key3", true) + .addAttribute("key4", "key4"); + Span span = tracingTelemetry.createSpan("span_name", parentSpan, attributes); + io.opentelemetry.api.common.Attributes otelAttributes = io.opentelemetry.api.common.Attributes.builder() + .put("key1", 1l) + .put("key2", 2.0) + .put("key3", true) + .put("key4", "key4") + .build(); verify(mockSpanBuilder).setParent(any()); + verify(mockSpanBuilder).setAllAttributes(otelAttributes); assertNotNull(span.getParentSpan()); + assertEquals("parent_span", span.getParentSpan().getSpanName()); } + private io.opentelemetry.api.common.Attributes createAttribute(Attributes attributes) { + AttributesBuilder attributesBuilder = io.opentelemetry.api.common.Attributes.builder(); + attributes.getAttributesMap().forEach((x, y) -> attributesBuilder.put(x, (String) y)); + return attributesBuilder.build(); + } + + private io.opentelemetry.api.common.Attributes createAttributeLong(Attributes attributes) { + AttributesBuilder attributesBuilder = io.opentelemetry.api.common.Attributes.builder(); + attributes.getAttributesMap().forEach((x, y) -> attributesBuilder.put(x, (Long) y)); + return attributesBuilder.build(); + } + public void testGetContextPropagator() { OpenTelemetry mockOpenTelemetry = mock(OpenTelemetry.class); Tracer mockTracer = mock(Tracer.class); diff --git a/server/src/main/java/org/opensearch/telemetry/tracing/WrappedTracer.java b/server/src/main/java/org/opensearch/telemetry/tracing/WrappedTracer.java index f633d3ebebe86..466abaac435f3 100644 --- a/server/src/main/java/org/opensearch/telemetry/tracing/WrappedTracer.java +++ b/server/src/main/java/org/opensearch/telemetry/tracing/WrappedTracer.java @@ -9,6 +9,7 @@ package org.opensearch.telemetry.tracing; import org.opensearch.telemetry.TelemetrySettings; +import org.opensearch.telemetry.tracing.attributes.Attributes; import org.opensearch.telemetry.tracing.noop.NoopTracer; import java.io.IOException; @@ -36,7 +37,12 @@ public WrappedTracer(TelemetrySettings telemetrySettings, Tracer defaultTracer) @Override public SpanScope startSpan(String spanName) { - return startSpan(spanName, null); + return startSpan(spanName, Attributes.EMPTY); + } + + @Override + public SpanScope startSpan(String spanName, Attributes attributes) { + return startSpan(spanName, null, attributes); } @Override @@ -46,9 +52,9 @@ public SpanContext getCurrentSpan() { } @Override - public SpanScope startSpan(String spanName, SpanContext parentSpan) { + public SpanScope startSpan(String spanName, SpanContext parentSpan, Attributes attributes) { Tracer delegateTracer = getDelegateTracer(); - return delegateTracer.startSpan(spanName, parentSpan); + return delegateTracer.startSpan(spanName, parentSpan, attributes); } @Override diff --git a/server/src/test/java/org/opensearch/telemetry/tracing/WrappedTracerTests.java b/server/src/test/java/org/opensearch/telemetry/tracing/WrappedTracerTests.java index 50c720e1c8555..f45381e3b4cc4 100644 --- a/server/src/test/java/org/opensearch/telemetry/tracing/WrappedTracerTests.java +++ b/server/src/test/java/org/opensearch/telemetry/tracing/WrappedTracerTests.java @@ -13,6 +13,7 @@ import org.opensearch.common.settings.Settings; import org.opensearch.common.util.FeatureFlags; import org.opensearch.telemetry.TelemetrySettings; +import org.opensearch.telemetry.tracing.attributes.Attributes; import org.opensearch.telemetry.tracing.noop.NoopTracer; import org.opensearch.test.OpenSearchTestCase; @@ -21,6 +22,8 @@ import java.util.List; import java.util.Set; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; @@ -48,7 +51,20 @@ public void testStartSpanWithTracingEnabledInvokesDefaultTracer() throws Excepti wrappedTracer.startSpan("foo"); assertTrue(wrappedTracer.getDelegateTracer() instanceof DefaultTracer); - verify(mockDefaultTracer).startSpan("foo", null); + verify(mockDefaultTracer).startSpan(eq("foo"), eq(null), any(Attributes.class)); + } + } + + public void testStartSpanWithTracingEnabledInvokesDefaultTracerWithAttr() throws Exception { + Settings settings = Settings.builder().put(TelemetrySettings.TRACER_ENABLED_SETTING.getKey(), true).build(); + TelemetrySettings telemetrySettings = new TelemetrySettings(settings, new ClusterSettings(settings, getClusterSettings())); + DefaultTracer mockDefaultTracer = mock(DefaultTracer.class); + Attributes attributes = Attributes.create().addAttribute("key", "value"); + try (WrappedTracer wrappedTracer = new WrappedTracer(telemetrySettings, mockDefaultTracer)) { + wrappedTracer.startSpan("foo", attributes); + + assertTrue(wrappedTracer.getDelegateTracer() instanceof DefaultTracer); + verify(mockDefaultTracer).startSpan("foo", null, attributes); } } diff --git a/test/telemetry/src/main/java/org/opensearch/test/telemetry/tracing/MockSpan.java b/test/telemetry/src/main/java/org/opensearch/test/telemetry/tracing/MockSpan.java index 876145f6bf653..a7744b8de50d5 100644 --- a/test/telemetry/src/main/java/org/opensearch/test/telemetry/tracing/MockSpan.java +++ b/test/telemetry/src/main/java/org/opensearch/test/telemetry/tracing/MockSpan.java @@ -15,6 +15,7 @@ import java.util.function.Supplier; import org.opensearch.telemetry.tracing.AbstractSpan; import org.opensearch.telemetry.tracing.Span; +import org.opensearch.telemetry.tracing.attributes.Attributes; /** * MockSpan for testing and strict check validations. Not to be used for production cases. @@ -37,6 +38,24 @@ public class MockSpan extends AbstractSpan { * @param spanName span name * @param parentSpan parent span * @param spanProcessor span processor + * @param attributes attributes + */ + public MockSpan(String spanName, Span parentSpan, SpanProcessor spanProcessor, Attributes attributes) { + this( + spanName, + parentSpan, + parentSpan != null ? parentSpan.getTraceId() : IdGenerator.generateTraceId(), + IdGenerator.generateSpanId(), + spanProcessor, + attributes + ); + } + + /** + * Constructor. + * @param spanName span name. + * @param parentSpan parent span name + * @param spanProcessor span processor. */ public MockSpan(String spanName, Span parentSpan, SpanProcessor spanProcessor) { this( @@ -44,7 +63,8 @@ public MockSpan(String spanName, Span parentSpan, SpanProcessor spanProcessor) { parentSpan, parentSpan != null ? parentSpan.getTraceId() : IdGenerator.generateTraceId(), IdGenerator.generateSpanId(), - spanProcessor + spanProcessor, + Attributes.EMPTY ); } @@ -55,14 +75,18 @@ public MockSpan(String spanName, Span parentSpan, SpanProcessor spanProcessor) { * @param traceId Trace ID * @param spanId Span ID * @param spanProcessor Span Processor + * @param attributes attributes */ - public MockSpan(String spanName, Span parentSpan, String traceId, String spanId, SpanProcessor spanProcessor) { + public MockSpan(String spanName, Span parentSpan, String traceId, String spanId, SpanProcessor spanProcessor, Attributes attributes) { super(spanName, parentSpan); this.spanProcessor = spanProcessor; this.metadata = new HashMap<>(); this.traceId = traceId; this.spanId = spanId; this.startTime = System.nanoTime(); + if (attributes != null) { + this.metadata.putAll(attributes.getAttributesMap()); + } } @Override @@ -160,4 +184,13 @@ private static String generateTraceId() { } } + + /** + * Returns attribute. + * @param key key + * @return value + */ + public Object getAttribute(String key) { + return metadata.get(key); + } } diff --git a/test/telemetry/src/main/java/org/opensearch/test/telemetry/tracing/MockTracingContextPropagator.java b/test/telemetry/src/main/java/org/opensearch/test/telemetry/tracing/MockTracingContextPropagator.java index 7e3f5a9031100..a83012548aff0 100644 --- a/test/telemetry/src/main/java/org/opensearch/test/telemetry/tracing/MockTracingContextPropagator.java +++ b/test/telemetry/src/main/java/org/opensearch/test/telemetry/tracing/MockTracingContextPropagator.java @@ -13,6 +13,7 @@ import java.util.function.BiConsumer; import org.opensearch.telemetry.tracing.Span; import org.opensearch.telemetry.tracing.TracingContextPropagator; +import org.opensearch.telemetry.tracing.attributes.Attributes; /** * Mock {@link TracingContextPropagator} to persist the span for internode communication. @@ -38,7 +39,7 @@ public Span extract(Map props) { String[] values = value.split(SEPARATOR); String traceId = values[0]; String spanId = values[1]; - return new MockSpan(null, null, traceId, spanId, spanProcessor); + return new MockSpan(null, null, traceId, spanId, spanProcessor, Attributes.EMPTY); } else { return null; } diff --git a/test/telemetry/src/main/java/org/opensearch/test/telemetry/tracing/MockTracingTelemetry.java b/test/telemetry/src/main/java/org/opensearch/test/telemetry/tracing/MockTracingTelemetry.java index 7013ed6653e63..9b958bbb40f84 100644 --- a/test/telemetry/src/main/java/org/opensearch/test/telemetry/tracing/MockTracingTelemetry.java +++ b/test/telemetry/src/main/java/org/opensearch/test/telemetry/tracing/MockTracingTelemetry.java @@ -11,6 +11,7 @@ import org.opensearch.telemetry.tracing.Span; import org.opensearch.telemetry.tracing.TracingContextPropagator; import org.opensearch.telemetry.tracing.TracingTelemetry; +import org.opensearch.telemetry.tracing.attributes.Attributes; import org.opensearch.test.telemetry.tracing.validators.AllSpansAreEndedProperly; import org.opensearch.test.telemetry.tracing.validators.AllSpansHaveUniqueId; @@ -32,8 +33,8 @@ public MockTracingTelemetry() { } @Override - public Span createSpan(String spanName, Span parentSpan) { - Span span = new MockSpan(spanName, parentSpan, spanProcessor); + public Span createSpan(String spanName, Span parentSpan, Attributes attributes) { + Span span = new MockSpan(spanName, parentSpan, spanProcessor, attributes); spanProcessor.onStart(span); return span; }