Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

overwrite span id when attribute set, query unique traces #155

Merged
merged 1 commit into from
Nov 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion app-server/src/db/trace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -402,6 +402,7 @@ pub async fn get_traces(
query.push(
"
SELECT
DISTINCT ON (start_time, id)
id,
start_time,
end_time,
Expand Down Expand Up @@ -435,7 +436,7 @@ pub async fn get_traces(
add_filters_to_traces_query(&mut query, &filters);

query
.push(" ORDER BY start_time DESC OFFSET ")
.push(" ORDER BY start_time DESC, id OFFSET ")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ORDER BY clause should match the columns in the DISTINCT ON clause to ensure consistent results. Consider ordering by start_time, id instead of start_time DESC, id.

Suggested change
.push(" ORDER BY start_time DESC, id OFFSET ")
.push(" ORDER BY start_time, id OFFSET ")

.push_bind(offset as i64)
.push(" LIMIT ")
.push_bind(limit as i64);
Expand Down
14 changes: 14 additions & 0 deletions app-server/src/traces/spans.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,10 @@ use super::span_attributes::{

const INPUT_ATTRIBUTE_NAME: &str = "lmnr.span.input";
const OUTPUT_ATTRIBUTE_NAME: &str = "lmnr.span.output";
/// If this attribute is set to true, the parent span will be overridden with
/// null. We hackily use this when we wrap a span in a NonRecordingSpan that
/// is not sent to the backend – this is done to overwrite trace IDs for spans.
const OVERRIDE_PARENT_SPAN_ATTRIBUTE_NAME: &str = "lmnr.internal.override_parent_span";

pub struct SpanAttributes {
pub attributes: HashMap<String, Value>,
Expand Down Expand Up @@ -309,6 +313,12 @@ impl Span {
}
}

// Spans with this attribute are wrapped in a NonRecordingSpan that, and we only
// do that when we add a new span to a trace as a root span.
if let Some(Value::Bool(true)) = attributes.get(OVERRIDE_PARENT_SPAN_ATTRIBUTE_NAME) {
span.parent_span_id = None;
}

span
}

Expand Down Expand Up @@ -504,6 +514,10 @@ fn should_keep_attribute(attribute: &str) -> bool {
return false;
}

if attribute == OVERRIDE_PARENT_SPAN_ATTRIBUTE_NAME {
return false;
}

// remove gen_ai.prompt/completion attributes as they are stored in LLM span's input/output
let pattern = Regex::new(r"gen_ai\.(prompt|completion)\.\d+\.(content|role)").unwrap();
return !pattern.is_match(attribute);
Expand Down