diff --git a/src/instana/span/readable_span.py b/src/instana/span/readable_span.py index 3e95ec67..00c9a83a 100644 --- a/src/instana/span/readable_span.py +++ b/src/instana/span/readable_span.py @@ -1,8 +1,9 @@ # (c) Copyright IBM Corp. 2024 from time import time_ns -from typing import Optional, Sequence, List +from typing import List, Optional, Sequence +from opentelemetry.trace import SpanKind from opentelemetry.trace.status import Status, StatusCode from opentelemetry.util import types @@ -56,6 +57,7 @@ def __init__( events: Sequence[Event] = [], status: Optional[Status] = Status(StatusCode.UNSET), stack: Optional[List] = None, + kind: SpanKind = SpanKind.INTERNAL, ) -> None: self._name = name self._context = context @@ -74,6 +76,7 @@ def __init__( self.synthetic = False if context.synthetic: self.synthetic = True + self._kind = kind @property def name(self) -> str: @@ -110,3 +113,7 @@ def status(self) -> Status: @property def parent_id(self) -> int: return self._parent_id + + @property + def kind(self) -> SpanKind: + return self._kind diff --git a/src/instana/span/registered_span.py b/src/instana/span/registered_span.py index 597c371b..e6f7e60d 100644 --- a/src/instana/span/registered_span.py +++ b/src/instana/span/registered_span.py @@ -24,18 +24,22 @@ def __init__( # pylint: disable=invalid-name super(RegisteredSpan, self).__init__(span, source, **kwargs) self.n = span.name - self.k = SpanKind.SERVER # entry -> Server span represents a synchronous incoming remote call such as an incoming HTTP request - + self.k = span.kind self.data["service"] = service_name + if span.name in ENTRY_SPANS: - # entry + # Entry spans - Server span represents a synchronous incoming remote call such as an incoming HTTP request. + self.k = SpanKind.SERVER self._populate_entry_span_data(span) self._populate_extra_span_attributes(span) elif span.name in EXIT_SPANS: - self.k = SpanKind.CLIENT # exit -> Client span represents a synchronous outgoing remote call such as an outgoing HTTP request or database call + # Exit spans - Client span represents a synchronous outgoing remote call such as an outgoing HTTP request + # or a database call. + self.k = SpanKind.CLIENT self._populate_exit_span_data(span) elif span.name in LOCAL_SPANS: - self.k = SpanKind.INTERNAL # intermediate -> Internal span represents an internal operation within an application + # Intermediate or SDK spans - Internal span represents an internal operation within an application. + self.k = SpanKind.INTERNAL self._populate_local_span_data(span) if "rabbitmq" in self.data and self.data["rabbitmq"]["sort"] == "publish": diff --git a/src/instana/span/span.py b/src/instana/span/span.py index ed853fdb..49dbe859 100644 --- a/src/instana/span/span.py +++ b/src/instana/span/span.py @@ -27,6 +27,7 @@ INVALID_SPAN_ID, INVALID_TRACE_ID, Span, + SpanKind, ) from opentelemetry.trace.span import NonRecordingSpan from opentelemetry.trace.status import Status, StatusCode @@ -52,6 +53,7 @@ def __init__( attributes: types.Attributes = {}, events: Sequence[Event] = [], status: Optional[Status] = Status(StatusCode.UNSET), + kind: SpanKind = SpanKind.INTERNAL, ) -> None: super().__init__( name=name, @@ -62,7 +64,7 @@ def __init__( attributes=attributes, events=events, status=status, - # kind=kind, + kind=kind, ) self._span_processor = span_processor self._lock = Lock() @@ -190,7 +192,7 @@ def _readable_span(self) -> ReadableSpan: events=self.events, status=self.status, stack=self.stack, - # kind=self.kind, + kind=self.kind, ) def end(self, end_time: Optional[int] = None) -> None: diff --git a/src/instana/tracer.py b/src/instana/tracer.py index 795f0d53..b5b9e2df 100644 --- a/src/instana/tracer.py +++ b/src/instana/tracer.py @@ -131,6 +131,7 @@ def start_span( parent_id=(None if parent_context is None else parent_context.span_id), start_time=(time.time_ns() if start_time is None else start_time), attributes=attributes, + kind=kind, # events: Sequence[Event] = None, ) diff --git a/tests/collector/test_utils.py b/tests/collector/test_utils.py index f6eba0ff..ec2d7f15 100644 --- a/tests/collector/test_utils.py +++ b/tests/collector/test_utils.py @@ -39,13 +39,13 @@ def test_format_span(self, context: Context) -> None: formatted_spans = format_span(span_list) assert len(formatted_spans) == 2 assert formatted_spans[0].t == expected_trace_id - assert formatted_spans[0].k == 1 + assert formatted_spans[0].k == 3 assert formatted_spans[0].s == expected_span_id assert formatted_spans[0].n == "span1" assert formatted_spans[1].t == expected_trace_id assert formatted_spans[1].p == formatted_spans[0].s - assert formatted_spans[1].k == 1 + assert formatted_spans[1].k == 3 assert formatted_spans[1].s != formatted_spans[0].s assert formatted_spans[1].n == "span2" assert formatted_spans[1].n == "span2" diff --git a/tests/span/test_base_span.py b/tests/span/test_base_span.py index 2e0fbf43..0551a7d5 100644 --- a/tests/span/test_base_span.py +++ b/tests/span/test_base_span.py @@ -1,9 +1,8 @@ # (c) Copyright IBM Corp. 2024 -from typing import Generator from unittest.mock import Mock, patch -import pytest +from opentelemetry.trace import SpanKind from instana.recorder import StanRecorder from instana.span.base_span import BaseSpan @@ -173,3 +172,82 @@ def test_convert_attribute_value_exception( converted_value = base_span._convert_attribute_value(mock) assert not converted_value + + +def test_basespan_does_not_store_kind( + span_context: SpanContext, + span_processor: StanRecorder, +) -> None: + """Test that BaseSpan does not directly store or interfere with kind parameter.""" + span = InstanaSpan( + "test-base-span", span_context, span_processor, kind=SpanKind.CLIENT + ) + base_span = BaseSpan(span, None) + + # BaseSpan should not have a kind attribute + assert not hasattr(base_span, "k") + assert not hasattr(base_span, "kind") + + # But the original span should still have it + assert span.kind == SpanKind.CLIENT + + +def test_basespan_with_different_span_kinds( + span_context: SpanContext, + span_processor: StanRecorder, +) -> None: + """Test that BaseSpan works correctly with spans of different kinds.""" + kinds = [ + SpanKind.INTERNAL, + SpanKind.SERVER, + SpanKind.CLIENT, + SpanKind.PRODUCER, + SpanKind.CONSUMER, + ] + + for kind in kinds: + span = InstanaSpan( + f"test-span-{kind.name}", span_context, span_processor, kind=kind + ) + base_span = BaseSpan(span, None) + + # Verify BaseSpan is created successfully regardless of kind + assert base_span.t == span_context.trace_id + assert base_span.s == span_context.span_id + + # Verify original span retains its kind + assert span.kind == kind + + +def test_basespan_kind_inheritance_to_registered_span( + span_context: SpanContext, + span_processor: StanRecorder, +) -> None: + """Test that kind is properly inherited by RegisteredSpan through BaseSpan.""" + from instana.span.registered_span import RegisteredSpan + + span = InstanaSpan("wsgi", span_context, span_processor, kind=SpanKind.SERVER) + reg_span = RegisteredSpan(span, None, "test-service") + + # RegisteredSpan should have k field set correctly + assert reg_span.k == SpanKind.SERVER + # Verify it inherits BaseSpan attributes + assert reg_span.t == span_context.trace_id + assert reg_span.s == span_context.span_id + + +def test_basespan_kind_inheritance_to_sdk_span( + span_context: SpanContext, + span_processor: StanRecorder, +) -> None: + """Test that kind is accessible by SDKSpan through BaseSpan.""" + from instana.span.sdk_span import SDKSpan + + span = InstanaSpan("test-sdk", span_context, span_processor, kind=SpanKind.PRODUCER) + sdk_span = SDKSpan(span, None, "test-service") + + # SDKSpan should be able to access span.kind + assert span.kind == SpanKind.PRODUCER + # Verify it inherits BaseSpan attributes + assert sdk_span.t == span_context.trace_id + assert sdk_span.s == span_context.span_id diff --git a/tests/span/test_readable_span.py b/tests/span/test_readable_span.py index ca506227..f71759e5 100644 --- a/tests/span/test_readable_span.py +++ b/tests/span/test_readable_span.py @@ -4,6 +4,7 @@ from typing import Generator import pytest +from opentelemetry.trace import SpanKind from opentelemetry.trace.status import Status, StatusCode from instana.span.readable_span import Event, ReadableSpan @@ -26,6 +27,8 @@ def test_readablespan( ) -> None: span_name = "test-span" timestamp = time.time_ns() + time.sleep(0.01) + self.span = ReadableSpan(span_name, span_context) assert self.span is not None @@ -45,10 +48,10 @@ def test_readablespan( assert not self.span.events assert not self.span.parent_id assert not self.span.duration - assert self.span.status - assert not self.span.stack assert self.span.synthetic is False + assert self.span.status + assert self.span.kind == SpanKind.INTERNAL def test_readablespan_with_params( self, @@ -63,6 +66,8 @@ def test_readablespan_with_params( events = [Event(event_name, attributes, start_time)] status = Status(StatusCode.OK) stack = ["span-1", "span-2"] + kind = SpanKind.CLIENT + self.span = ReadableSpan( span_name, span_context, @@ -73,6 +78,7 @@ def test_readablespan_with_params( events, status, stack, + kind, ) assert self.span.name == span_name @@ -84,3 +90,51 @@ def test_readablespan_with_params( assert self.span.status == status assert self.span.duration == end_time - start_time assert self.span.stack == stack + assert self.span.kind == kind + assert self.span.kind != SpanKind.INTERNAL + + @pytest.mark.parametrize( + "kind", + [ + SpanKind.INTERNAL, + SpanKind.SERVER, + SpanKind.CLIENT, + SpanKind.PRODUCER, + SpanKind.CONSUMER, + ], + ) + def test_readablespan_all_kind_values( + self, + span_context: SpanContext, + kind: SpanKind, + ) -> None: + """Test that ReadableSpan correctly stores all SpanKind enum values.""" + span_name = "test-span-kind" + self.span = ReadableSpan(span_name, span_context, kind=kind) + + assert self.span.kind == kind + assert isinstance(self.span.kind, SpanKind) + + def test_readablespan_kind_default( + self, + span_context: SpanContext, + ) -> None: + """Test that ReadableSpan defaults to SpanKind.INTERNAL when kind is not specified.""" + span_name = "test-span-default-kind" + self.span = ReadableSpan(span_name, span_context) + + assert self.span.kind == SpanKind.INTERNAL + + def test_readablespan_kind_property_readonly( + self, + span_context: SpanContext, + ) -> None: + """Test that kind property is read-only and cannot be modified after creation.""" + span_name = "test-span-readonly" + self.span = ReadableSpan(span_name, span_context, kind=SpanKind.SERVER) + + assert self.span.kind == SpanKind.SERVER + + # Verify kind is stored in private attribute and property returns it + assert hasattr(self.span, "_kind") + assert self.span._kind == SpanKind.SERVER diff --git a/tests/span/test_registered_span.py b/tests/span/test_registered_span.py index d707dafa..d54100a4 100644 --- a/tests/span/test_registered_span.py +++ b/tests/span/test_registered_span.py @@ -495,3 +495,70 @@ def test_collect_kafka_attributes( assert excepted_result["kafka.service"] == reg_span.data["kafka"]["service"] assert excepted_result["kafka.access"] == reg_span.data["kafka"]["access"] + + @pytest.mark.parametrize( + "span_name, expected_kind", + [ + ("wsgi", SpanKind.SERVER), + ("django", SpanKind.SERVER), + ("rabbitmq", SpanKind.SERVER), + ("redis", SpanKind.CLIENT), + ("mysql", SpanKind.CLIENT), + ("mongodb", SpanKind.CLIENT), + ("urllib", SpanKind.CLIENT), + ("asyncio", SpanKind.INTERNAL), + ("render", SpanKind.INTERNAL), + ("gcps-producer", SpanKind.CLIENT), + ("gcps-consumer", SpanKind.SERVER), + ("kafka-producer", SpanKind.CLIENT), + ("kafka-consumer", SpanKind.SERVER), + ], + ) + def test_registered_span_kind_from_instana_span( + self, + span_context: SpanContext, + span_processor: StanRecorder, + span_name: str, + expected_kind: SpanKind, + ) -> None: + """Test that RegisteredSpan uses kind from InstanaSpan when provided.""" + service_name = "test-service" + + # Create InstanaSpan with explicit kind + self.span = InstanaSpan( + span_name, span_context, span_processor, kind=expected_kind + ) + reg_span = RegisteredSpan(self.span, None, service_name) + + # Verify RegisteredSpan has correct kind for ENTRY span + assert reg_span.k == expected_kind + + # Verify name unification + if "gcps" in span_name: + assert reg_span.n == "gcps" + elif "kafka" in span_name: + assert reg_span.n == "kafka" + else: + assert reg_span.n == span_name + + def test_registered_span_rabbitmq_publish_override( + self, + span_context: SpanContext, + span_processor: StanRecorder, + ) -> None: + """Test that rabbitmq with sort=publish overrides to SpanKind.CLIENT.""" + span_name = "rabbitmq" + attributes = {"sort": "publish"} + + self.span = InstanaSpan( + span_name, + span_context, + span_processor, + kind=SpanKind.SERVER, + attributes=attributes, + ) + reg_span = RegisteredSpan(self.span, None, "test-service") + + # Should be overridden to CLIENT for publish operation + assert reg_span.k == SpanKind.CLIENT + assert reg_span.data["rabbitmq"]["sort"] == "publish" diff --git a/tests/test_tracer.py b/tests/test_tracer.py index 55b94be7..4e18a7d4 100644 --- a/tests/test_tracer.py +++ b/tests/test_tracer.py @@ -2,13 +2,18 @@ import pytest from opentelemetry.context.context import Context +from opentelemetry.trace import SpanKind from opentelemetry.trace.span import _SPAN_ID_MAX_VALUE from instana.agent.host import HostAgent from instana.recorder import StanRecorder from instana.sampling import InstanaSampler -from instana.span.span import (INVALID_SPAN, INVALID_SPAN_ID, InstanaSpan, - get_current_span) +from instana.span.span import ( + INVALID_SPAN, + INVALID_SPAN_ID, + InstanaSpan, + get_current_span, +) from instana.span_context import SpanContext from instana.tracer import InstanaTracer, InstanaTracerProvider @@ -137,3 +142,117 @@ def test_tracer_create_span_context_root( assert new_span_context.trace_id <= _SPAN_ID_MAX_VALUE assert new_span_context.trace_id == new_span_context.span_id + + +@pytest.mark.parametrize( + "kind", + [ + SpanKind.INTERNAL, + SpanKind.SERVER, + SpanKind.CLIENT, + SpanKind.PRODUCER, + SpanKind.CONSUMER, + ], +) +def test_tracer_start_span_with_kind( + tracer_provider: InstanaTracerProvider, context: Context, kind: SpanKind +) -> None: + """Test that tracer.start_span correctly passes kind parameter to InstanaSpan.""" + span_name = f"test-span-{kind.name.lower()}" + tracer = InstanaTracer( + tracer_provider.sampler, + tracer_provider._span_processor, + tracer_provider._exporter, + tracer_provider._propagators, + ) + span = tracer.start_span(name=span_name, context=context, kind=kind) + + assert span + assert isinstance(span, InstanaSpan) + assert span.name == span_name + assert span.kind == kind + + +def test_tracer_start_span_default_kind( + tracer_provider: InstanaTracerProvider, context: Context +) -> None: + """Test that tracer.start_span defaults to SpanKind.INTERNAL when kind is not specified.""" + span_name = "test-span-default-kind" + tracer = InstanaTracer( + tracer_provider.sampler, + tracer_provider._span_processor, + tracer_provider._exporter, + tracer_provider._propagators, + ) + span = tracer.start_span(name=span_name, context=context) + + assert span + assert isinstance(span, InstanaSpan) + assert span.kind == SpanKind.INTERNAL + + +def test_tracer_start_as_current_span_with_kind( + tracer_provider: InstanaTracerProvider, +) -> None: + """Test that tracer.start_as_current_span correctly passes kind parameter.""" + span_name = "test-span-context-manager" + tracer = InstanaTracer( + tracer_provider.sampler, + tracer_provider._span_processor, + tracer_provider._exporter, + tracer_provider._propagators, + ) + with tracer.start_as_current_span(name=span_name, kind=SpanKind.SERVER) as span: + assert span is not None + assert isinstance(span, InstanaSpan) + assert span.name == span_name + assert span.kind == SpanKind.SERVER + + +def test_tracer_nested_span_with_different_kinds( + tracer_provider: InstanaTracerProvider, +) -> None: + """Test that nested spans can have different kind values.""" + tracer = InstanaTracer( + tracer_provider.sampler, + tracer_provider._span_processor, + tracer_provider._exporter, + tracer_provider._propagators, + ) + parent_span_name = "parent-server-span" + child_span_name = "child-client-span" + + with tracer.start_as_current_span( + name=parent_span_name, kind=SpanKind.SERVER + ) as pspan: + assert pspan.kind == SpanKind.SERVER + + with tracer.start_as_current_span( + name=child_span_name, kind=SpanKind.CLIENT + ) as cspan: + assert cspan.kind == SpanKind.CLIENT + assert cspan.parent_id == pspan.context.span_id + # Verify kinds are independent + assert pspan.kind == SpanKind.SERVER + assert cspan.kind == SpanKind.CLIENT + + +def test_tracer_kind_propagation_to_readable_span( + tracer_provider: InstanaTracerProvider, context: Context +) -> None: + """Test that kind is properly propagated when span is converted to ReadableSpan.""" + span_name = "test-span-readable" + tracer = InstanaTracer( + tracer_provider.sampler, + tracer_provider._span_processor, + tracer_provider._exporter, + tracer_provider._propagators, + ) + span = tracer.start_span(name=span_name, context=context, kind=SpanKind.PRODUCER) + + assert span.kind == SpanKind.PRODUCER + + # Create readable span (this happens internally when span.end() is called) + readable_span = span._readable_span() + + assert readable_span.kind == SpanKind.PRODUCER