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

[chore] [exporterhelper] Increase test coverage for persistent queue #8250

Conversation

dmitryax
Copy link
Member

@dmitryax dmitryax commented Aug 19, 2023

Add tests covering e2e data delivery using the persistent queue

@dmitryax dmitryax requested review from a team and Aneurysm9 August 19, 2023 06:57
@codecov
Copy link

codecov bot commented Aug 19, 2023

Codecov Report

Patch coverage: 80.00% and project coverage change: +0.13% 🎉

Comparison is base (92f5fe6) 90.18% compared to head (85329d9) 90.32%.
Report is 1 commits behind head on main.

❗ Current head 85329d9 differs from pull request most recent head ca19987. Consider uploading reports for the commit ca19987 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #8250      +/-   ##
==========================================
+ Coverage   90.18%   90.32%   +0.13%     
==========================================
  Files         302      303       +1     
  Lines       15859    15904      +45     
==========================================
+ Hits        14303    14365      +62     
+ Misses       1261     1243      -18     
- Partials      295      296       +1     
Files Changed Coverage Δ
exporter/exporterhelper/internal/mock_storage.go 80.00% <80.00%> (ø)

... and 6 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@dmitryax dmitryax force-pushed the exporter-helper-persistent-queue-e2e-tests branch 2 times, most recently from c752e83 to 3930908 Compare August 21, 2023 16:30
@dmitryax dmitryax closed this Aug 21, 2023
@dmitryax dmitryax deleted the exporter-helper-persistent-queue-e2e-tests branch August 21, 2023 22:10
@dmitryax dmitryax restored the exporter-helper-persistent-queue-e2e-tests branch August 21, 2023 22:31
@dmitryax dmitryax reopened this Aug 21, 2023
@dmitryax dmitryax force-pushed the exporter-helper-persistent-queue-e2e-tests branch 2 times, most recently from 84c4ad2 to e11eb7b Compare August 21, 2023 23:19
exporter/exporterhelper/queued_retry_test.go Outdated Show resolved Hide resolved
@codeboten
Copy link
Contributor

There's a test failing:

        	Error:      	could not find metric exporter/queue_capacity with tags [{{exporter} test}] reported

@dmitryax
Copy link
Member Author

There's a test failing:

It looks like a conflict caused by reusing the global instrument for OC. Looking into how to workaround or fix it it

@dmitryax dmitryax marked this pull request as draft August 25, 2023 22:38
@dmitryax dmitryax force-pushed the exporter-helper-persistent-queue-e2e-tests branch 14 times, most recently from 042e173 to 676366f Compare August 28, 2023 04:06
@dmitryax dmitryax force-pushed the exporter-helper-persistent-queue-e2e-tests branch from 676366f to 85329d9 Compare August 28, 2023 04:09
@dmitryax dmitryax force-pushed the exporter-helper-persistent-queue-e2e-tests branch from 85329d9 to 8f4a3cb Compare August 28, 2023 04:45
dmitryax added a commit that referenced this pull request Aug 28, 2023
`checkValueForProducer` test helper assumes that every OC metric has
only one timeseries. But, since the queue metrics are defined globally,
they accumulate more timeseries with different labels from different
tests. This change removes that assumption. It allows having more tests
for the exporter queue without breaking
TestQueuedRetry_QueueMetricsReported.

Ideally, we should avoid defining instruments globally or clear them up
after each test. The first option would require passing the OpenCensus
registry in as a public field of TelemetrySettings from the service
start. The second option would require a bigger refactoring.

I think we can allow several tests emitting datapoints for the global
instruments for now until we migrate to OTel metrics.

Fixes tests failing in
#8250
@dmitryax dmitryax marked this pull request as ready for review August 28, 2023 15:33
Add tests covering e2e data delivery through persistent storage
@dmitryax dmitryax force-pushed the exporter-helper-persistent-queue-e2e-tests branch from 8f4a3cb to ca19987 Compare August 28, 2023 15:34
@dmitryax dmitryax merged commit 0af1c11 into open-telemetry:main Aug 28, 2023
29 of 30 checks passed
@github-actions github-actions bot added this to the next release milestone Aug 28, 2023
@dmitryax dmitryax deleted the exporter-helper-persistent-queue-e2e-tests branch August 28, 2023 17:32
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.

2 participants