Skip to content

Conversation

@nico34638
Copy link
Contributor

@nico34638 nico34638 commented Dec 16, 2025

Description

Jaeger has a new specification for the GRPC remote storage API. They have removed the old version from release 2.6 onwards

https://github.com/jaegertracing/jaeger/tree/v2.13.0/internal/storage/v2/grpc

How was this PR tested?

I add unit test for grpc endpoint and new helper function. And I tested the features with the latest version of Jaeger, and everything works fine.


if root_only {
let is_root = UserInputQuery {
user_text: "NOT is_root:false".to_string(),
Copy link
Collaborator

@fulmicoton fulmicoton Dec 17, 2025

Choose a reason for hiding this comment

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

why not is_root:true ? Why not a term query? Is this in v1?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

https://github.com/quickwit-oss/quickwit/blob/main/quickwit/quickwit-jaeger/src/lib.rs#L322 same in v1 implementation

I don't think a term query is appropriate because, if I understand correctly, a term query searches for fields where is_root is false, or in cases where there are child spans, the field does not exist

default_operator: BooleanOperand::And,
lenient: true,
};
let mut new_query = BoolQuery::default();
Copy link
Collaborator

Choose a reason for hiding this comment

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

nitpick: BooleanQuery::intersection

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can't find the BoolQuery::intersection method, are you sure it exists?

Copy link
Collaborator

@fulmicoton fulmicoton left a comment

Choose a reason for hiding this comment

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

A couple of nitpicks here and there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants