Instrument MetricsReader with metrics #3667
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Albert Teoh <see.kwang.teoh@gmail.com>
Signed-off-by: Albert Teoh <see.kwang.teoh@gmail.com>
Signed-off-by: Albert Teoh <see.kwang.teoh@gmail.com>
Signed-off-by: Albert Teoh <see.kwang.teoh@gmail.com>
Signed-off-by: Albert Teoh <see.kwang.teoh@gmail.com>
Signed-off-by: Albert Teoh <see.kwang.teoh@gmail.com>
3 tasks
Codecov Report
@@ Coverage Diff @@
## main #3667 +/- ##
==========================================
- Coverage 96.52% 96.49% -0.03%
==========================================
Files 265 266 +1
Lines 15581 15622 +41
==========================================
+ Hits 15040 15075 +35
- Misses 453 457 +4
- Partials 88 90 +2
Continue to review full report at Codecov.
|
yurishkuro
reviewed
May 9, 2022
cmd/all-in-one/main.go
Outdated
| if err := factory.Initialize(logger); err != nil { | ||
| func createMetricsQueryService( | ||
| metricsReaderFactory *metricsPlugin.Factory, | ||
| metricsReaderMetricsFactory metrics.Factory, |
There was a problem hiding this comment.
Group logger and metrics next to each other
There was a problem hiding this comment.
Addressed in a41b9be.
| type queryMetrics struct { | ||
| Errors metrics.Counter `metric:"requests" tags:"result=err"` | ||
| Successes metrics.Counter `metric:"requests" tags:"result=ok"` | ||
| Responses metrics.Timer `metric:"responses"` // used as a histogram, not necessary for GetTrace |
There was a problem hiding this comment.
Removed in 857f64a.
| } else { | ||
| q.Successes.Inc(1) | ||
| q.OKLatency.Record(latency) | ||
| q.Responses.Record(time.Duration(responses)) |
There was a problem hiding this comment.
I don't follow, what is 1ns here?
Signed-off-by: Albert Teoh <see.kwang.teoh@gmail.com>
Signed-off-by: Albert Teoh <see.kwang.teoh@gmail.com>
Signed-off-by: Albert Teoh <see.kwang.teoh@gmail.com>
yurishkuro
approved these changes
May 9, 2022
albertteoh
added a commit
to albertteoh/jaeger
that referenced
this pull request
Jul 13, 2022
* Add metrics instrumentation for SPM (MetricsReader) Signed-off-by: Albert Teoh <see.kwang.teoh@gmail.com> * Add metrics for all-in-one and new prometheus reader Signed-off-by: Albert Teoh <see.kwang.teoh@gmail.com> * Remove new prometheus metrics instrumentation Signed-off-by: Albert Teoh <see.kwang.teoh@gmail.com> * Add license Signed-off-by: Albert Teoh <see.kwang.teoh@gmail.com> * Revert unused metrics for prometheus factory Signed-off-by: Albert Teoh <see.kwang.teoh@gmail.com> * Long params to multiline and inline return Signed-off-by: Albert Teoh <see.kwang.teoh@gmail.com> * make fmt Signed-off-by: Albert Teoh <see.kwang.teoh@gmail.com> * Group logger and metrics next to each other Signed-off-by: Albert Teoh <see.kwang.teoh@gmail.com> * Remove copied comment Signed-off-by: Albert Teoh <see.kwang.teoh@gmail.com> * Remove responses metric Signed-off-by: Albert Teoh <see.kwang.teoh@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Which problem is this PR solving?
Short description of the changes
Testing
jaeger-query
Success
With Prometheus server running.
requests
latency_buckets
Fail
Prometheus server NOT running.
requests
all-in-one
Success
With Prometheus server running.
requests
latency_buckets
Fail
Prometheus server NOT running.
requests