Skip to content

Conversation

@nilebox
Copy link
Member

@nilebox nilebox commented Sep 17, 2020

Description: Getting rid of redundant nil ResourceObject when all metrics have resource specified.

After removal of pdatautil package in #1739, I've migrated our custom components from pdatautil.MetricsFromMetricsData to internaldata.OCToMetrics and found that some tests are failing due to converted metrics having an extra "empty" ResourceMetrics object in the slice.

Testing: Added unit test reproducing the issue

/cc @bogdandrutu

@codecov
Copy link

codecov bot commented Sep 17, 2020

Codecov Report

Merging #1803 into master will decrease coverage by 0.89%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1803      +/-   ##
==========================================
- Coverage   91.66%   90.76%   -0.90%     
==========================================
  Files         262      262              
  Lines       18603    15914    -2689     
==========================================
- Hits        17052    14445    -2607     
+ Misses       1118     1039      -79     
+ Partials      433      430       -3     
Impacted Files Coverage Δ
translator/internaldata/oc_to_metrics.go 92.38% <100.00%> (-0.61%) ⬇️
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%) ⬇️
receiver/otlpreceiver/marshal_jsonpb.go 12.61% <0.00%> (-4.45%) ⬇️
... and 245 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...9b18367. Read the comment docs.

@nilebox nilebox changed the title Skip creation of redundant nil resource if there are no combined metrics Skip creation of redundant nil resource in translation from OC if there are no combined metrics Sep 17, 2020
@tigrannajaryan tigrannajaryan merged commit 3fafbbc into open-telemetry:master Sep 24, 2020
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.

5 participants