Skip to content

Commit 15aabe4

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 15aabe4

File tree

3 files changed

+271
-74
lines changed

3 files changed

+271
-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+
spans[span_data.span_id] = span
218223

219-
spans[span_data.span_id] = span
224+
span.end(end_time=end_time_ns)
Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,137 @@
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+
import time
17+
from typing import List
18+
19+
import pytest
20+
from opentelemetry import trace
21+
from opentelemetry.sdk.trace import TracerProvider
22+
from opentelemetry.sdk.trace.export import SimpleSpanProcessor
23+
24+
from nemoguardrails.eval.models import Span
25+
from nemoguardrails.tracing import InteractionLog
26+
from nemoguardrails.tracing.adapters.opentelemetry import OpenTelemetryAdapter
27+
28+
29+
class InMemorySpanExporter:
30+
"""Simple in-memory span exporter to capture spans for testing."""
31+
32+
def __init__(self):
33+
self.spans: List = []
34+
35+
def export(self, spans):
36+
self.spans.extend(spans)
37+
return 0 # Success
38+
39+
def shutdown(self):
40+
pass
41+
42+
43+
class TestOpenTelemetryTimingBehavior:
44+
"""
45+
Test that verifies OpenTelemetry spans are created with correct timestamps.
46+
47+
This test focuses on the ACTUAL timing behavior, not implementation details.
48+
It will fail with the old broken code (retrospective timing) and pass with
49+
the fixed code (historical timing).
50+
"""
51+
52+
def setup_method(self):
53+
self.exporter = InMemorySpanExporter()
54+
self.tracer_provider = TracerProvider()
55+
self.tracer_provider.add_span_processor(SimpleSpanProcessor(self.exporter))
56+
57+
trace._TRACER_PROVIDER = None
58+
trace.set_tracer_provider(self.tracer_provider)
59+
60+
self.adapter = OpenTelemetryAdapter()
61+
62+
def teardown_method(self):
63+
# Clean up - reset to no-op
64+
trace._TRACER_PROVIDER = None
65+
trace.set_tracer_provider(trace.NoOpTracerProvider())
66+
67+
def test_spans_use_historical_timestamps_not_current_time(self):
68+
"""
69+
Test that spans are created with historical timestamps from span_data,
70+
not with the current time when transform() is called.
71+
72+
This test will:
73+
- FAIL with old broken code (uses current time)
74+
- PASS with fixed code (uses historical time)
75+
"""
76+
historical_start = 1234567890.5 # January 1, 2009
77+
historical_end = 1234567892.0 # 1.5 seconds later
78+
79+
interaction_log = InteractionLog(
80+
id="timing_test",
81+
activated_rails=[],
82+
events=[],
83+
trace=[
84+
Span(
85+
name="historical_operation",
86+
span_id="span_1",
87+
parent_id=None,
88+
start_time=historical_start,
89+
end_time=historical_end,
90+
duration=1.5,
91+
metrics={"test_metric": 42},
92+
)
93+
],
94+
)
95+
96+
current_time_before = time.time()
97+
98+
self.adapter.transform(interaction_log)
99+
100+
current_time_after = time.time()
101+
102+
assert len(self.exporter.spans) == 1
103+
captured_span = self.exporter.spans[0]
104+
105+
actual_start_time = captured_span.start_time / 1_000_000_000
106+
actual_end_time = captured_span.end_time / 1_000_000_000
107+
108+
assert (
109+
abs(actual_start_time - historical_start) < 0.001
110+
), f"Span start time ({actual_start_time}) should match historical time ({historical_start})"
111+
112+
assert (
113+
abs(actual_end_time - historical_end) < 0.001
114+
), f"Span end time ({actual_end_time}) should match historical time ({historical_end})"
115+
116+
time_diff_start = abs(actual_start_time - current_time_before)
117+
time_diff_end = abs(actual_end_time - current_time_after)
118+
119+
assert time_diff_start > 1000000, (
120+
f"Span start time should be very different from current time. "
121+
f"Difference: {time_diff_start} seconds. This suggests the old bug is present."
122+
)
123+
124+
assert time_diff_end > 1000000, (
125+
f"Span end time should be very different from current time. "
126+
f"Difference: {time_diff_end} seconds. This suggests the old bug is present."
127+
)
128+
actual_duration = actual_end_time - actual_start_time
129+
expected_duration = historical_end - historical_start
130+
assert (
131+
abs(actual_duration - expected_duration) < 0.001
132+
), f"Span duration should be {expected_duration}s, got {actual_duration}s"
133+
134+
assert captured_span.name == "historical_operation"
135+
assert captured_span.attributes.get("test_metric") == 42
136+
assert captured_span.attributes.get("span_id") == "span_1"
137+
assert captured_span.attributes.get("trace_id") == "timing_test"

0 commit comments

Comments
 (0)