Skip to content

Commit fc840fe

Browse files
committed
fix(tracing): use actual operation timestamps in OpenTelemetry spans
Fixes OpenTelemetry tracing issue where spans were created with current timestamps during export rather than original timestamps from when operations actually occurred. Changed from start_as_current_span() to start_span() with proper start_time and span.end() with end_time.
1 parent dd1ce5f commit fc840fe

File tree

3 files changed

+274
-74
lines changed

3 files changed

+274
-74
lines changed

nemoguardrails/tracing/adapters/opentelemetry.py

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@
5555

5656
import warnings
5757
from importlib.metadata import version
58-
from typing import TYPE_CHECKING, Optional, Type
58+
from typing import TYPE_CHECKING, Type
5959

6060
if TYPE_CHECKING:
6161
from nemoguardrails.tracing import InteractionLog
@@ -203,17 +203,22 @@ def _create_span(
203203
spans,
204204
trace_id,
205205
):
206-
with self.tracer.start_as_current_span(
206+
start_time_ns = int(span_data.start_time * 1_000_000_000)
207+
end_time_ns = int(span_data.end_time * 1_000_000_000)
208+
209+
span = self.tracer.start_span(
207210
span_data.name,
208211
context=parent_context,
209-
) as span:
210-
for key, value in span_data.metrics.items():
211-
span.set_attribute(key, value)
212+
start_time=start_time_ns,
213+
)
214+
215+
for key, value in span_data.metrics.items():
216+
span.set_attribute(key, value)
217+
218+
span.set_attribute("span_id", span_data.span_id)
219+
span.set_attribute("trace_id", trace_id)
220+
span.set_attribute("duration", span_data.duration)
212221

213-
span.set_attribute("span_id", span_data.span_id)
214-
span.set_attribute("trace_id", trace_id)
215-
span.set_attribute("start_time", span_data.start_time)
216-
span.set_attribute("end_time", span_data.end_time)
217-
span.set_attribute("duration", span_data.duration)
222+
span.end(end_time=end_time_ns)
218223

219-
spans[span_data.span_id] = span
224+
spans[span_data.span_id] = span
Lines changed: 140 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,140 @@
1+
# SPDX-FileCopyrightText: Copyright (c) 2023 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
2+
# SPDX-License-Identifier: Apache-2.0
3+
#
4+
# Licensed under the Apache License, Version 2.0 (the "License");
5+
# you may not use this file except in compliance with the License.
6+
# You may obtain a copy of the License at
7+
#
8+
# http://www.apache.org/licenses/LICENSE-2.0
9+
#
10+
# Unless required by applicable law or agreed to in writing, software
11+
# distributed under the License is distributed on an "AS IS" BASIS,
12+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
# See the License for the specific language governing permissions and
14+
# limitations under the License.
15+
16+
# SPDX-FileCopyrightText: Copyright (c) 2023 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
17+
# SPDX-License-Identifier: Apache-2.0
18+
19+
import time
20+
from typing import List
21+
22+
import pytest
23+
from opentelemetry import trace
24+
from opentelemetry.sdk.trace import TracerProvider
25+
from opentelemetry.sdk.trace.export import SimpleSpanProcessor
26+
27+
from nemoguardrails.eval.models import Span
28+
from nemoguardrails.tracing import InteractionLog
29+
from nemoguardrails.tracing.adapters.opentelemetry import OpenTelemetryAdapter
30+
31+
32+
class InMemorySpanExporter:
33+
"""Simple in-memory span exporter to capture spans for testing."""
34+
35+
def __init__(self):
36+
self.spans: List = []
37+
38+
def export(self, spans):
39+
self.spans.extend(spans)
40+
return 0 # Success
41+
42+
def shutdown(self):
43+
pass
44+
45+
46+
class TestOpenTelemetryTimingBehavior:
47+
"""
48+
Test that verifies OpenTelemetry spans are created with correct timestamps.
49+
50+
This test focuses on the ACTUAL timing behavior, not implementation details.
51+
It will fail with the old broken code (retrospective timing) and pass with
52+
the fixed code (historical timing).
53+
"""
54+
55+
def setup_method(self):
56+
self.exporter = InMemorySpanExporter()
57+
self.tracer_provider = TracerProvider()
58+
self.tracer_provider.add_span_processor(SimpleSpanProcessor(self.exporter))
59+
60+
trace._TRACER_PROVIDER = None
61+
trace.set_tracer_provider(self.tracer_provider)
62+
63+
self.adapter = OpenTelemetryAdapter()
64+
65+
def teardown_method(self):
66+
# Clean up - reset to no-op
67+
trace._TRACER_PROVIDER = None
68+
trace.set_tracer_provider(trace.NoOpTracerProvider())
69+
70+
def test_spans_use_historical_timestamps_not_current_time(self):
71+
"""
72+
Test that spans are created with historical timestamps from span_data,
73+
not with the current time when transform() is called.
74+
75+
This test will:
76+
- FAIL with old broken code (uses current time)
77+
- PASS with fixed code (uses historical time)
78+
"""
79+
historical_start = 1234567890.5 # January 1, 2009
80+
historical_end = 1234567892.0 # 1.5 seconds later
81+
82+
interaction_log = InteractionLog(
83+
id="timing_test",
84+
activated_rails=[],
85+
events=[],
86+
trace=[
87+
Span(
88+
name="historical_operation",
89+
span_id="span_1",
90+
parent_id=None,
91+
start_time=historical_start,
92+
end_time=historical_end,
93+
duration=1.5,
94+
metrics={"test_metric": 42},
95+
)
96+
],
97+
)
98+
99+
current_time_before = time.time()
100+
101+
self.adapter.transform(interaction_log)
102+
103+
current_time_after = time.time()
104+
105+
assert len(self.exporter.spans) == 1
106+
captured_span = self.exporter.spans[0]
107+
108+
actual_start_time = captured_span.start_time / 1_000_000_000
109+
actual_end_time = captured_span.end_time / 1_000_000_000
110+
111+
assert (
112+
abs(actual_start_time - historical_start) < 0.001
113+
), f"Span start time ({actual_start_time}) should match historical time ({historical_start})"
114+
115+
assert (
116+
abs(actual_end_time - historical_end) < 0.001
117+
), f"Span end time ({actual_end_time}) should match historical time ({historical_end})"
118+
119+
time_diff_start = abs(actual_start_time - current_time_before)
120+
time_diff_end = abs(actual_end_time - current_time_after)
121+
122+
assert time_diff_start > 1000000, (
123+
f"Span start time should be very different from current time. "
124+
f"Difference: {time_diff_start} seconds. This suggests the old bug is present."
125+
)
126+
127+
assert time_diff_end > 1000000, (
128+
f"Span end time should be very different from current time. "
129+
f"Difference: {time_diff_end} seconds. This suggests the old bug is present."
130+
)
131+
actual_duration = actual_end_time - actual_start_time
132+
expected_duration = historical_end - historical_start
133+
assert (
134+
abs(actual_duration - expected_duration) < 0.001
135+
), f"Span duration should be {expected_duration}s, got {actual_duration}s"
136+
137+
assert captured_span.name == "historical_operation"
138+
assert captured_span.attributes.get("test_metric") == 42
139+
assert captured_span.attributes.get("span_id") == "span_1"
140+
assert captured_span.attributes.get("trace_id") == "timing_test"

0 commit comments

Comments
 (0)