Skip to content

Commit 2376dc9

Browse files
feat: propagate custom attributes through non-sdk traces (#71)
Co-authored-by: Jean Scherf <jean.scherf@sap.com>
1 parent 88b0043 commit 2376dc9

6 files changed

Lines changed: 210 additions & 11 deletions

File tree

.github/workflows/check-version-bump.yaml

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -23,15 +23,21 @@ jobs:
2323
CHANGED_FILES=$(git diff --name-only "$BASE_SHA" "$HEAD_SHA")
2424
2525
if echo "$CHANGED_FILES" | grep -q "^src/"; then
26-
VERSION_BUMPED=$(git diff -U0 "$BASE_SHA" "$HEAD_SHA" -- pyproject.toml \
27-
| grep "^+version = ")
26+
BASE_VERSION=$(git show "$BASE_SHA:pyproject.toml" | grep "^version = " | cut -d'"' -f2)
27+
HEAD_VERSION=$(git show "$HEAD_SHA:pyproject.toml" | grep "^version = " | cut -d'"' -f2)
2828
29-
if [ -z "$VERSION_BUMPED" ]; then
30-
echo "ERROR: Files under src/ were modified but the version in pyproject.toml was not bumped."
29+
if [ "$BASE_VERSION" = "$HEAD_VERSION" ]; then
30+
echo "ERROR: Files under src/ were modified but the version in pyproject.toml was not bumped (still $HEAD_VERSION)."
3131
exit 1
32-
else
33-
echo "Version bump detected: $VERSION_BUMPED"
3432
fi
33+
34+
HIGHER=$(printf '%s\n' "$BASE_VERSION" "$HEAD_VERSION" | sort -V | tail -1)
35+
if [ "$HIGHER" != "$HEAD_VERSION" ]; then
36+
echo "ERROR: Version regression detected. Base is $BASE_VERSION but PR has $HEAD_VERSION."
37+
exit 1
38+
fi
39+
40+
echo "Version bump OK: $BASE_VERSION → $HEAD_VERSION"
3541
else
3642
echo "No src/ changes. Version bump not required."
3743
fi

pyproject.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
[project]
22
name = "sap-cloud-sdk"
3-
version = "0.14.0"
3+
version = "0.14.1"
44
description = "SAP Cloud SDK for Python"
55
readme = "README.md"
66
license = "Apache-2.0"

src/sap_cloud_sdk/core/telemetry/auto_instrument.py

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,9 @@
3030
GenAIAttributeTransformer,
3131
)
3232
from sap_cloud_sdk.core.telemetry.metrics_decorator import record_metrics
33+
from sap_cloud_sdk.core.telemetry.propagated_attributes_processor import (
34+
PropagatedAttributesSpanProcessor,
35+
)
3336

3437
logger = logging.getLogger(__name__)
3538

@@ -116,6 +119,11 @@ def _set_baggage_processor():
116119
provider.add_span_processor(BaggageSpanProcessor(ALLOW_ALL_BAGGAGE_KEYS))
117120
logger.info("Registered BaggageSpanProcessor for extension attribute propagation")
118121

122+
provider.add_span_processor(PropagatedAttributesSpanProcessor())
123+
logger.info(
124+
"Registered PropagatedAttributesSpanProcessor for ContextVar attribute propagation"
125+
)
126+
119127

