Skip to content
New issue

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

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

Already on GitHub? # to your account

[Storage] Emit spans for elastic storage backend #1128

Merged
merged 4 commits into from
Nov 11, 2018

Conversation

annanay25
Copy link
Member

First draft, aims to fix #1043

Which problem is this PR solving?

  • Context is passed, but spans are not emitted for the ES backend.

Short description of the changes

  • Create spans in calls to ES backend.

@codecov
Copy link

codecov bot commented Oct 19, 2018

Codecov Report

Merging #1128 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff           @@
##           master   #1128   +/-   ##
======================================
  Coverage     100%    100%           
======================================
  Files         144     144           
  Lines        6804    6822   +18     
======================================
+ Hits         6804    6822   +18
Impacted Files Coverage Δ
plugin/storage/es/spanstore/reader.go 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update de8a9ad...31c6a9d. Read the comment docs.

@jpkrohling
Copy link
Contributor

I'm restarting the CI for this PR, but I suspect the failure might actually be related to this change.

@annanay25
Copy link
Member Author

@jpkrohling the CI test has passed, could you review the changes?

@yurishkuro
Copy link
Member

pleas ensure full test coverage

Annanay added 4 commits November 11, 2018 19:37
Signed-off-by: Annanay <annanay.a@media.net>
Signed-off-by: Annanay <annanay.a@media.net>
Signed-off-by: Annanay <annanay.a@media.net>
Signed-off-by: Annanay <annanay.a@media.net>
@annanay25
Copy link
Member Author

Patch updated. @yurishkuro

@yurishkuro yurishkuro merged commit be62816 into jaegertracing:master Nov 11, 2018
@yurishkuro
Copy link
Member

Thank you, @annanay25 !

If you are using Jaeger in your organization, please consider commenting on #207.

@annanay25
Copy link
Member Author

@yurishkuro thanks! I'll look into the survey soon.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enable tracing of ES queries
3 participants