Skip to content

[receiver/windowsperfcounters] fix: Drop metrics with empty datapoints.#32384

Merged
dmitryax merged 14 commits into
open-telemetry:mainfrom
alxbl:windowsperfcounters/drop-empty-points
Jul 15, 2024
Merged

[receiver/windowsperfcounters] fix: Drop metrics with empty datapoints.#32384
dmitryax merged 14 commits into
open-telemetry:mainfrom
alxbl:windowsperfcounters/drop-empty-points

Conversation

@alxbl

@alxbl alxbl commented Apr 15, 2024

Copy link
Copy Markdown
Member

Description: When scraping Windows Performance Counters, it's possible that some counter objects do not exist. When that is the case, windowsperfcounters will still create the Metric object with no datapoints in it. Some exporters throw errors when encountering this. The fix proposed in this PR does an extra pass after all metrics have been scraped and removes the Metric objects for which no datapoints were scraped.

Link to tracking Issue: #4972

Testing:

  • Confirmed that debug exporter sees ResourceMetrics with no metrics and doesn't throw.
  • Confirmed that prometheusremotewrite exporter no longer complains about empty datapoints and that it skips the export when no metrics are available
  • No unit tests added for now. I will add a unit test once I have confirmation that this is the right way to remove empty datapoints Unit test covering the changes and enabling fixture validation which was not implemented.

Documentation:

  • CHANGELOG (user-facing)

Comment thread receiver/windowsperfcountersreceiver/windowsperfcounters_scraper.go Outdated
@dashpole

Copy link
Copy Markdown
Contributor

@braydonk

@alxbl

alxbl commented Apr 18, 2024

Copy link
Copy Markdown
Member Author

Added a unit test for empty data points and ran it:
No Fix:

--- FAIL: Test_WindowsPerfCounterScraper (0.36s)
    --- FAIL: Test_WindowsPerfCounterScraper/MetricDefinedButNoScrapedValue (0.00s)
        c:\git\opentelemetry-contrib\receiver\windowsperfcountersreceiver\windowsperfcounters_scraper_test.go:200: 
            	Error Trace:	c:/git/opentelemetry-contrib/receiver/windowsperfcountersreceiver/windowsperfcounters_scraper_test.go:200
            	Error:      	Received unexpected error:
            	            	resource "map[]": scope "": number of metrics doesn't match expected: 1, actual: 2
            	Test:       	Test_WindowsPerfCounterScraper/MetricDefinedButNoScrapedValue
FAIL
FAIL	github.com/open-telemetry/opentelemetry-collector-contrib/receiver/windowsperfcountersreceiver	0.483s
FAIL

With Fix:

PASS
ok      github.com/open-telemetry/opentelemetry-collector-contrib/receiver/windowsperfcountersreceiver  3.697s

I also found out that the tests didn't actually validate any of the data fixtures (so essentially they weren't testing much). I took the opportunity to fix that as well.

Comment thread receiver/windowsperfcountersreceiver/windowsperfcounters_scraper_test.go Outdated
Comment thread receiver/windowsperfcountersreceiver/factory_windows.go Outdated

@songy23 songy23 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please rebase to include #32518

@songy23 songy23 added the Run Windows Enable running windows test on a PR label Apr 18, 2024
@alxbl alxbl force-pushed the windowsperfcounters/drop-empty-points branch from 44e3896 to ac3d4ae Compare May 1, 2024 13:55
@alxbl alxbl force-pushed the windowsperfcounters/drop-empty-points branch from 76d09ba to 0b14085 Compare May 2, 2024 13:27
@github-actions

Copy link
Copy Markdown
Contributor

This PR was marked stale due to lack of activity. It will be closed in 14 days.

@github-actions github-actions Bot added the Stale label May 24, 2024
@github-actions github-actions Bot removed the Stale label May 25, 2024
@github-actions

github-actions Bot commented Jun 8, 2024

Copy link
Copy Markdown
Contributor

This PR was marked stale due to lack of activity. It will be closed in 14 days.

@github-actions github-actions Bot added the Stale label Jun 8, 2024
Comment thread receiver/windowsperfcountersreceiver/windowsperfcounters_scraper.go Outdated
@crobert-1 crobert-1 added the ready to merge Code review completed; ready to merge by maintainers label Jul 15, 2024
@dmitryax dmitryax merged commit f34e5ee into open-telemetry:main Jul 15, 2024
@github-actions github-actions Bot added this to the next release milestone Jul 15, 2024
pjanotti added a commit to pjanotti/opentelemetry-service-contrib that referenced this pull request Jul 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready to merge Code review completed; ready to merge by maintainers receiver/windowsperfcounters Run Windows Enable running windows test on a PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants