Skip to content

Commit 0871dd4

Browse files
rapphilocelotl
andauthored
Revert "update awslambda to use _X_AMZN_TRACE_ID as a Span Link" (#1911)
Co-authored-by: Diego Hurtado <[email protected]>
1 parent 9627f74 commit 0871dd4

File tree

4 files changed

+114
-104
lines changed

4 files changed

+114
-104
lines changed

CHANGELOG.md

-3
Original file line numberDiff line numberDiff line change
@@ -162,9 +162,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
162162
([#1592](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1592))
163163
- `opentelemetry-instrumentation-django` Allow explicit `excluded_urls` configuration through `instrument()`
164164
([#1618](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1618))
165-
- `opentelemetry-instrumentation-aws-lambda` Use env var `_X_AMZN_TRACE_ID` as a
166-
Span Link instead of parent
167-
([#1657](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1657))
168165

169166
### Fixed
170167

instrumentation/opentelemetry-instrumentation-aws-lambda/src/opentelemetry/instrumentation/aws_lambda/__init__.py

+45-35
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ def custom_event_context_extractor(lambda_event):
7171
import os
7272
import time
7373
from importlib import import_module
74-
from typing import Any, Callable, Collection, Optional, Sequence
74+
from typing import Any, Callable, Collection
7575
from urllib.parse import urlencode
7676

7777
from wrapt import wrap_function_wrapper
@@ -90,7 +90,6 @@ def custom_event_context_extractor(lambda_event):
9090
from opentelemetry.semconv.resource import ResourceAttributes
9191
from opentelemetry.semconv.trace import SpanAttributes
9292
from opentelemetry.trace import (
93-
Link,
9493
Span,
9594
SpanKind,
9695
TracerProvider,
@@ -107,6 +106,9 @@ def custom_event_context_extractor(lambda_event):
107106
OTEL_INSTRUMENTATION_AWS_LAMBDA_FLUSH_TIMEOUT = (
108107
"OTEL_INSTRUMENTATION_AWS_LAMBDA_FLUSH_TIMEOUT"
109108
)
109+
OTEL_LAMBDA_DISABLE_AWS_CONTEXT_PROPAGATION = (
110+
"OTEL_LAMBDA_DISABLE_AWS_CONTEXT_PROPAGATION"
111+
)
110112

111113

112114
def _default_event_context_extractor(lambda_event: Any) -> Context:
@@ -140,12 +142,14 @@ def _default_event_context_extractor(lambda_event: Any) -> Context:
140142

141143

142144
def _determine_parent_context(
143-
lambda_event: Any, event_context_extractor: Callable[[Any], Context]
145+
lambda_event: Any,
146+
event_context_extractor: Callable[[Any], Context],
147+
disable_aws_context_propagation: bool = False,
144148
) -> Context:
145149
"""Determine the parent context for the current Lambda invocation.
146150
147151
See more:
148-
https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/semantic_conventions/instrumentation/aws-lambda.md
152+
https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/semantic_conventions/instrumentation/aws-lambda.md#determining-the-parent-of-a-span
149153
150154
Args:
151155
lambda_event: user-defined, so it could be anything, but this
@@ -154,11 +158,30 @@ def _determine_parent_context(
154158
Event as input and extracts an OTel Context from it. By default,
155159
the context is extracted from the HTTP headers of an API Gateway
156160
request.
161+
disable_aws_context_propagation: By default, this instrumentation
162+
will try to read the context from the `_X_AMZN_TRACE_ID` environment
163+
variable set by Lambda, set this to `True` to disable this behavior.
157164
Returns:
158165
A Context with configuration found in the carrier.
159166
"""
160167
parent_context = None
161168

169+
if not disable_aws_context_propagation:
170+
xray_env_var = os.environ.get(_X_AMZN_TRACE_ID)
171+
172+
if xray_env_var:
173+
parent_context = AwsXRayPropagator().extract(
174+
{TRACE_HEADER_KEY: xray_env_var}
175+
)
176+
177+
if (
178+
parent_context
179+
and get_current_span(parent_context)
180+
.get_span_context()
181+
.trace_flags.sampled
182+
):
183+
return parent_context
184+
162185
if event_context_extractor:
163186
parent_context = event_context_extractor(lambda_event)
164187
else:
@@ -167,33 +190,6 @@ def _determine_parent_context(
167190
return parent_context
168191

169192

170-
def _determine_links() -> Optional[Sequence[Link]]:
171-
"""Determine if a Link should be added to the Span based on the
172-
environment variable `_X_AMZN_TRACE_ID`.
173-
174-
175-
See more:
176-
https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/semantic_conventions/instrumentation/aws-lambda.md#aws-x-ray-environment-span-link
177-
178-
Returns:
179-
A Link or None
180-
"""
181-
links = None
182-
183-
xray_env_var = os.environ.get(_X_AMZN_TRACE_ID)
184-
185-
if xray_env_var:
186-
env_context = AwsXRayPropagator().extract(
187-
{TRACE_HEADER_KEY: xray_env_var}
188-
)
189-
190-
span_context = get_current_span(env_context).get_span_context()
191-
if span_context.is_valid:
192-
links = [Link(span_context, {"source": "x-ray-env"})]
193-
194-
return links
195-
196-
197193
def _set_api_gateway_v1_proxy_attributes(
198194
lambda_event: Any, span: Span
199195
) -> Span:
@@ -288,6 +284,7 @@ def _instrument(
288284
flush_timeout,
289285
event_context_extractor: Callable[[Any], Context],
290286
tracer_provider: TracerProvider = None,
287+
disable_aws_context_propagation: bool = False,
291288
meter_provider: MeterProvider = None,
292289
):
293290
def _instrumented_lambda_handler_call( # noqa pylint: disable=too-many-branches
@@ -300,11 +297,11 @@ def _instrumented_lambda_handler_call( # noqa pylint: disable=too-many-branches
300297
lambda_event = args[0]
301298

302299
parent_context = _determine_parent_context(
303-
lambda_event, event_context_extractor
300+
lambda_event,
301+
event_context_extractor,
302+
disable_aws_context_propagation,
304303
)
305304

306-
links = _determine_links()
307-
308305
span_kind = None
309306
try:
310307
if lambda_event["Records"][0]["eventSource"] in {
@@ -330,7 +327,6 @@ def _instrumented_lambda_handler_call( # noqa pylint: disable=too-many-branches
330327
name=orig_handler_name,
331328
context=parent_context,
332329
kind=span_kind,
333-
links=links,
334330
) as span:
335331
if span.is_recording():
336332
lambda_context = args[1]
@@ -424,6 +420,9 @@ def _instrument(self, **kwargs):
424420
Event as input and extracts an OTel Context from it. By default,
425421
the context is extracted from the HTTP headers of an API Gateway
426422
request.
423+
``disable_aws_context_propagation``: By default, this instrumentation
424+
will try to read the context from the `_X_AMZN_TRACE_ID` environment
425+
variable set by Lambda, set this to `True` to disable this behavior.
427426
"""
428427
lambda_handler = os.environ.get(ORIG_HANDLER, os.environ.get(_HANDLER))
429428
# pylint: disable=attribute-defined-outside-init
@@ -445,6 +444,16 @@ def _instrument(self, **kwargs):
445444
flush_timeout_env,
446445
)
447446

447+
disable_aws_context_propagation = kwargs.get(
448+
"disable_aws_context_propagation", False
449+
) or os.getenv(
450+
OTEL_LAMBDA_DISABLE_AWS_CONTEXT_PROPAGATION, "False"
451+
).strip().lower() in (
452+
"true",
453+
"1",
454+
"t",
455+
)
456+
448457
_instrument(
449458
self._wrapped_module_name,
450459
self._wrapped_function_name,
@@ -453,6 +462,7 @@ def _instrument(self, **kwargs):
453462
"event_context_extractor", _default_event_context_extractor
454463
),
455464
tracer_provider=kwargs.get("tracer_provider"),
465+
disable_aws_context_propagation=disable_aws_context_propagation,
456466
meter_provider=kwargs.get("meter_provider"),
457467
)
458468

instrumentation/opentelemetry-instrumentation-aws-lambda/tests/test_aws_lambda_instrumentation_manual.py

+68-65
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
_HANDLER,
2828
_X_AMZN_TRACE_ID,
2929
OTEL_INSTRUMENTATION_AWS_LAMBDA_FLUSH_TIMEOUT,
30+
OTEL_LAMBDA_DISABLE_AWS_CONTEXT_PROPAGATION,
3031
AwsLambdaInstrumentor,
3132
)
3233
from opentelemetry.propagate import get_global_textmap
@@ -137,9 +138,7 @@ def test_active_tracing(self):
137138
self.assertEqual(len(spans), 1)
138139
span = spans[0]
139140
self.assertEqual(span.name, os.environ[_HANDLER])
140-
self.assertNotEqual(
141-
span.get_span_context().trace_id, MOCK_XRAY_TRACE_ID
142-
)
141+
self.assertEqual(span.get_span_context().trace_id, MOCK_XRAY_TRACE_ID)
143142
self.assertEqual(span.kind, SpanKind.SERVER)
144143
self.assertSpanHasAttributes(
145144
span,
@@ -150,7 +149,11 @@ def test_active_tracing(self):
150149
)
151150

152151
parent_context = span.parent
153-
self.assertEqual(None, parent_context)
152+
self.assertEqual(
153+
parent_context.trace_id, span.get_span_context().trace_id
154+
)
155+
self.assertEqual(parent_context.span_id, MOCK_XRAY_PARENT_SPAN_ID)
156+
self.assertTrue(parent_context.is_remote)
154157

155158
test_env_patch.stop()
156159

@@ -162,8 +165,11 @@ class TestCase:
162165
context: Dict
163166
expected_traceid: int
164167
expected_parentid: int
168+
xray_traceid: str
165169
expected_state_value: str = None
166170
expected_trace_state_len: int = 0
171+
disable_aws_context_propagation: bool = False
172+
disable_aws_context_propagation_envvar: str = ""
167173

168174
def custom_event_context_extractor(lambda_event):
169175
return get_global_textmap().extract(lambda_event["foo"]["headers"])
@@ -182,9 +188,42 @@ def custom_event_context_extractor(lambda_event):
182188
expected_parentid=MOCK_W3C_PARENT_SPAN_ID,
183189
expected_trace_state_len=3,
184190
expected_state_value=MOCK_W3C_TRACE_STATE_VALUE,
191+
xray_traceid=MOCK_XRAY_TRACE_CONTEXT_NOT_SAMPLED,
192+
),
193+
TestCase(
194+
name="custom_extractor_not_sampled_xray",
195+
custom_extractor=custom_event_context_extractor,
196+
context={
197+
"foo": {
198+
"headers": {
199+
TraceContextTextMapPropagator._TRACEPARENT_HEADER_NAME: MOCK_W3C_TRACE_CONTEXT_SAMPLED,
200+
TraceContextTextMapPropagator._TRACESTATE_HEADER_NAME: f"{MOCK_W3C_TRACE_STATE_KEY}={MOCK_W3C_TRACE_STATE_VALUE},foo=1,bar=2",
201+
}
202+
}
203+
},
204+
expected_traceid=MOCK_W3C_TRACE_ID,
205+
expected_parentid=MOCK_W3C_PARENT_SPAN_ID,
206+
expected_trace_state_len=3,
207+
expected_state_value=MOCK_W3C_TRACE_STATE_VALUE,
208+
xray_traceid=MOCK_XRAY_TRACE_CONTEXT_NOT_SAMPLED,
209+
),
210+
TestCase(
211+
name="custom_extractor_sampled_xray",
212+
custom_extractor=custom_event_context_extractor,
213+
context={
214+
"foo": {
215+
"headers": {
216+
TraceContextTextMapPropagator._TRACEPARENT_HEADER_NAME: MOCK_W3C_TRACE_CONTEXT_SAMPLED,
217+
TraceContextTextMapPropagator._TRACESTATE_HEADER_NAME: f"{MOCK_W3C_TRACE_STATE_KEY}={MOCK_W3C_TRACE_STATE_VALUE},foo=1,bar=2",
218+
}
219+
}
220+
},
221+
expected_traceid=MOCK_XRAY_TRACE_ID,
222+
expected_parentid=MOCK_XRAY_PARENT_SPAN_ID,
223+
xray_traceid=MOCK_XRAY_TRACE_CONTEXT_SAMPLED,
185224
),
186225
TestCase(
187-
name="custom_extractor",
226+
name="custom_extractor_sampled_xray_disable_aws_propagation",
188227
custom_extractor=custom_event_context_extractor,
189228
context={
190229
"foo": {
@@ -194,24 +233,47 @@ def custom_event_context_extractor(lambda_event):
194233
}
195234
}
196235
},
236+
disable_aws_context_propagation=True,
197237
expected_traceid=MOCK_W3C_TRACE_ID,
198238
expected_parentid=MOCK_W3C_PARENT_SPAN_ID,
199239
expected_trace_state_len=3,
200240
expected_state_value=MOCK_W3C_TRACE_STATE_VALUE,
241+
xray_traceid=MOCK_XRAY_TRACE_CONTEXT_SAMPLED,
242+
),
243+
TestCase(
244+
name="no_custom_extractor_xray_disable_aws_propagation_via_env_var",
245+
custom_extractor=None,
246+
context={
247+
"headers": {
248+
TraceContextTextMapPropagator._TRACEPARENT_HEADER_NAME: MOCK_W3C_TRACE_CONTEXT_SAMPLED,
249+
TraceContextTextMapPropagator._TRACESTATE_HEADER_NAME: f"{MOCK_W3C_TRACE_STATE_KEY}={MOCK_W3C_TRACE_STATE_VALUE},foo=1,bar=2",
250+
}
251+
},
252+
disable_aws_context_propagation=False,
253+
disable_aws_context_propagation_envvar="true",
254+
expected_traceid=MOCK_W3C_TRACE_ID,
255+
expected_parentid=MOCK_W3C_PARENT_SPAN_ID,
256+
expected_trace_state_len=3,
257+
expected_state_value=MOCK_W3C_TRACE_STATE_VALUE,
258+
xray_traceid=MOCK_XRAY_TRACE_CONTEXT_SAMPLED,
201259
),
202260
]
203261
for test in tests:
204262
test_env_patch = mock.patch.dict(
205263
"os.environ",
206264
{
207265
**os.environ,
266+
# NOT Active Tracing
267+
_X_AMZN_TRACE_ID: test.xray_traceid,
268+
OTEL_LAMBDA_DISABLE_AWS_CONTEXT_PROPAGATION: test.disable_aws_context_propagation_envvar,
208269
# NOT using the X-Ray Propagator
209270
OTEL_PROPAGATORS: "tracecontext",
210271
},
211272
)
212273
test_env_patch.start()
213274
AwsLambdaInstrumentor().instrument(
214-
event_context_extractor=test.custom_extractor
275+
event_context_extractor=test.custom_extractor,
276+
disable_aws_context_propagation=test.disable_aws_context_propagation,
215277
)
216278
mock_execute_lambda(test.context)
217279
spans = self.memory_exporter.get_finished_spans()
@@ -239,65 +301,6 @@ def custom_event_context_extractor(lambda_event):
239301
AwsLambdaInstrumentor().uninstrument()
240302
test_env_patch.stop()
241303

242-
def test_links_from_lambda_event(self):
243-
@dataclass
244-
class TestCase:
245-
name: str
246-
context: Dict
247-
expected_link_trace_id: int
248-
expected_link_attributes: dict
249-
xray_traceid: str
250-
251-
tests = [
252-
TestCase(
253-
name="valid_xray_trace",
254-
context={},
255-
expected_link_trace_id=MOCK_XRAY_TRACE_ID,
256-
expected_link_attributes={"source": "x-ray-env"},
257-
xray_traceid=MOCK_XRAY_TRACE_CONTEXT_SAMPLED,
258-
),
259-
TestCase(
260-
name="invalid_xray_trace",
261-
context={},
262-
expected_link_trace_id=None,
263-
expected_link_attributes={},
264-
xray_traceid="0",
265-
),
266-
]
267-
for test in tests:
268-
test_env_patch = mock.patch.dict(
269-
"os.environ",
270-
{
271-
**os.environ,
272-
# NOT Active Tracing
273-
_X_AMZN_TRACE_ID: test.xray_traceid,
274-
# NOT using the X-Ray Propagator
275-
OTEL_PROPAGATORS: "tracecontext",
276-
},
277-
)
278-
test_env_patch.start()
279-
AwsLambdaInstrumentor().instrument()
280-
mock_execute_lambda(test.context)
281-
spans = self.memory_exporter.get_finished_spans()
282-
assert spans
283-
self.assertEqual(len(spans), 1)
284-
span = spans[0]
285-
286-
if test.expected_link_trace_id is None:
287-
self.assertEqual(0, len(span.links))
288-
else:
289-
link = span.links[0]
290-
self.assertEqual(
291-
link.context.trace_id, test.expected_link_trace_id
292-
)
293-
self.assertEqual(
294-
link.attributes, test.expected_link_attributes
295-
)
296-
297-
self.memory_exporter.clear()
298-
AwsLambdaInstrumentor().uninstrument()
299-
test_env_patch.stop()
300-
301304
def test_lambda_no_error_with_invalid_flush_timeout(self):
302305
test_env_patch = mock.patch.dict(
303306
"os.environ",

tox.ini

+1-1
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ envlist =
3434
; instrumentation-aiopg intentionally excluded from pypy3
3535

3636
; opentelemetry-instrumentation-aws-lambda
37-
py3{7,8,9,10,11}-test-instrumentation-aws-lambda
37+
py3{7,8,9}-test-instrumentation-aws-lambda
3838

3939
; opentelemetry-instrumentation-botocore
4040
py3{7,8,9,10,11}-test-instrumentation-botocore

0 commit comments

Comments
 (0)