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

Enable tracing of Cassandra queries #1038

Merged
merged 4 commits into from
Sep 5, 2018
Merged

Conversation

yurishkuro
Copy link
Member

@yurishkuro yurishkuro commented Sep 1, 2018

Which problem is this PR solving?

Sometimes searching for spans takes a while, it is interesting to see where the time is spent, since the search in Cassandra is multi-step process.

Short description of the changes

  • modify SpanReader API to accept Context in all methods
  • create spans inside Cassandra span reader

Example:

image

@ghost ghost assigned yurishkuro Sep 1, 2018
@ghost ghost added the review label Sep 1, 2018
@yurishkuro yurishkuro changed the title Enable tracing of Cassandra queries WIP: Enable tracing of Cassandra queries Sep 1, 2018
@codecov
Copy link

codecov bot commented Sep 1, 2018

Codecov Report

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

Impacted file tree graph

@@          Coverage Diff           @@
##           master   #1038   +/-   ##
======================================
  Coverage     100%    100%           
======================================
  Files         139     139           
  Lines        6419    6452   +33     
======================================
+ Hits         6419    6452   +33
Impacted Files Coverage Δ
plugin/storage/memory/memory.go 100% <100%> (ø) ⬆️
plugin/storage/cassandra/spanstore/reader.go 100% <100%> (ø) ⬆️
cmd/query/app/handler.go 100% <100%> (ø) ⬆️
plugin/storage/es/spanstore/reader.go 100% <100%> (ø) ⬆️
storage/spanstore/metrics/decorator.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 e5e0fde...e878994. Read the comment docs.

Copy link
Contributor

@jpkrohling jpkrohling left a comment

Choose a reason for hiding this comment

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

I'd consider spending a few more minutes thinking about the operation names, but having them follow the method then are in is also OK.

LGTM.

@yurishkuro yurishkuro force-pushed the trace-cassandra-queries branch 2 times, most recently from ad2344b to 4ee81bb Compare September 5, 2018 16:06
Yuri Shkuro added 3 commits September 5, 2018 15:08
Signed-off-by: Yuri Shkuro <ys@uber.com>
Signed-off-by: Yuri Shkuro <ys@uber.com>
Signed-off-by: Yuri Shkuro <ys@uber.com>
@yurishkuro yurishkuro changed the title WIP: Enable tracing of Cassandra queries Enable tracing of Cassandra queries Sep 5, 2018
Signed-off-by: Yuri Shkuro <ys@uber.com>
@yurishkuro yurishkuro merged commit 96ce340 into master Sep 5, 2018
@ghost ghost removed the review label Sep 5, 2018
@yurishkuro yurishkuro deleted the trace-cassandra-queries branch September 5, 2018 22:48
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.

3 participants