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

Make the metrics produced by jaeger query scoped to the query compone… #1074

Merged
merged 1 commit into from
Sep 20, 2018

Conversation

objectiser
Copy link
Contributor

…nt, and generated for all span readers (not just ES)

Signed-off-by: Gary Brown gary@brownuk.com

Which problem is this PR solving?

Two problems:

  1. metrics provided by collector, agent and client are all scoped to the component name - the query metrics were not, so the 'query' component has now been added
  2. metrics were only being generated when the ES storage was used - the metrics decorator now wraps any span reader impl.

Short description of the changes

Moved use of the span reader decorator to the all-in-one and query mains, so it wraps any span reader impl. Added namespace scope 'query' to be consistent with metrics reported for other components.

@codecov
Copy link

codecov bot commented Sep 20, 2018

Codecov Report

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

Impacted file tree graph

@@          Coverage Diff           @@
##           master   #1074   +/-   ##
======================================
  Coverage     100%    100%           
======================================
  Files         140     140           
  Lines        6622    6622           
======================================
  Hits         6622    6622
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 79d10f2...bb11536. Read the comment docs.

@jpkrohling
Copy link
Contributor

LGTM, but would it be possible to capture a sample of what it was before and how it looks now?

@objectiser
Copy link
Contributor Author

From jaeger-query using ES storage:

# HELP jaeger_find_traces_attempts jaeger_find_traces_attempts
# TYPE jaeger_find_traces_attempts counter
jaeger_find_traces_attempts 1
# HELP jaeger_find_traces_errLatency jaeger_find_traces_errLatency
# TYPE jaeger_find_traces_errLatency histogram
jaeger_find_traces_errLatency_bucket{le="0.005"} 0
jaeger_find_traces_errLatency_bucket{le="0.01"} 0
jaeger_find_traces_errLatency_bucket{le="0.025"} 0
jaeger_find_traces_errLatency_bucket{le="0.05"} 0
jaeger_find_traces_errLatency_bucket{le="0.1"} 0
jaeger_find_traces_errLatency_bucket{le="0.25"} 0
jaeger_find_traces_errLatency_bucket{le="0.5"} 0
jaeger_find_traces_errLatency_bucket{le="1"} 0
jaeger_find_traces_errLatency_bucket{le="2.5"} 0
jaeger_find_traces_errLatency_bucket{le="5"} 0
jaeger_find_traces_errLatency_bucket{le="10"} 0
jaeger_find_traces_errLatency_bucket{le="+Inf"} 0
jaeger_find_traces_errLatency_sum 0
jaeger_find_traces_errLatency_count 0
# HELP jaeger_find_traces_errors jaeger_find_traces_errors
# TYPE jaeger_find_traces_errors counter
jaeger_find_traces_errors 0

and now, from all-in-one:

# HELP jaeger_query_find_traces_attempts jaeger_query_find_traces_attempts
# TYPE jaeger_query_find_traces_attempts counter
jaeger_query_find_traces_attempts 1
# HELP jaeger_query_find_traces_errLatency jaeger_query_find_traces_errLatency
# TYPE jaeger_query_find_traces_errLatency histogram
jaeger_query_find_traces_errLatency_bucket{le="0.005"} 0
jaeger_query_find_traces_errLatency_bucket{le="0.01"} 0
jaeger_query_find_traces_errLatency_bucket{le="0.025"} 0
jaeger_query_find_traces_errLatency_bucket{le="0.05"} 0
jaeger_query_find_traces_errLatency_bucket{le="0.1"} 0
jaeger_query_find_traces_errLatency_bucket{le="0.25"} 0
jaeger_query_find_traces_errLatency_bucket{le="0.5"} 0
jaeger_query_find_traces_errLatency_bucket{le="1"} 0
jaeger_query_find_traces_errLatency_bucket{le="2.5"} 0
jaeger_query_find_traces_errLatency_bucket{le="5"} 0
jaeger_query_find_traces_errLatency_bucket{le="10"} 0
jaeger_query_find_traces_errLatency_bucket{le="+Inf"} 0
jaeger_query_find_traces_errLatency_sum 0
jaeger_query_find_traces_errLatency_count 0

…nt, and generated for all span readers (not just ES)

Signed-off-by: Gary Brown <gary@brownuk.com>
@black-adder black-adder merged commit e8e5962 into jaegertracing:master Sep 20, 2018
@ghost ghost removed the review label Sep 20, 2018
@objectiser objectiser deleted the querymetrics branch September 20, 2018 17:04
pavolloffay added a commit that referenced this pull request Sep 20, 2018
… component, and generated for all span readers (not just ES) (#1074)"

This reverts commit e8e5962.
pavolloffay added a commit that referenced this pull request Sep 21, 2018
* Revert "Make the metrics produced by jaeger query scoped to the query component, and generated for all span readers (not just ES) (#1074)"

Signed-off-by: Pavol Loffay <ploffay@redhat.com>
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.

4 participants