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

Prometheus Receiver: Print a more informative message about 'up' metric value #1826

Merged
merged 2 commits into from
Sep 23, 2020
Merged

Conversation

nilebox
Copy link
Member

@nilebox nilebox commented Sep 22, 2020

Description:

  • Handle 0 separately from other (invalid) values and print informative log message.
  • Delete unused OpenCensus metric

Link to tracking Issue: Fixes #1825

@codecov
Copy link

codecov bot commented Sep 22, 2020

Codecov Report

Merging #1826 into master will decrease coverage by 0.87%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1826      +/-   ##
==========================================
- Coverage   91.66%   90.78%   -0.88%     
==========================================
  Files         262      262              
  Lines       18603    15900    -2703     
==========================================
- Hits        17052    14435    -2617     
+ Misses       1118     1037      -81     
+ Partials      433      428       -5     
Impacted Files Coverage Δ
...eceiver/prometheusreceiver/internal/transaction.go 95.34% <ø> (+0.20%) ⬆️
...iver/prometheusreceiver/internal/metricsbuilder.go 100.00% <100.00%> (ø)
service/internal/templates.go 30.43% <0.00%> (-21.09%) ⬇️
processor/cloningfanoutconnector.go 57.57% <0.00%> (-9.10%) ⬇️
cmd/mdatagen/metricdata.go 71.42% <0.00%> (-8.29%) ⬇️
exporter/kafkaexporter/otlp_marshaller.go 71.42% <0.00%> (-6.35%) ⬇️
...rnal/scraper/processesscraper/processes_scraper.go 80.00% <0.00%> (-5.72%) ⬇️
extension/pprofextension/pprofextension.go 57.14% <0.00%> (-5.36%) ⬇️
service/builder/extensions_builder.go 80.76% <0.00%> (-4.53%) ⬇️
...eceiver/internal/scraper/processscraper/factory.go 55.55% <0.00%> (-4.45%) ⬇️
... and 246 more

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 e00fdab...a053e80. Read the comment docs.

@@ -159,14 +156,6 @@ func (tr *transaction) Commit() error {
return err
}

if tr.metricBuilder.hasInternalMetric {
m := ochttp.ClientRoundtripLatency.M(tr.metricBuilder.scrapeLatencyMs)
Copy link
Member Author

Choose a reason for hiding this comment

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

Removed redundant OC metric and related code since it must be registered via OpenCensus view to be used which we don't do in the collector.

Copy link
Member

@dmitryax dmitryax left a comment

Choose a reason for hiding this comment

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

LGTM

@bogdandrutu
Copy link
Member

@nilebox please fix lint issues.

@nilebox
Copy link
Member Author

nilebox commented Sep 23, 2020

@bogdandrutu fixed.

@bogdandrutu bogdandrutu merged commit 3c32f36 into open-telemetry:master Sep 23, 2020
@nilebox nilebox deleted the up-error-message branch September 23, 2020 04:38
MovieStoreGuy pushed a commit to atlassian-forks/opentelemetry-collector that referenced this pull request Nov 11, 2021
* Made copy of attribute.KeyValue

* Add comments and test

* move test to sdk/metric

* Update CHANGELOG.md

Co-authored-by: Anthony Mirabella <a9@aneurysm9.com>

* Add label order assertions for final results

* Update CHANGELOG PR number

* Revert code changes and update docs

Co-authored-by: Anthony Mirabella <a9@aneurysm9.com>
Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.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.

Misleading error message for invalid "up" metric value in Prometheus receiver
3 participants