120128
def _register_middleware_processors(middlewares: list[TelemetryMiddleware]) -> None:
121129
from sap_cloud_sdk.core.telemetry.middleware.span_processor import (
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
"""SpanProcessor that injects ContextVar-propagated attributes into every span at start time."""
2+
3+
import logging
4+
from typing import Optional
5+
6+
from opentelemetry.context import Context
7+
from opentelemetry.sdk.trace import SpanProcessor, ReadableSpan
8+
from opentelemetry.trace import Span
9+
10+
from sap_cloud_sdk.core.telemetry.telemetry import _propagated_attrs_var
11+
12+
logger = logging.getLogger(__name__)
13+
14+
15+
class PropagatedAttributesSpanProcessor(SpanProcessor):
16+
"""Injects ContextVar-propagated attributes into every span at start time.
17+
18+
Gives SDK context overlay attributes (set via propagate=True) visibility on
19+
framework-generated child spans (e.g. LangChain, LiteLLM spans created by
20+
Traceloop auto-instrumentation) that don't call get_propagated_attributes().
21+
22+
Priority rule: propagated attrs are lowest priority — any attribute already
23+
set on the span at creation time wins.
24+
"""
25+
26+
def on_start(self, span: Span, parent_context: Optional[Context] = None) -> None:
27+
if not span.is_recording():
28+
return
29+
propagated = _propagated_attrs_var.get()
30+
if not propagated:
31+
return
32+
try:
33+
existing = getattr(span, "attributes", None) or {}
34+
for key, value in propagated.items():
35+
if key not in existing:
36+
span.set_attribute(key, value)
37+
except Exception as exc:
38+
logger.debug(
39+
"PropagatedAttributesSpanProcessor: error injecting attributes into span %r: %s",
40+
getattr(span, "name", "<unknown>"),
41+
exc,
42+
)
43+
44+
def on_end(self, span: ReadableSpan) -> None:
45+
pass

tests/core/unit/telemetry/test_auto_instrument.py

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ def mock_traceloop_components():
2121
'console_exporter': stack.enter_context(patch('sap_cloud_sdk.core.telemetry.auto_instrument.ConsoleSpanExporter')),
2222
'transformer': stack.enter_context(patch('sap_cloud_sdk.core.telemetry.auto_instrument.GenAIAttributeTransformer')),
2323
'baggage_processor': stack.enter_context(patch('sap_cloud_sdk.core.telemetry.auto_instrument.BaggageSpanProcessor')),
24+
'propagated_processor': stack.enter_context(patch('sap_cloud_sdk.core.telemetry.auto_instrument.PropagatedAttributesSpanProcessor')),
2425
'get_tracer_provider': stack.enter_context(patch('sap_cloud_sdk.core.telemetry.auto_instrument.trace.get_tracer_provider', return_value=create_autospec(SDKTracerProvider))),
2526
'create_resource': stack.enter_context(patch('sap_cloud_sdk.core.telemetry.auto_instrument.create_resource_attributes_from_env')),
2627
'get_app_name': stack.enter_context(patch('sap_cloud_sdk.core.telemetry.auto_instrument._get_app_name')),
@@ -232,7 +233,20 @@ def test_auto_instrument_passes_baggage_span_processor(self, mock_traceloop_comp
232233
auto_instrument()
233234

234235
mock_traceloop_components['baggage_processor'].assert_called_once()
235-
mock_traceloop_components['get_tracer_provider'].return_value.add_span_processor.assert_called_once_with(mock_processor_instance)
236+
mock_traceloop_components['get_tracer_provider'].return_value.add_span_processor.assert_any_call(mock_processor_instance)
237+
238+
def test_auto_instrument_registers_propagated_attributes_processor(self, mock_traceloop_components):
239+
"""Test that auto_instrument registers a PropagatedAttributesSpanProcessor on the tracer provider."""
240+
mock_traceloop_components['get_app_name'].return_value = 'test-app'
241+
mock_traceloop_components['create_resource'].return_value = {}
242+
mock_processor_instance = MagicMock()
243+
mock_traceloop_components['propagated_processor'].return_value = mock_processor_instance
244+
245+
with patch.dict('os.environ', {'OTEL_EXPORTER_OTLP_ENDPOINT': 'http://localhost:4317'}, clear=True):
246+
auto_instrument()
247+
248+
mock_traceloop_components['propagated_processor'].assert_called_once()
249+
mock_traceloop_components['get_tracer_provider'].return_value.add_span_processor.assert_any_call(mock_processor_instance)
236250

237251
def test_auto_instrument_merges_resource_when_wrapper_installed(self, mock_traceloop_components):
238252
"""When an OTel auto-instrumentation wrapper (e.g. an OpenTelemetry-Operator
@@ -389,6 +403,5 @@ def test_baggage_and_middleware_processors_both_added(self, mock_traceloop_compo
389403
with patch.dict('os.environ', {'OTEL_EXPORTER_OTLP_ENDPOINT': 'http://localhost:4317'}, clear=True):
390404
auto_instrument(middlewares=[middleware])
391405

392-
# add_span_processor called twice: once for baggage, once for middleware
393-
assert mock_traceloop_components['get_tracer_provider'].return_value.add_span_processor.call_count == 2
394-
406+
# add_span_processor called 3 times: baggage, propagated attributes, middleware
407+
assert mock_traceloop_components['get_tracer_provider'].return_value.add_span_processor.call_count == 3
Lines changed: 127 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,127 @@
1+
"""Tests for PropagatedAttributesSpanProcessor."""
2+
3+
import threading
4+
from unittest.mock import MagicMock
5+
6+
import pytest
7+
8+
from sap_cloud_sdk.core.telemetry.propagated_attributes_processor import (
9+
PropagatedAttributesSpanProcessor,
10+
)
11+
from sap_cloud_sdk.core.telemetry.telemetry import _propagated_attrs_var
12+
13+
14+
@pytest.fixture(autouse=True)
15+
def reset_propagated_attrs():
16+
"""Ensure _propagated_attrs_var is empty before and after each test."""
17+
token = _propagated_attrs_var.set({})
18+
yield
19+
_propagated_attrs_var.reset(token)
20+
21+
22+
def _make_recording_span(existing_attrs=None):
23+
span = MagicMock()
24+
span.is_recording.return_value = True
25+
span.attributes = existing_attrs or {}
26+
span.name = "test-span"
27+
return span
28+
29+
30+
class TestPropagatedAttributesSpanProcessor:
31+
def setup_method(self):
32+
self.processor = PropagatedAttributesSpanProcessor()
33+
34+
def test_injects_propagated_attrs_into_span_with_no_existing_attrs(self):
35+
_propagated_attrs_var.set({"user.id": "u1", "session.id": "s1"})
36+
span = _make_recording_span()
37+
38+
self.processor.on_start(span, None)
39+
40+
span.set_attribute.assert_any_call("user.id", "u1")
41+
span.set_attribute.assert_any_call("session.id", "s1")
42+
assert span.set_attribute.call_count == 2
43+
44+
def test_does_not_override_attrs_already_present_on_span(self):
45+
_propagated_attrs_var.set(
46+
{"gen_ai.request.model": "override", "custom.key": "val"}
47+
)
48+
span = _make_recording_span(existing_attrs={"gen_ai.request.model": "gpt-4"})
49+
50+
self.processor.on_start(span, None)
51+
52+
span.set_attribute.assert_called_once_with("custom.key", "val")
53+
54+
def test_does_nothing_when_no_propagated_attrs(self):
55+
# _propagated_attrs_var is {} by default (from autouse fixture)
56+
span = _make_recording_span()
57+
58+
self.processor.on_start(span, None)
59+
60+
span.set_attribute.assert_not_called()
61+
62+
def test_does_nothing_for_non_recording_span(self):
63+
_propagated_attrs_var.set({"key": "val"})
64+
span = MagicMock()
65+
span.is_recording.return_value = False
66+
67+
self.processor.on_start(span, None)
68+
69+
span.set_attribute.assert_not_called()
70+
71+
def test_swallows_exception_from_set_attribute(self):
72+
_propagated_attrs_var.set({"key": "val"})
73+
span = _make_recording_span()
74+
span.set_attribute.side_effect = RuntimeError("boom")
75+
76+
# Must not raise
77+
self.processor.on_start(span, None)
78+
79+
def test_injects_all_attrs_when_all_absent(self):
80+
_propagated_attrs_var.set({"a": 1, "b": 2, "c": 3})
81+
span = _make_recording_span()
82+
83+
self.processor.on_start(span, None)
84+
85+
assert span.set_attribute.call_count == 3
86+
span.set_attribute.assert_any_call("a", 1)
87+
span.set_attribute.assert_any_call("b", 2)
88+
span.set_attribute.assert_any_call("c", 3)
89+
90+
def test_partial_override_only_missing_keys_added(self):
91+
_propagated_attrs_var.set({"existing": "old", "new_key": "new_val"})
92+
span = _make_recording_span(existing_attrs={"existing": "current"})
93+
94+
self.processor.on_start(span, None)
95+
96+
span.set_attribute.assert_called_once_with("new_key", "new_val")
97+
98+
def test_on_end_is_noop(self):
99+
readable_span = MagicMock()
100+
# Must not raise and must not interact with span
101+
self.processor.on_end(readable_span)
102+
readable_span.assert_not_called()
103+
104+
def test_context_var_isolation_across_contexts(self):
105+
"""Propagated attrs in one context don't bleed into a separate context."""
106+
_propagated_attrs_var.set({"main": "yes"})
107+
108+
results = []
109+
110+
def run_in_fresh_context():
111+
# This thread starts with a copy of the parent context,
112+
# but we reset the ContextVar to empty to simulate isolation.
113+
token = _propagated_attrs_var.set({})
114+
try:
115+
span = _make_recording_span()
116+
self.processor.on_start(span, None)
117+
results.append(span.set_attribute.call_count)
118+
finally:
119+
_propagated_attrs_var.reset(token)
120+
121+
t = threading.Thread(target=run_in_fresh_context)
122+
t.start()
123+
t.join()
124+
125+
assert results == [0], (
126+
"No attrs should be injected in the isolated thread context"
127+
)

0 commit comments

Comments
 (0)