Skip to content

Rewrite Prometheus exporter tests#4274

Merged
pellared merged 7 commits into
open-telemetry:mainfrom
pellared:4270
Jul 4, 2023
Merged

Rewrite Prometheus exporter tests#4274
pellared merged 7 commits into
open-telemetry:mainfrom
pellared:4270

Conversation

@pellared
Copy link
Copy Markdown
Member

@pellared pellared commented Jun 30, 2023

Fixes #4270

  • Rewrite TesInvalidInsrtrumentForPrometheusIsIgnored into TestIncompatibleMeterName - it was not being executed (sic!) because of a bad name (my fault).
    The test should be more readable and it uses exported API.
    It is also more stable and does not uses goroutines which were not needed.
    The test is based on TestPrometheusExporter.
    I have also created Meter name validation opentelemetry-specification#3579

  • Rewrite TestCollectConcurrentSafe into TestCollectorConcurrentSafe.
    I reimplemented the concurrency test so that it uses exported API.
    It should not leak any goroutines (as previously the goroutines were leaking).
    I double-checked that exporter.Collect is indeed called concurrently (I used this hack to triple-check):

$ go test -timeout 30s -run ^TestCollectorConcurrentSafe$ -race
Collect start: 50
Collect start: 35
Collect end: 50
Collect start: 5
Collect end: 35
Collect start: 52
Collect start: 32
Collect end: 32
Collect end: 52
Collect end: 5
Collect start: 37
Collect start: 54
Collect start: 66
Collect start: 7
Collect start: 68
Collect end: 37
Collect end: 54
Collect end: 66
Collect end: 7
Collect end: 68
PASS
ok      go.opentelemetry.io/otel/exporters/prometheus   0.032s

@pellared pellared added the Skip Changelog PRs that do not require a CHANGELOG.md entry label Jun 30, 2023
@pellared pellared changed the title Refactor Prometheus exporter tests Rewrite Prometheus exporter tests Jun 30, 2023
@pellared pellared marked this pull request as ready for review June 30, 2023 10:35
@pellared
Copy link
Copy Markdown
Member Author

FYI @hexdigest

@codecov
Copy link
Copy Markdown

codecov Bot commented Jun 30, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 83.5%. Comparing base (10c3445) to head (cbd8a87).
Report is 1566 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@          Coverage Diff          @@
##            main   #4274   +/-   ##
=====================================
  Coverage   83.5%   83.5%           
=====================================
  Files        183     183           
  Lines      14207   14207           
=====================================
+ Hits       11869   11871    +2     
  Misses      2110    2110           
+ Partials     228     226    -2     

see 2 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@pellared pellared marked this pull request as draft June 30, 2023 11:27
@pellared pellared changed the title Rewrite Prometheus exporter tests [WIP] Rewrite Prometheus exporter tests Jun 30, 2023
@pellared pellared marked this pull request as ready for review June 30, 2023 11:44
@pellared pellared changed the title [WIP] Rewrite Prometheus exporter tests Rewrite Prometheus exporter tests Jun 30, 2023
@pellared pellared added the pkg:exporter:prometheus Related to the Prometheus exporter package label Jun 30, 2023
@pellared pellared merged commit c404a30 into open-telemetry:main Jul 4, 2023
@pellared pellared deleted the 4270 branch July 4, 2023 12:58
dashpole pushed a commit to dashpole/opentelemetry-go that referenced this pull request Jul 7, 2023
* Remove TesInvalidInsrtrumentForPrometheusIsIgnored

* Reimplement ConcurrentSafe test

* Add TestIncompatibleMeterName
@MrAlias MrAlias added the area:metrics Part of OpenTelemetry Metrics label Jun 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:metrics Part of OpenTelemetry Metrics pkg:exporter:prometheus Related to the Prometheus exporter package Skip Changelog PRs that do not require a CHANGELOG.md entry

Projects

No open projects

Development

Successfully merging this pull request may close these issues.

Address comments from #3899

4 participants