-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
[feat] Add time window for GetTrace in span store interface #6242
[feat] Add time window for GetTrace in span store interface #6242
Conversation
fcd8538
to
5e2d862
Compare
gw.getTracesAndVerify(t, "/api/v3/traces/"+traceID.String(), traceID) | ||
trace, query := makeTestTrace() | ||
gw.reader.On("GetTrace", matchContext, query).Return(trace, nil).Once() | ||
gw.getTracesAndVerify(t, "/api/v3/traces/"+query.TraceID.String(), query.TraceID) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could introduce a preliminary PR that adds timestamps args to the REST API calls
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
here: #6248
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that PR doesn't seem to compile, can you get it to green state?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here is a PR contains changes of current PR, and the PR above: #6258
5e2d862
to
5610442
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #6242 +/- ##
==========================================
- Coverage 96.12% 96.11% -0.01%
==========================================
Files 360 360
Lines 20433 20479 +46
==========================================
+ Hits 19641 19684 +43
- Misses 605 607 +2
- Partials 187 188 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
289e0a8
to
7c37eee
Compare
fd25a38
to
16da94c
Compare
PR updated, please review, thanks @yurishkuro |
56f361a
to
5edb49b
Compare
for _, input := range inputs { | ||
tsc := newTestServerClient(t) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for _, input := range inputs { | |
tsc := newTestServerClient(t) | |
for _, input := range inputs { | |
t.Run(input.name, func(t *testing.T) { | |
tsc := newTestServerClient(t) | |
... | |
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
Signed-off-by: rim99 <zhangxin@outlook.com>
Signed-off-by: rim99 <zhangxin@outlook.com>
Co-authored-by: Yuri Shkuro <yurishkuro@users.noreply.github.com> Signed-off-by: Zhang Xin <zxin3306@126.com>
Co-authored-by: Yuri Shkuro <yurishkuro@users.noreply.github.com> Signed-off-by: Zhang Xin <zxin3306@126.com>
Co-authored-by: Yuri Shkuro <yurishkuro@users.noreply.github.com> Signed-off-by: Zhang Xin <zxin3306@126.com>
Signed-off-by: rim99 <zhangxin@outlook.com>
Signed-off-by: rim99 <zhangxin@outlook.com>
Signed-off-by: rim99 <zhangxin@outlook.com>
Signed-off-by: rim99 <zhangxin@outlook.com>
Signed-off-by: rim99 <zhangxin@outlook.com>
Signed-off-by: rim99 <zhangxin@outlook.com>
af0dee9
to
491c826
Compare
@rim99 could you please resolve the conflicts? |
…nterface-add-time-window
Resolved. Could you please reivew? |
Signed-off-by: Yuri Shkuro <yurishkuro@users.noreply.github.com>
TODOs added to
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Which problem is this PR solving?
Part of #4150
Description of the changes
Refactor trace get method in span store interface:
How was this change tested?
Checklist
jaeger
:make lint test
jaeger-ui
:yarn lint
andyarn test