search

census-instrumentation / opencensus-service

OpenCensus service allows OpenCensus libraries to export to an exporter service rather than having to link vendor-specific exports.
Apache License 2.0
153 stars 63 forks source link

Fix starttime and summary has no quantiles for prometheus receiver #597

Closed fivesheep closed 5 years ago

fivesheep commented 5 years ago

To address start time for counter/histogram/summary, and take delta as values for these types.
For issue: https://github.com/census-instrumentation/opencensus-service/issues/588

It also fixes the issue when summary has no quantiles, allow it to produce a summary with nil snapshots For issue: https://github.com/census-instrumentation/opencensus-service/issues/589

cc @dinooliva @rghetia

codecov[bot] commented 5 years ago

Codecov Report

Merging #597 into master will decrease coverage by 0.15%. The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #597      +/-   ##
==========================================
- Coverage   69.13%   68.97%   -0.16%     
==========================================
  Files          92       93       +1     
  Lines        6110     6125      +15     
==========================================
+ Hits         4224     4225       +1     
- Misses       1665     1679      +14     
  Partials      221      221
Impacted Files Coverage Δ
...ceiver/prometheusreceiver/internal/metricfamily.go 100% <100%> (ø)
...iver/prometheusreceiver/internal/metricsbuilder.go 100% <100%> (ø) :arrow_up:
...eceiver/prometheusreceiver/internal/transaction.go 84.84% <100%> (-4.22%) :arrow_down:
receiver/prometheusreceiver/internal/ocastore.go 100% <100%> (+27.58%) :arrow_up:
receiver/prometheusreceiver/internal/logger.go 0% <0%> (-77.78%) :arrow_down:
receiver/prometheusreceiver/internal/metadata.go 0% <0%> (-75%) :arrow_down:
receiver/prometheusreceiver/metrics_receiver.go 67.21% <0%> (-4.92%) :arrow_down:

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 7b57179...a837176. Read the comment docs.

fivesheep commented 5 years ago

@rghetia @dinooliva updated as per #598
please do review the rewritten TestEndToEnd which integrated all the new things together, it's the best place to describe the receiver's behavior

fivesheep commented 5 years ago

the test failure is not related to the PR:

=== RUN   TestReception
--- FAIL: TestReception (0.00s)
    trace_receiver_test.go:47: Starting
    trace_receiver_test.go:50: StartTraceReception failed: listen udp :6832: bind: address already in use
FAIL
coverage: 80.5% of statements
FAIL    github.com/census-instrumentation/opencensus-service/receiver/jaegerreceiver    1.173s
fivesheep commented 5 years ago

@songy23 merged @dinooliva I have add a test for the resetting scenario, in case we might change the behavior of using the previous scrape timestamp as starttime, I have also restructure the end to end tests to make it easy to modify and adjust

songy23 commented 5 years ago

Will merge this PR by EOD tomorrow if there're no further concerns/comments.